diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index a309186f0..f34c6a159 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -1,7 +1,9 @@ import { Console } from 'node:console'; import { isIntegerNumber, + isNullOrUndefined, isNumber, + isRecord, isString, isStringUndefinedNullEmpty, Utility, @@ -240,8 +242,10 @@ class Metrics extends Utility implements MetricsInterface { super(); this.dimensions = {}; - this.setOptions(options); + this.setEnvConfig(); + this.setConsole(); this.#logger = options.logger || this.console; + this.setOptions(options); } /** @@ -824,13 +828,41 @@ class Metrics extends Utility implements MetricsInterface { * @param dimensions - The dimensions to be added to the default dimensions object */ public setDefaultDimensions(dimensions: Dimensions | undefined): void { + if (isNullOrUndefined(dimensions) || !isRecord(dimensions)) { + return; + } + + const cleanedDimensions: Dimensions = {}; + + for (const [key, value] of Object.entries(dimensions)) { + if ( + isStringUndefinedNullEmpty(key) || + isStringUndefinedNullEmpty(value) + ) { + this.#logger.warn( + `The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings` + ); + continue; + } + + if (Object.hasOwn(this.defaultDimensions, key)) { + this.#logger.warn( + `Dimension "${key}" has already been added. The previous value will be overwritten.` + ); + } + + cleanedDimensions[key] = value; + } + const targetDimensions = { ...this.defaultDimensions, - ...dimensions, + ...cleanedDimensions, }; - if (MAX_DIMENSION_COUNT <= Object.keys(targetDimensions).length) { + + if (Object.keys(targetDimensions).length >= MAX_DIMENSION_COUNT) { throw new Error('Max dimension count hit'); } + this.defaultDimensions = targetDimensions; } @@ -1058,8 +1090,6 @@ class Metrics extends Utility implements MetricsInterface { functionName, } = options; - this.setEnvConfig(); - this.setConsole(); this.setCustomConfigService(customConfigService); this.setDisabled(); this.setNamespace(namespace); diff --git a/packages/metrics/tests/unit/customTimestamp.test.ts b/packages/metrics/tests/unit/customTimestamp.test.ts index 61e3ffa22..01d6589eb 100644 --- a/packages/metrics/tests/unit/customTimestamp.test.ts +++ b/packages/metrics/tests/unit/customTimestamp.test.ts @@ -48,7 +48,9 @@ describe('Setting custom timestamp', () => { it('logs a warning when the provided timestamp is too far in the past', () => { // Prepare - const metrics = new Metrics({ singleMetric: true }); + const metrics = new Metrics({ + singleMetric: true, + }); // Act metrics.setTimestamp(Date.now() - EMF_MAX_TIMESTAMP_PAST_AGE - 1000); @@ -63,7 +65,9 @@ describe('Setting custom timestamp', () => { it('logs a warning when the provided timestamp is too far in the future', () => { // Prepare - const metrics = new Metrics({ singleMetric: true }); + const metrics = new Metrics({ + singleMetric: true, + }); // Act metrics.setTimestamp(Date.now() + EMF_MAX_TIMESTAMP_FUTURE_AGE + 1000); diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index 1a874ec02..d9d39f471 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -552,4 +552,104 @@ describe('Working with dimensions', () => { }) ); }); + + it('warns when setDefaultDimensions overwrites existing dimensions', () => { + // Prepare + const metrics = new Metrics({ + namespace: DEFAULT_NAMESPACE, + defaultDimensions: { environment: 'prod' }, + }); + + // Act + metrics.setDefaultDimensions({ region: 'us-east-1' }); + metrics.setDefaultDimensions({ + environment: 'staging', // overwrites default dimension + }); + + // Assess + expect(console.warn).toHaveBeenCalledOnce(); + expect(console.warn).toHaveBeenCalledWith( + 'Dimension "environment" has already been added. The previous value will be overwritten.' + ); + }); + + it('returns immediately if dimensions is undefined', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + namespace: DEFAULT_NAMESPACE, + }); + + // Act + metrics.addMetric('myMetric', MetricUnit.Count, 1); + + // Assert + expect(console.warn).not.toHaveBeenCalled(); + + expect(console.log).toHaveEmittedEMFWith( + expect.objectContaining({ + service: 'hello-world', + }) + ); + }); + + it.each([ + { value: undefined, name: 'valid-name' }, + { value: null, name: 'valid-name' }, + { value: '', name: 'valid-name' }, + { value: 'valid-value', name: '' }, + ])( + 'skips invalid default dimension values in setDefaultDimensions ($name)', + ({ value, name }) => { + // Arrange + const metrics = new Metrics({ + singleMetric: true, + namespace: DEFAULT_NAMESPACE, + }); + + // Act + metrics.setDefaultDimensions({ + validDimension: 'valid', + [name as string]: value as string, + }); + + metrics.addMetric('test', MetricUnit.Count, 1); + metrics.publishStoredMetrics(); + + // Assert + expect(console.warn).toHaveBeenCalledWith( + `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings` + ); + + expect(console.log).toHaveEmittedEMFWith( + expect.objectContaining({ validDimension: 'valid' }) + ); + + expect(console.log).toHaveEmittedEMFWith( + expect.not.objectContaining({ [name]: value }) + ); + } + ); + it('returns immediately without logging if dimensions is not a plain object', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + namespace: DEFAULT_NAMESPACE, + }); + + // Act + // @ts-expect-error – simulate runtime misuse + metrics.setDefaultDimensions('not-an-object'); + + // Assert + expect(console.warn).not.toHaveBeenCalled(); + + metrics.addMetric('someMetric', MetricUnit.Count, 1); + + expect(console.log).toHaveEmittedEMFWith( + expect.objectContaining({ + service: 'hello-world', + }) + ); + }); }); diff --git a/packages/metrics/tests/unit/initializeMetrics.test.ts b/packages/metrics/tests/unit/initializeMetrics.test.ts index b1193e99d..7bb1ab5c7 100644 --- a/packages/metrics/tests/unit/initializeMetrics.test.ts +++ b/packages/metrics/tests/unit/initializeMetrics.test.ts @@ -65,7 +65,9 @@ describe('Initialize Metrics', () => { it('uses the default namespace when none is provided', () => { // Prepare - const metrics = new Metrics({ singleMetric: true }); + const metrics = new Metrics({ + singleMetric: true, + }); // Act metrics.addMetric('test', MetricUnit.Count, 1);