From db6330a19dcccd14c5da9911e74f4fbbbd5dbd08 Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Mon, 28 Jul 2025 16:18:17 +0530 Subject: [PATCH 1/7] fix(metrics): emit warning when default dimensions are overwritten --- packages/metrics/src/Metrics.ts | 36 +++++++-- .../metrics/tests/unit/dimensions.test.ts | 75 +++++++++++++++++++ 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index a309186f0e..87b53fc886 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -240,8 +240,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 +826,39 @@ 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 (!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 +1086,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/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index 1a874ec024..e40df93983 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -552,4 +552,79 @@ 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.setDefaultDimensions(undefined); + metrics.addMetric('myMetric', MetricUnit.Count, 1); + // Assess: No warning, only default dimensions/service + 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 }) + ); + } + ); }); From 250ece6d2f4cef1fd8e20fe2e36d3f88a50c14b7 Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Mon, 28 Jul 2025 20:21:48 +0530 Subject: [PATCH 2/7] feat(metrics): warn when setDefaultDimensions is called without dimensions --- packages/metrics/src/Metrics.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 87b53fc886..36b3e2b24f 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -826,7 +826,12 @@ 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 (!dimensions) return; + if (!dimensions) { + this.#logger.warn( + 'No dimensions were supplied to setDefaultDimensions. Skipping update.' + ); + return; + } const cleanedDimensions: Dimensions = {}; From cf503fea985932d0d74b0852de3c0362ac562b53 Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Mon, 28 Jul 2025 20:22:16 +0530 Subject: [PATCH 3/7] test(metrics): update tests to pass defaultDimensions and avoid warning logs --- .../tests/unit/creatingMetrics.test.ts | 3 +++ .../tests/unit/customTimestamp.test.ts | 20 +++++++++++++++-- .../metrics/tests/unit/dimensions.test.ts | 22 +++++++++++++++---- .../tests/unit/initializeMetrics.test.ts | 7 +++++- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/metrics/tests/unit/creatingMetrics.test.ts b/packages/metrics/tests/unit/creatingMetrics.test.ts index e8b42005f9..b37d583470 100644 --- a/packages/metrics/tests/unit/creatingMetrics.test.ts +++ b/packages/metrics/tests/unit/creatingMetrics.test.ts @@ -182,6 +182,9 @@ describe('Creating metrics', () => { const metrics = new Metrics({ singleMetric: false, namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + enviroment: 'test', + }, }); // Act diff --git a/packages/metrics/tests/unit/customTimestamp.test.ts b/packages/metrics/tests/unit/customTimestamp.test.ts index 61e3ffa221..22915f1d15 100644 --- a/packages/metrics/tests/unit/customTimestamp.test.ts +++ b/packages/metrics/tests/unit/customTimestamp.test.ts @@ -48,7 +48,12 @@ 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, + defaultDimensions: { + environment: 'test', + }, + }); // Act metrics.setTimestamp(Date.now() - EMF_MAX_TIMESTAMP_PAST_AGE - 1000); @@ -63,7 +68,12 @@ 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, + defaultDimensions: { + environment: 'test', + }, + }); // Act metrics.setTimestamp(Date.now() + EMF_MAX_TIMESTAMP_FUTURE_AGE + 1000); @@ -81,6 +91,9 @@ describe('Setting custom timestamp', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + environment: 'test', + }, }); // Act @@ -108,6 +121,9 @@ describe('Setting custom timestamp', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + environment: 'test', + }, }); // Act diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index e40df93983..ad7adfa660 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -462,6 +462,9 @@ describe('Working with dimensions', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + enviroment: 'test', + }, }); // Act & Assess @@ -498,6 +501,9 @@ describe('Working with dimensions', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + enviroment: 'test', + }, }); // Act & Assess @@ -573,23 +579,28 @@ describe('Working with dimensions', () => { ); }); - it('returns immediately if dimensions is undefined', () => { + it('logs a warning and returns immediately if dimensions is undefined', () => { // Prepare const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, }); + // Act - metrics.setDefaultDimensions(undefined); metrics.addMetric('myMetric', MetricUnit.Count, 1); - // Assess: No warning, only default dimensions/service - expect(console.warn).not.toHaveBeenCalled(); + + // Assert + expect(console.warn).toHaveBeenCalledWith( + 'No dimensions were supplied to setDefaultDimensions. Skipping update.' + ); + expect(console.log).toHaveEmittedEMFWith( expect.objectContaining({ service: 'hello-world', }) ); }); + it.each([ { value: undefined, name: 'valid-name' }, { value: null, name: 'valid-name' }, @@ -602,6 +613,9 @@ describe('Working with dimensions', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + enviroment: 'test', + }, }); // Act diff --git a/packages/metrics/tests/unit/initializeMetrics.test.ts b/packages/metrics/tests/unit/initializeMetrics.test.ts index b1193e99da..1bba8405cf 100644 --- a/packages/metrics/tests/unit/initializeMetrics.test.ts +++ b/packages/metrics/tests/unit/initializeMetrics.test.ts @@ -65,7 +65,12 @@ 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, + defaultDimensions: { + environment: 'test', + }, + }); // Act metrics.addMetric('test', MetricUnit.Count, 1); From 0870337d293a3a95d2da079d20d303ab495841ea Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Tue, 29 Jul 2025 12:24:02 +0530 Subject: [PATCH 4/7] feat(metrics): warn and skip setDefaultDimensions when input fails isRecord check --- packages/metrics/src/Metrics.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 36b3e2b24f..136bb4b973 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, @@ -826,13 +828,20 @@ 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 (!dimensions) { + if (isNullOrUndefined(dimensions)) { this.#logger.warn( 'No dimensions were supplied to setDefaultDimensions. Skipping update.' ); return; } + if (!isRecord(dimensions)) { + this.#logger.warn( + 'Invalid dimensions type provided to setDefaultDimensions. Expected an object.' + ); + return; + } + const cleanedDimensions: Dimensions = {}; for (const [key, value] of Object.entries(dimensions)) { From b64a1bb5e14c8b81cc2195cb38d3ddccba558a2a Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Tue, 29 Jul 2025 12:26:49 +0530 Subject: [PATCH 5/7] test(metrics): warn and skip setDefaultDimensions when input fails isRecord check Adds a unit test to ensure setDefaultDimensions logs a warning and returns early when passed a non-object value (e.g., a string), validating the runtime isRecord guard. --- .../metrics/tests/unit/dimensions.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index ad7adfa660..bec86647f0 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -641,4 +641,31 @@ describe('Working with dimensions', () => { ); } ); + it('logs a warning and returns immediately if dimensions is not a plain object (fails isRecord)', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: true, + namespace: DEFAULT_NAMESPACE, + defaultDimensions: { + enviroment: 'test', + }, + }); + + // Act + // @ts-expect-error – intentionally passing an invalid value to simulate bad input at runtime + metrics.setDefaultDimensions('invalid-dimensions'); + + // Assert + expect(console.warn).toHaveBeenCalledWith( + 'Invalid dimensions type provided to setDefaultDimensions. Expected an object.' + ); + + metrics.addMetric('someMetric', MetricUnit.Count, 1); + + expect(console.log).toHaveEmittedEMFWith( + expect.objectContaining({ + service: 'hello-world', + }) + ); + }); }); From 231dd43ebe573f2345ede80312b9b9b6281e458b Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Tue, 29 Jul 2025 13:14:38 +0530 Subject: [PATCH 6/7] test(metrics): update setDefaultDimensions tests to reflect silent return on invalid input --- packages/metrics/src/Metrics.ts | 12 +----------- .../metrics/tests/unit/dimensions.test.ts | 19 ++++++------------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 136bb4b973..f34c6a1597 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -828,17 +828,7 @@ 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)) { - this.#logger.warn( - 'No dimensions were supplied to setDefaultDimensions. Skipping update.' - ); - return; - } - - if (!isRecord(dimensions)) { - this.#logger.warn( - 'Invalid dimensions type provided to setDefaultDimensions. Expected an object.' - ); + if (isNullOrUndefined(dimensions) || !isRecord(dimensions)) { return; } diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index bec86647f0..6f7c030a9a 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -579,7 +579,7 @@ describe('Working with dimensions', () => { ); }); - it('logs a warning and returns immediately if dimensions is undefined', () => { + it('returns immediately if dimensions is undefined', () => { // Prepare const metrics = new Metrics({ singleMetric: true, @@ -590,9 +590,7 @@ describe('Working with dimensions', () => { metrics.addMetric('myMetric', MetricUnit.Count, 1); // Assert - expect(console.warn).toHaveBeenCalledWith( - 'No dimensions were supplied to setDefaultDimensions. Skipping update.' - ); + expect(console.warn).not.toHaveBeenCalled(); expect(console.log).toHaveEmittedEMFWith( expect.objectContaining({ @@ -641,24 +639,19 @@ describe('Working with dimensions', () => { ); } ); - it('logs a warning and returns immediately if dimensions is not a plain object (fails isRecord)', () => { + it('returns immediately without logging if dimensions is not a plain object', () => { // Prepare const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - enviroment: 'test', - }, }); // Act - // @ts-expect-error – intentionally passing an invalid value to simulate bad input at runtime - metrics.setDefaultDimensions('invalid-dimensions'); + // @ts-expect-error – simulate runtime misuse + metrics.setDefaultDimensions('not-an-object'); // Assert - expect(console.warn).toHaveBeenCalledWith( - 'Invalid dimensions type provided to setDefaultDimensions. Expected an object.' - ); + expect(console.warn).not.toHaveBeenCalled(); metrics.addMetric('someMetric', MetricUnit.Count, 1); From ca20e8bd1c1be6772fd6068afac9fb223e333ef8 Mon Sep 17 00:00:00 2001 From: uttam282005 Date: Tue, 29 Jul 2025 13:47:50 +0530 Subject: [PATCH 7/7] test(metrics): removed unnecessary default dimensions --- packages/metrics/tests/unit/creatingMetrics.test.ts | 3 --- packages/metrics/tests/unit/customTimestamp.test.ts | 12 ------------ packages/metrics/tests/unit/dimensions.test.ts | 9 --------- .../metrics/tests/unit/initializeMetrics.test.ts | 3 --- 4 files changed, 27 deletions(-) diff --git a/packages/metrics/tests/unit/creatingMetrics.test.ts b/packages/metrics/tests/unit/creatingMetrics.test.ts index b37d583470..e8b42005f9 100644 --- a/packages/metrics/tests/unit/creatingMetrics.test.ts +++ b/packages/metrics/tests/unit/creatingMetrics.test.ts @@ -182,9 +182,6 @@ describe('Creating metrics', () => { const metrics = new Metrics({ singleMetric: false, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - enviroment: 'test', - }, }); // Act diff --git a/packages/metrics/tests/unit/customTimestamp.test.ts b/packages/metrics/tests/unit/customTimestamp.test.ts index 22915f1d15..01d6589ebf 100644 --- a/packages/metrics/tests/unit/customTimestamp.test.ts +++ b/packages/metrics/tests/unit/customTimestamp.test.ts @@ -50,9 +50,6 @@ describe('Setting custom timestamp', () => { // Prepare const metrics = new Metrics({ singleMetric: true, - defaultDimensions: { - environment: 'test', - }, }); // Act @@ -70,9 +67,6 @@ describe('Setting custom timestamp', () => { // Prepare const metrics = new Metrics({ singleMetric: true, - defaultDimensions: { - environment: 'test', - }, }); // Act @@ -91,9 +85,6 @@ describe('Setting custom timestamp', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - environment: 'test', - }, }); // Act @@ -121,9 +112,6 @@ describe('Setting custom timestamp', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - environment: 'test', - }, }); // Act diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts index 6f7c030a9a..d9d39f4715 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -462,9 +462,6 @@ describe('Working with dimensions', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - enviroment: 'test', - }, }); // Act & Assess @@ -501,9 +498,6 @@ describe('Working with dimensions', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - enviroment: 'test', - }, }); // Act & Assess @@ -611,9 +605,6 @@ describe('Working with dimensions', () => { const metrics = new Metrics({ singleMetric: true, namespace: DEFAULT_NAMESPACE, - defaultDimensions: { - enviroment: 'test', - }, }); // Act diff --git a/packages/metrics/tests/unit/initializeMetrics.test.ts b/packages/metrics/tests/unit/initializeMetrics.test.ts index 1bba8405cf..7bb1ab5c72 100644 --- a/packages/metrics/tests/unit/initializeMetrics.test.ts +++ b/packages/metrics/tests/unit/initializeMetrics.test.ts @@ -67,9 +67,6 @@ describe('Initialize Metrics', () => { // Prepare const metrics = new Metrics({ singleMetric: true, - defaultDimensions: { - environment: 'test', - }, }); // Act