fix(metrics): skip empty string dimension values (#3319)

Author: arnabrahman

packages/metrics/src/Metrics.ts

@@ -218,6 +218,7 @@ class Metrics extends Utility implements MetricsInterface {
    * Add a dimension to metrics.
    *
    * A dimension is a key-value pair that is used to group metrics, and it is included in all metrics emitted after it is added.
+   * Invalid dimension values are skipped and a warning is logged.
    *
    * When calling the {@link Metrics.publishStoredMetrics | `publishStoredMetrics()`} method, the dimensions are cleared. This type of
    * dimension is useful when you want to add request-specific dimensions to your metrics. If you want to add dimensions that are
@@ -227,6 +228,12 @@ class Metrics extends Utility implements MetricsInterface {
    * @param value - The value of the dimension
    */
   public addDimension(name: string, value: string): void {
+    if (!value) {
+      this.#logger.warn(
+        `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
+      );
+      return;
+    }
     if (MAX_DIMENSION_COUNT <= this.getCurrentDimensionsCount()) {
       throw new RangeError(
         `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
@@ -239,6 +246,7 @@ class Metrics extends Utility implements MetricsInterface {
    * Add multiple dimensions to the metrics.
    *
    * This method is useful when you want to add multiple dimensions to the metrics at once.
+   * Invalid dimension values are skipped and a warning is logged.
    *
    * When calling the {@link Metrics.publishStoredMetrics | `publishStoredMetrics()`} method, the dimensions are cleared. This type of
    * dimension is useful when you want to add request-specific dimensions to your metrics. If you want to add dimensions that are
@@ -249,7 +257,14 @@ class Metrics extends Utility implements MetricsInterface {
   public addDimensions(dimensions: Dimensions): void {
     const newDimensions = { ...this.dimensions };
     for (const dimensionName of Object.keys(dimensions)) {
-      newDimensions[dimensionName] = dimensions[dimensionName];
+      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(

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

@@ -431,6 +431,45 @@ describe('Class: Metrics', () => {
         `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
       );
     });
+
+    describe('invalid values should not be added as dimensions', () => {
+      const testCases = [
+        { value: undefined as unknown as string, description: 'undefined' },
+        { value: null as unknown as string, description: 'null' },
+        { value: '', description: 'empty string' },
+      ];
+
+      for (const { value, description } of testCases) {
+        it(`it should not add dimension with ${description} value and log a warning`, () => {
+          // Prepare
+          const customLogger = {
+            warn: jest.fn(),
+            debug: jest.fn(),
+            error: jest.fn(),
+            info: jest.fn(),
+          };
+          const metrics: Metrics = new Metrics({
+            namespace: TEST_NAMESPACE,
+            logger: customLogger,
+          });
+          const consoleWarnSpy = jest.spyOn(customLogger, 'warn');
+          const testDimensionName = 'test-dimension';
+
+          // Act
+          metrics.addDimension(testDimensionName, value);
+
+          // Assess
+          expect(consoleWarnSpy).toHaveBeenCalledWith(
+            `The dimension ${testDimensionName} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
+          );
+          expect(metrics).toEqual(
+            expect.objectContaining({
+              dimensions: {},
+            })
+          );
+        });
+      }
+    });
   });
 
   describe('Method: addDimensions', () => {
@@ -520,6 +559,53 @@ describe('Class: Metrics', () => {
         `Unable to add 1 dimensions: the number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
       );
     });
+
+    describe('invalid values should not be added as dimensions', () => {
+      const testCases = [
+        { value: undefined as unknown as string, description: 'undefined' },
+        { value: null as unknown as string, description: 'null' },
+        { value: '', description: 'empty string' },
+      ];
+
+      for (const { value, description } of testCases) {
+        it(`it should not add dimension with ${description} value and log a warning`, () => {
+          // Prepare
+          const customLogger = {
+            warn: jest.fn(),
+            debug: jest.fn(),
+            error: jest.fn(),
+            info: jest.fn(),
+          };
+          const metrics: Metrics = new Metrics({
+            namespace: TEST_NAMESPACE,
+            logger: customLogger,
+          });
+          const consoleWarnSpy = jest.spyOn(customLogger, 'warn');
+          const dimensionsToBeAdded: LooseObject = {
+            'test-dimension-1': 'test-value-1',
+            'test-dimension-2': 'test-value-2',
+          };
+          const testDimensionName = 'test-dimension';
+
+          // Act
+          metrics.addDimensions(dimensionsToBeAdded);
+          metrics.addDimensions({ [testDimensionName]: value });
+
+          // Assess
+          expect(consoleWarnSpy).toHaveBeenCalledWith(
+            `The dimension ${testDimensionName} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
+          );
+          expect(metrics).toEqual(
+            expect.objectContaining({
+              dimensions: {
+                'test-dimension-1': 'test-value-1',
+                'test-dimension-2': 'test-value-2',
+              },
+            })
+          );
+        });
+      }
+    });
   });
 
   describe('Method: addMetadata', () => {