fix(metrics): emit warning when default dimensions are overwritten (#4222)

Co-authored-by: Swopnil Dangol <[email protected]>

Author: uttam282005

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

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

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

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