feat(metrics): warn when overwriting dimension (#3352)

aws-powertools · Nov 25, 2024 · 12f3e44 · 12f3e44
1 parent a403567
commit 12f3e44
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 25 deletions.
diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts
@@ -239,6 +239,14 @@ class Metrics extends Utility implements MetricsInterface {
         `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
       );
     }
+    if (
+      Object.hasOwn(this.dimensions, name) ||
+      Object.hasOwn(this.defaultDimensions, name)
+    ) {
+      this.#logger.warn(
+        `Dimension "${name}" has already been added. The previous value will be overwritten.`
+      );
+    }
     this.dimensions[name] = value;
   }
 
@@ -255,25 +263,9 @@ class Metrics extends Utility implements MetricsInterface {
    * @param dimensions - An object with key-value pairs of dimensions
    */
   public addDimensions(dimensions: Dimensions): void {
-    const newDimensions = { ...this.dimensions };
-    for (const dimensionName of Object.keys(dimensions)) {
-      const value = dimensions[dimensionName];
-      if (value) {
-        newDimensions[dimensionName] = value;
-      } else {
-        this.#logger.warn(
-          `The dimension ${dimensionName} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
-        );
-      }
-    }
-    if (Object.keys(newDimensions).length > MAX_DIMENSION_COUNT) {
-      throw new RangeError(
-        `Unable to add ${
-          Object.keys(dimensions).length
-        } dimensions: the number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
-      );
+    for (const [name, value] of Object.entries(dimensions)) {
+      this.addDimension(name, value);
     }
-    this.dimensions = newDimensions;
   }
 
   /**

diff --git a/packages/metrics/tests/unit/Metrics.test.ts b/packages/metrics/tests/unit/Metrics.test.ts
@@ -3,7 +3,10 @@
  *
  * @group unit/metrics/class
  */
-import type { LambdaInterface } from '@aws-lambda-powertools/commons/types';
+import type {
+  GenericLogger,
+  LambdaInterface,
+} from '@aws-lambda-powertools/commons/types';
 import context from '@aws-lambda-powertools/testing-utils/context';
 import type { Context, Handler } from 'aws-lambda';
 import { EnvironmentVariablesService } from '../../src/config/EnvironmentVariablesService.js';
@@ -350,7 +353,13 @@ describe('Class: Metrics', () => {
 
     test('it should update existing dimension value if same dimension is added again', () => {
       // Prepare
-      const metrics: Metrics = new Metrics({ namespace: TEST_NAMESPACE });
+      const logger = {
+        warn: jest.fn(),
+      } as unknown as GenericLogger;
+      const metrics: Metrics = new Metrics({
+        namespace: TEST_NAMESPACE,
+        logger,
+      });
       const dimensionName = 'test-dimension';
 
       // Act
@@ -365,6 +374,9 @@ describe('Class: Metrics', () => {
           },
         })
       );
+      expect(logger.warn).toHaveBeenCalledWith(
+        `Dimension "test-dimension" has already been added. The previous value will be overwritten.`
+      );
     });
 
     test('it should throw error if the number of dimensions exceeds the maximum allowed', () => {
@@ -521,7 +533,7 @@ describe('Class: Metrics', () => {
       const dimensionName = 'test-dimension';
       const dimensionValue = 'test-value';
       const dimensionsToBeAdded: LooseObject = {};
-      for (let i = 0; i < MAX_DIMENSION_COUNT; i++) {
+      for (let i = 0; i < MAX_DIMENSION_COUNT - 1; i++) {
         dimensionsToBeAdded[`${dimensionName}-${i}`] = `${dimensionValue}-${i}`;
       }
 
@@ -531,7 +543,7 @@ describe('Class: Metrics', () => {
       ).not.toThrowError();
       // biome-ignore  lint/complexity/useLiteralKeys: This needs to be accessed with literal key for testing
       expect(Object.keys(metrics['dimensions']).length).toBe(
-        MAX_DIMENSION_COUNT
+        MAX_DIMENSION_COUNT - 1 // Starts from 1 because the service dimension is already added by default
       );
     });
 
@@ -541,22 +553,22 @@ describe('Class: Metrics', () => {
       const dimensionName = 'test-dimension';
       const dimensionValue = 'test-value';
       const dimensionsToBeAdded: LooseObject = {};
-      for (let i = 0; i < MAX_DIMENSION_COUNT; i++) {
+      for (let i = 0; i < MAX_DIMENSION_COUNT - 1; i++) {
         dimensionsToBeAdded[`${dimensionName}-${i}`] = `${dimensionValue}-${i}`;
       }
 
       // Act & Assess
       metrics.addDimensions(dimensionsToBeAdded);
       // biome-ignore  lint/complexity/useLiteralKeys: This needs to be accessed with literal key for testing
       expect(Object.keys(metrics['dimensions']).length).toBe(
-        MAX_DIMENSION_COUNT
+        MAX_DIMENSION_COUNT - 1 // Starts from 1 because the service dimension is already added by default
       );
       expect(() =>
         metrics.addDimensions({
           'another-dimension': 'another-dimension-value',
         })
       ).toThrowError(
-        `Unable to add 1 dimensions: the number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
+        `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
       );
     });