improv(metrics): emit warning when default dimensions are overwritten (#4287)

Author: uttam282005

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);
   }
 
   /**
@@ -293,38 +295,16 @@ class Metrics extends Utility implements MetricsInterface {
    * @param dimensions - An object with key-value pairs of dimensions
    */
   public addDimensions(dimensions: Dimensions): void {
-    const newDimensionSet: 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.dimensions, key) ||
-        Object.hasOwn(this.defaultDimensions, key) ||
-        Object.hasOwn(newDimensionSet, key)
-      ) {
-        this.#logger.warn(
-          `Dimension "${key}" has already been added. The previous value will be overwritten.`
-        );
-      }
-      newDimensionSet[key] = value;
-    }
-
+    const newDimensions = this.#sanitizeDimensions(dimensions);
     const currentCount = this.getCurrentDimensionsCount();
-    const newSetCount = Object.keys(newDimensionSet).length;
+    const newSetCount = Object.keys(newDimensions).length;
     if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) {
       throw new RangeError(
         `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
       );
     }
 
-    this.dimensionSets.push(newDimensionSet);
+    this.dimensionSets.push(newDimensions);
   }
 
   /**
@@ -432,12 +412,6 @@ class Metrics extends Utility implements MetricsInterface {
   public captureColdStartMetric(functionName?: string): void {
     if (!this.getColdStart()) return;
     const singleMetric = this.singleMetric();
-
-    if (this.defaultDimensions.service) {
-      singleMetric.setDefaultDimensions({
-        service: this.defaultDimensions.service,
-      });
-    }
     const value = this.functionName?.trim() ?? functionName?.trim();
     if (value && value.length > 0) {
       singleMetric.addDimension('function_name', value);
@@ -821,15 +795,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 {
-    const targetDimensions = {
+  public setDefaultDimensions(dimensions: Dimensions): void {
+    const newDimensions = this.#sanitizeDimensions(dimensions);
+    const currentCount = Object.keys(this.defaultDimensions).length;
+    const newSetCount = Object.keys(newDimensions).length;
+    if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) {
+      throw new RangeError(
+        `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
+      );
+    }
+
+    this.defaultDimensions = {
       ...this.defaultDimensions,
-      ...dimensions,
+      ...newDimensions,
     };
-    if (MAX_DIMENSION_COUNT <= Object.keys(targetDimensions).length) {
-      throw new Error('Max dimension count hit');
-    }
-    this.defaultDimensions = targetDimensions;
   }
 
   /**
@@ -886,7 +865,6 @@ class Metrics extends Utility implements MetricsInterface {
   public singleMetric(): Metrics {
     return new Metrics({
       namespace: this.namespace,
-      serviceName: this.dimensions.service,
       defaultDimensions: this.defaultDimensions,
       singleMetric: true,
       logger: this.#logger,
@@ -1056,33 +1034,33 @@ class Metrics extends Utility implements MetricsInterface {
       functionName,
     } = options;
 
-    this.setEnvConfig();
-    this.setConsole();
     this.setCustomConfigService(customConfigService);
     this.setDisabled();
     this.setNamespace(namespace);
-    this.setService(serviceName);
-    this.setDefaultDimensions(defaultDimensions);
+    const resolvedServiceName = this.#resolveServiceName(serviceName);
+    this.setDefaultDimensions(
+      defaultDimensions
+        ? { service: resolvedServiceName, ...defaultDimensions }
+        : { service: resolvedServiceName }
+    );
     this.setFunctionNameForColdStartMetric(functionName);
     this.isSingleMetric = singleMetric || false;
 
     return this;
   }
 
   /**
-   * Set the service to be used.
+   * Set the service dimension that will be included in the metrics.
    *
    * @param service - The service to be used
    */
-  private setService(service: string | undefined): void {
-    const targetService =
+  #resolveServiceName(service?: string): string {
+    return (
       service ||
       this.getCustomConfigService()?.getServiceName() ||
       this.#envConfig.serviceName ||
-      this.defaultServiceName;
-    if (targetService.length > 0) {
-      this.setDefaultDimensions({ service: targetService });
-    }
+      this.defaultServiceName
+    );
   }
 
   /**
@@ -1189,6 +1167,38 @@ class Metrics extends Utility implements MetricsInterface {
      **/
     return 0;
   }
+
+  /**
+   * Sanitizes the dimensions by removing invalid entries and skipping duplicates.
+   *
+   * @param dimensions - The dimensions to sanitize.
+   */
+  #sanitizeDimensions(dimensions: Dimensions): Dimensions {
+    const newDimensions: 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.dimensions, key) ||
+        Object.hasOwn(this.defaultDimensions, key) ||
+        Object.hasOwn(newDimensions, key)
+      ) {
+        this.#logger.warn(
+          `Dimension "${key}" has already been added. The previous value will be overwritten.`
+        );
+      }
+      newDimensions[key] = value;
+    }
+
+    return newDimensions;
+  }
 }
 
 export { Metrics };

packages/metrics/tests/unit/dimensions.test.ts

@@ -358,7 +358,7 @@ describe('Working with dimensions', () => {
 
     // Assess
     expect(() => metrics.setDefaultDimensions({ extra: 'test' })).toThrowError(
-      'Max dimension count hit'
+      'The number of metric dimensions must be lower than 29'
     );
   });
 
@@ -552,4 +552,62 @@ 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.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();
+
+      // Assess
+      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 })
+      );
+    }
+  );
 });