Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/controllers/brands.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ function BrandsController(ctx, log, env) {
Product: context?.pathInfo?.headers?.['x-product'],
},
},
{ environment: resolveEnvironment(env), namespace: BRAND_METRICS_NAMESPACE },
{ environment: resolveEnvironment(env, { log }), namespace: BRAND_METRICS_NAMESPACE },
);
log.warn(`BrandDemotionBlocked: ${operation} attempted an active->pending demotion `
+ `(org=${context?.params?.spaceCatId}, brand=${context?.params?.brandId}, `
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function RedirectsController(ctx) {
// Capture start time up front so every terminal branch reports duration.
const startedAt = Date.now();
const emitOpts = {
environment: resolveEnvironment(env),
environment: resolveEnvironment(env, { log }),
namespace: ASO_OVERLAY_NAMESPACE,
};

Expand Down
4 changes: 2 additions & 2 deletions src/controllers/webhooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function WebhooksController(context) {
return await fn(ctx);
} catch (e) {
log.error('GitHub webhook handler error', e);
emitMetric({ name: 'WebhookHandlerError', dimensions: { Reason: 'uncaught_exception' } }, { environment: resolveEnvironment(env) });
emitMetric({ name: 'WebhookHandlerError', dimensions: { Reason: 'uncaught_exception' } }, { environment: resolveEnvironment(env, { log }) });
return internalServerError('Internal error');
}
};
Expand All @@ -98,7 +98,7 @@ function WebhooksController(context) {
const deliveryId = ctx.pathInfo?.headers?.['x-github-delivery'];
const { data } = ctx;

const environment = resolveEnvironment(env);
const environment = resolveEnvironment(env, { log });
const startedAt = Date.now();
let outcome = 'unknown';

Expand Down
2 changes: 1 addition & 1 deletion src/support/aso-overlay-key-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class AsoOverlayKeyHandler extends AbstractHandler {

// From here on, we're on the overlay route — every outcome is worth a metric.
const emitOpts = {
environment: resolveEnvironment(context.env),
environment: resolveEnvironment(context.env, { log: context.log }),
namespace: ASO_OVERLAY_NAMESPACE,
};

Expand Down
2 changes: 1 addition & 1 deletion src/support/github-webhook-hmac-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class GitHubWebhookHmacHandler extends AbstractHandler {
// Best-effort metric helpers — defined here so context.env is in scope.
// Both swallow errors (emitMetric is itself best-effort, and these are
// called on auth-rejection paths where we must never throw).
const environment = resolveEnvironment(context.env);
const environment = resolveEnvironment(context.env, { log: context.log });
const rejected = (reason) => emitMetric(
{ name: 'WebhookRejected', dimensions: { Reason: reason } },
{ environment },
Expand Down
48 changes: 46 additions & 2 deletions src/support/metrics-emf.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,59 @@

const NAMESPACE = 'Mysticat/GitHubService';

// Module-scoped flag so we warn at most once per Lambda instance when the
// environment falls back to the default. Emitting per-request would flood the
// log for every invocation on a mis-configured deploy; once-per-instance is
// enough for ops to notice on the first cold start and still avoid confusing
// on-call with a warn spike that dwarfs the actual outcome events.
let envFallbackWarned = false;

/**
* Resolves the deployment environment name from process/Lambda env vars.
* Precedence: AWS_ENV > ENV > 'dev'.
*
* If neither variable is set the caller falls back to 'dev'. In prod that is a
* silent mis-labeling — every metric routes to the dev dashboard while prod
* dashboards read as zero traffic. Pass `log` to have this warned once per
* Lambda instance (via `log.warn`) so a broken deploy manifest surfaces on the
* very first invocation rather than staying invisible until someone spot-checks
* the wrong-tier dashboard.
*
* @param {object} env - The env object (context.env or process.env)
* @param {object} [opts] - Options
* @param {object} [opts.log] - Optional logger for the default-fallback warning
* @returns {string} Environment name
*/
export function resolveEnvironment(env = {}) {
return env.AWS_ENV || env.ENV || 'dev';
export function resolveEnvironment(env = {}, { log } = {}) {
if (env.AWS_ENV) {
return env.AWS_ENV;
}
if (env.ENV) {
return env.ENV;
}
// Defensive: some callers pass a partial logger (info/error only) — helix
// and Lambda contexts historically only guarantee `log.info`, so `log.warn`
// may be absent. Skip silently rather than crash the request path.
if (log && typeof log.warn === 'function' && !envFallbackWarned) {
envFallbackWarned = true;
log.warn(
'[metrics-emf] resolveEnvironment: neither AWS_ENV nor ENV is set — '
+ "defaulting to 'dev'. In a production deploy this mis-labels metrics; "
+ 'check the Lambda env-var manifest.',
);
}
return 'dev';
}

/**
* Test-only: reset the module-scoped warn-once latch so unit tests can verify
* the first-call semantics without cross-test bleed. Exported (not prefixed
* with an underscore, which lint forbids) but named to make its scope obvious
* so no production caller reaches for it by mistake.
*/
// eslint-disable-next-line camelcase
export function resetEnvFallbackWarnedForTest() {
envFallbackWarned = false;
}

/**
Expand Down
42 changes: 41 additions & 1 deletion test/support/metrics-emf.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
*/

import { expect } from 'chai';
import { emitMetric, resolveEnvironment } from '../../src/support/metrics-emf.js';
import sinon from 'sinon';
import {
emitMetric,
resolveEnvironment,
resetEnvFallbackWarnedForTest,
} from '../../src/support/metrics-emf.js';

describe('metrics-emf', () => {
it('resolveEnvironment prefers AWS_ENV, then ENV, then dev', () => {
Expand All @@ -20,6 +25,41 @@ describe('metrics-emf', () => {
expect(resolveEnvironment({})).to.equal('dev');
});

describe('resolveEnvironment: default-fallback warn-once', () => {
beforeEach(() => resetEnvFallbackWarnedForTest());
afterEach(() => resetEnvFallbackWarnedForTest());

it('does NOT warn when AWS_ENV is set', () => {
const log = { warn: sinon.spy() };
expect(resolveEnvironment({ AWS_ENV: 'prod' }, { log })).to.equal('prod');
expect(log.warn.called).to.be.false;
});

it('does NOT warn when ENV is set (AWS_ENV missing)', () => {
const log = { warn: sinon.spy() };
expect(resolveEnvironment({ ENV: 'stage' }, { log })).to.equal('stage');
expect(log.warn.called).to.be.false;
});

it('warns exactly once per Lambda instance when defaulting to dev', () => {
const log = { warn: sinon.spy() };
// First call: log.warn fires.
expect(resolveEnvironment({}, { log })).to.equal('dev');
expect(log.warn.callCount).to.equal(1);
expect(log.warn.firstCall.args[0]).to.include('AWS_ENV nor ENV');
// Second call same instance: still 'dev', but no additional warn — the
// guard prevents flooding on every request when the manifest is broken.
expect(resolveEnvironment({}, { log })).to.equal('dev');
expect(log.warn.callCount).to.equal(1);
});

it('is a no-op when no log is passed (backwards compat with old callers)', () => {
// Legacy callers pass no options; the function must still return 'dev'
// without throwing, and must not require the log param.
expect(resolveEnvironment({})).to.equal('dev');
});
});

it('emits a well-formed EMF envelope to the injected sink', () => {
const lines = [];
emitMetric(
Expand Down
Loading