diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 3b4a8c9f95..101f03474c 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -36,6 +36,7 @@ import { MetricsStore } from './MetricsStore.js'; import type { ConfigServiceInterface, Dimensions, + EmfKeySource, EmfOutput, ExtraOptions, MetricDefinition, @@ -758,6 +759,14 @@ class Metrics extends Utility implements MetricsInterface { dimensionNames.push(Object.keys(defaultDimensions)); } + this.#checkKeyCollisions( + defaultDimensions, + dimensions, + dimensionSets, + metricValues, + this.#metadataStore.getAll() + ); + return { _aws: { Timestamp: this.#metricsStore.getTimestamp() ?? Date.now(), @@ -769,6 +778,10 @@ class Metrics extends Utility implements MetricsInterface { }, ], }, + // Metadata is spread first so that any colliding key from a dimension + // source overrides it: dimensions are load-bearing for EMF aggregation, + // metadata is incidental context. + ...this.#metadataStore.getAll(), ...defaultDimensions, ...dimensions, // Merge all dimension sets efficiently by mutating the accumulator @@ -779,10 +792,48 @@ class Metrics extends Utility implements MetricsInterface { return acc; }, {}), ...metricValues, - ...this.#metadataStore.getAll(), }; } + #checkKeyCollisions( + defaultDimensions: Dimensions, + dimensions: Dimensions, + dimensionSets: Dimensions[], + metricValues: Record, + metadata: Record + ): void { + const seenKeys = new Map(); + + const setKey = (k: string, source: EmfKeySource) => { + const existing = seenKeys.get(k); + if (existing !== undefined) { + this.#logger.warn( + `EMF key "${k}" is defined as both a ${existing} and ${source}; the ${source} value will take precedence in the serialized output` + ); + } + seenKeys.set(k, source); + }; + + // Seed in precedence order (lowest to highest). The spread in + // serializeMetrics() applies the same order, so the source named in the + // warning genuinely wins in the output. + for (const k of Object.keys(metadata)) setKey(k, 'metadata'); + for (const k of Object.keys(defaultDimensions)) + setKey(k, 'default dimension'); + for (const k of Object.keys(dimensions)) setKey(k, 'dimension'); + for (const set of dimensionSets) + for (const k of Object.keys(set)) setKey(k, 'dimension set'); + + for (const name of Object.keys(metricValues)) { + const src = seenKeys.get(name); + if (src !== undefined) { + throw new Error( + `EMF key collision on "${name}": registered as both a metric (number) and a ${src} (string)` + ); + } + } + } + /** * Set default dimensions that will be added to all metrics. * diff --git a/packages/metrics/src/types/Metrics.ts b/packages/metrics/src/types/Metrics.ts index 8430c65a61..33748283c8 100644 --- a/packages/metrics/src/types/Metrics.ts +++ b/packages/metrics/src/types/Metrics.ts @@ -13,6 +13,16 @@ import type { ConfigServiceInterface } from './ConfigServiceInterface.js'; */ type Dimensions = Record; +/** + * The source of a key when emitting EMF output, used by the collision + * detection in {@link MetricsInterface.serializeMetrics | `serializeMetrics()`}. + */ +type EmfKeySource = + | 'default dimension' + | 'dimension' + | 'dimension set' + | 'metadata'; + /** * Options to configure the Metrics class. * @@ -540,6 +550,7 @@ interface MetricsInterface { export type { Dimensions, + EmfKeySource, EmfOutput, ExtraOptions, MetricDefinition, diff --git a/packages/metrics/src/types/index.ts b/packages/metrics/src/types/index.ts index ced7349ffd..299f9dfd73 100644 --- a/packages/metrics/src/types/index.ts +++ b/packages/metrics/src/types/index.ts @@ -1,6 +1,7 @@ export type { ConfigServiceInterface } from './ConfigServiceInterface.js'; export type { Dimensions, + EmfKeySource, EmfOutput, ExtraOptions, MetricDefinition, diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index ace544da0f..64635fa72c 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -728,4 +728,127 @@ describe('Working with dimensions', () => { // Assess expect(() => metrics.publishStoredMetrics()).not.toThrow(); }); + + it('throws when a metric name conflicts with an existing dimension key', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + }); + metrics.addDimension('environment', 'prod'); + + // Act & Assess + expect(() => + metrics.addMetric('environment', MetricUnit.Count, 1) + ).toThrowError( + 'EMF key collision on "environment": registered as both a metric (number) and a dimension (string)' + ); + }); + + it('throws when a metric name conflicts with an existing default dimension key', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + defaultDimensions: { environment: 'prod' }, + }); + + // Act & Assess + expect(() => + metrics.addMetric('environment', MetricUnit.Count, 1) + ).toThrowError( + 'EMF key collision on "environment": registered as both a metric (number) and a default dimension (string)' + ); + }); + + it('throws when a metric name conflicts with the built-in service dimension', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + }); + + // Act & Assess + expect(() => + metrics.addMetric('service', MetricUnit.Count, 1) + ).toThrowError( + 'EMF key collision on "service": registered as both a metric (number) and a default dimension (string)' + ); + }); + + it('throws when a metric name conflicts with a key added via addDimensions', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + }); + metrics.addDimensions({ environment: 'prod' }); + + // Act & Assess + expect(() => + metrics.addMetric('environment', MetricUnit.Count, 1) + ).toThrowError( + 'EMF key collision on "environment": registered as both a metric (number) and a dimension set (string)' + ); + }); + + it('throws on serialize when a dimension key is added after a metric with the same name', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addMetric('environment', MetricUnit.Count, 1); + metrics.addDimension('environment', 'prod'); + + // Act & Assess + expect(() => metrics.serializeMetrics()).toThrowError( + 'EMF key collision on "environment": registered as both a metric (number) and a dimension (string)' + ); + }); + + it('warns on serialize when a dimension key overwrites a default dimension key', () => { + // Prepare + const metrics = new Metrics({ + namespace: 'test', + defaultDimensions: { environment: 'prod' }, + }); + metrics.addDimension('environment', 'staging'); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a default dimension and dimension; the dimension value will take precedence in the serialized output' + ); + }); + + it('warns on serialize when a dimension set key overwrites a regular dimension key', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addDimension('environment', 'prod'); + metrics.addDimensions({ environment: 'staging' }); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a dimension and dimension set; the dimension set value will take precedence in the serialized output' + ); + }); + + it('warns on serialize when a dimension set key overwrites a default dimension key', () => { + // Prepare + const metrics = new Metrics({ + namespace: 'test', + defaultDimensions: { environment: 'prod' }, + }); + metrics.addDimensions({ environment: 'staging' }); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a default dimension and dimension set; the dimension set value will take precedence in the serialized output' + ); + }); }); diff --git a/packages/metrics/tests/unit/metadata.test.ts b/packages/metrics/tests/unit/metadata.test.ts index 5b0353f0c5..44fbd58b40 100644 --- a/packages/metrics/tests/unit/metadata.test.ts +++ b/packages/metrics/tests/unit/metadata.test.ts @@ -86,4 +86,98 @@ describe('Working with metadata', () => { expect.not.objectContaining({ 'cost-center': '1234' }) ); }); + + it('throws on serialize when metadata key and metric name collide (metric first)', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addMetric('request_count', MetricUnit.Count, 42); + metrics.addMetadata('request_count', 'not-a-number'); + + // Act & Assess + expect(() => metrics.serializeMetrics()).toThrowError( + 'EMF key collision on "request_count": registered as both a metric (number) and a metadata (string)' + ); + }); + + it('throws on serialize when metadata key and metric name collide (metadata first)', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addMetadata('request_count', 'not-a-number'); + metrics.addMetric('request_count', MetricUnit.Count, 42); + + // Act & Assess + expect(() => metrics.serializeMetrics()).toThrowError( + 'EMF key collision on "request_count": registered as both a metric (number) and a metadata (string)' + ); + }); + + it('warns on serialize when a metadata key matches a dimension key, and the dimension wins', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addDimension('environment', 'prod'); + metrics.addMetadata('environment', 'metadata-value'); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + const serialized = metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a metadata and dimension; the dimension value will take precedence in the serialized output' + ); + expect(serialized).toMatchObject({ environment: 'prod' }); + }); + + it('warns on serialize when a metadata key matches a default dimension key, and the default dimension wins', () => { + // Prepare + const metrics = new Metrics({ + namespace: 'test', + defaultDimensions: { environment: 'prod' }, + }); + metrics.addMetadata('environment', 'metadata-value'); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + const serialized = metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a metadata and default dimension; the default dimension value will take precedence in the serialized output' + ); + expect(serialized).toMatchObject({ environment: 'prod' }); + }); + + it('warns on serialize when a metadata key matches a dimension set key, and the dimension set wins', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addDimensions({ environment: 'prod' }); + metrics.addMetadata('environment', 'metadata-value'); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + const serialized = metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a metadata and dimension set; the dimension set value will take precedence in the serialized output' + ); + expect(serialized).toMatchObject({ environment: 'prod' }); + }); + + it('still lets the dimension win and warns even when addMetadata is called before addDimension', () => { + // Prepare + const metrics = new Metrics({ namespace: 'test' }); + metrics.addMetadata('environment', 'metadata-value'); + metrics.addDimension('environment', 'prod'); + metrics.addMetric('test', MetricUnit.Count, 1); + + // Act + const serialized = metrics.serializeMetrics(); + + // Assess + expect(console.warn).toHaveBeenCalledWith( + 'EMF key "environment" is defined as both a metadata and dimension; the dimension value will take precedence in the serialized output' + ); + expect(serialized).toMatchObject({ environment: 'prod' }); + }); });