From ec8b0cd6f5932520c1628c9b4f8c595643c1a8a4 Mon Sep 17 00:00:00 2001 From: Stefano Vozza Date: Tue, 29 Jul 2025 12:59:50 +0100 Subject: [PATCH] =?UTF-8?q?Revert=20"fix(metrics):=20emit=20warning=20when?= =?UTF-8?q?=20default=20dimensions=20are=20overwritten=20(#=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit a933a6af6135b79b855353bbaf265ab0a3be60da. --- packages/metrics/src/Metrics.ts | 40 +------ .../tests/unit/customTimestamp.test.ts | 8 +- .../metrics/tests/unit/dimensions.test.ts | 100 ------------------ .../tests/unit/initializeMetrics.test.ts | 4 +- 4 files changed, 8 insertions(+), 144 deletions(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index f34c6a1597..a309186f0e 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -1,9 +1,7 @@ import { Console } from 'node:console'; import { isIntegerNumber, - isNullOrUndefined, isNumber, - isRecord, isString, isStringUndefinedNullEmpty, Utility, @@ -242,10 +240,8 @@ class Metrics extends Utility implements MetricsInterface { super(); this.dimensions = {}; - this.setEnvConfig(); - this.setConsole(); - this.#logger = options.logger || this.console; this.setOptions(options); + this.#logger = options.logger || this.console; } /** @@ -828,41 +824,13 @@ 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, - ...cleanedDimensions, + ...dimensions, }; - - if (Object.keys(targetDimensions).length >= MAX_DIMENSION_COUNT) { + if (MAX_DIMENSION_COUNT <= Object.keys(targetDimensions).length) { throw new Error('Max dimension count hit'); } - this.defaultDimensions = targetDimensions; } @@ -1090,6 +1058,8 @@ 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 01d6589ebf..61e3ffa221 100644 --- a/packages/metrics/tests/unit/customTimestamp.test.ts +++ b/packages/metrics/tests/unit/customTimestamp.test.ts @@ -48,9 +48,7 @@ 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); @@ -65,9 +63,7 @@ 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 d9d39f4715..1a874ec024 100644 --- a/packages/metrics/tests/unit/dimensions.test.ts +++ b/packages/metrics/tests/unit/dimensions.test.ts @@ -552,104 +552,4 @@ 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 7bb1ab5c72..b1193e99da 100644 --- a/packages/metrics/tests/unit/initializeMetrics.test.ts +++ b/packages/metrics/tests/unit/initializeMetrics.test.ts @@ -65,9 +65,7 @@ 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);