fix(metrics): revert changes when raise warning with overridden default dimensions (#4226)

Author: svozza

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);

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);

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',
-      })
-    );
-  });
 });

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);