fix: addDimensions() now creates a new dimension set (fixes #3777)

Author: matteofigus

packages/metrics/src/Metrics.ts

@@ -154,6 +154,12 @@ class Metrics extends Utility implements MetricsInterface {
    */
   private dimensions: Dimensions = {};
 
+  /**
+   * Additional dimension sets for the current metrics context
+   * @default []
+   */
+  private dimensionSets: Dimensions[] = [];
+
   /**
    * Service for accessing environment variables
    */
@@ -267,9 +273,15 @@ class Metrics extends Utility implements MetricsInterface {
    * @param dimensions - An object with key-value pairs of dimensions
    */
   public addDimensions(dimensions: Dimensions): void {
-    for (const [name, value] of Object.entries(dimensions)) {
-      this.addDimension(name, value);
+    if (
+      Object.keys(dimensions).length + this.getCurrentDimensionsCount() >=
+      MAX_DIMENSION_COUNT
+    ) {
+      throw new RangeError(
+        `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
+      );
     }
+    this.dimensionSets.push(dimensions);
   }
 
   /**
@@ -447,6 +459,7 @@ class Metrics extends Utility implements MetricsInterface {
    */
   public clearDimensions(): void {
     this.dimensions = {};
+    this.dimensionSets = [];
   }
 
   /**
@@ -692,26 +705,49 @@ class Metrics extends Utility implements MetricsInterface {
       {}
     );
 
-    const dimensionNames = [
-      ...new Set([
-        ...Object.keys(this.defaultDimensions),
-        ...Object.keys(this.dimensions),
-      ]),
-    ];
+    const dimensionNames = [];
+
+    if (Object.keys(this.dimensions).length > 0) {
+      dimensionNames.push([
+        ...new Set([
+          ...Object.keys(this.defaultDimensions),
+          ...Object.keys(this.dimensions),
+        ]),
+      ]);
+    }
+
+    for (const dimensionSet of this.dimensionSets) {
+      dimensionNames.push([
+        ...new Set([
+          ...Object.keys(this.defaultDimensions),
+          ...Object.keys(dimensionSet),
+        ]),
+      ]);
+    }
+
+    if (
+      dimensionNames.length === 0 &&
+      Object.keys(this.defaultDimensions).length > 0
+    ) {
+      dimensionNames.push([
+        ...new Set([...Object.keys(this.defaultDimensions)]),
+      ]);
+    }
 
     return {
       _aws: {
         Timestamp: this.#timestamp ?? new Date().getTime(),
         CloudWatchMetrics: [
           {
             Namespace: this.namespace || DEFAULT_NAMESPACE,
-            Dimensions: [dimensionNames],
+            Dimensions: dimensionNames as [string[]],
             Metrics: metricDefinitions,
           },
         ],
       },
       ...this.defaultDimensions,
       ...this.dimensions,
+      ...this.dimensionSets.reduce((acc, dims) => Object.assign(acc, dims), {}),
       ...metricValues,
       ...this.metadata,
     };

packages/metrics/src/types/Metrics.ts

@@ -190,8 +190,8 @@ interface MetricsInterface {
    * included in all metrics, use the {@link MetricsInterface.setDefaultDimensions | `setDefaultDimensions()`} method.
    *
    * @param dimensions - An object with key-value pairs of dimensions
+   * @returns The dimensions object that was passed in
    */
-  addDimensions(dimensions: Dimensions): void;
   /**
    * A metadata key-value pair to be included with metrics.
    *

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

@@ -14,6 +14,42 @@ describe('Working with dimensions', () => {
     vi.clearAllMocks();
   });
 
+  it('creates a new dimension set', () => {
+    // Prepare
+    const metrics = new Metrics({
+      namespace: DEFAULT_NAMESPACE,
+    });
+
+    // Act
+    metrics.addDimension('environment', 'prod');
+
+    metrics.addDimensions({
+      dimension1: '1',
+      dimension2: '2',
+    });
+
+    metrics.addMetric('foo', MetricUnit.Count, 1);
+    metrics.publishStoredMetrics();
+
+    // Assess
+    expect(console.log).toHaveEmittedEMFWith(
+      expect.objectContaining({
+        environment: 'prod',
+        dimension1: '1',
+        dimension2: '2',
+        foo: 1,
+      })
+    );
+    expect(console.log).toHaveEmittedMetricWith(
+      expect.objectContaining({
+        Dimensions: [
+          ['service', 'environment'],
+          ['service', 'dimension1', 'dimension2'],
+        ],
+      })
+    );
+  });
+
   it('adds default dimensions to the metric via constructor', () => {
     // Prepare
     const metrics = new Metrics({
@@ -284,6 +320,88 @@ describe('Working with dimensions', () => {
     );
   });
 
+  it('throws when adding dimension sets would exceed the limit', () => {
+    // Prepare
+    const metrics = new Metrics({
+      singleMetric: true,
+      defaultDimensions: {
+        environment: 'test',
+      },
+    });
+
+    // Act & Assess
+    let i = 1;
+    // We start with 2 dimensions because the default dimension & service name are already added
+    for (i = 2; i < MAX_DIMENSION_COUNT - 2; i++) {
+      metrics.addDimension(`dimension-${i}`, 'test');
+    }
+
+    // Adding a dimension set with 3 dimensions would exceed the limit
+    expect(() =>
+      metrics.addDimensions({
+        'dimension-extra-1': 'test',
+        'dimension-extra-2': 'test',
+        'dimension-extra-3': 'test',
+      })
+    ).toThrowError(
+      `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
+    );
+  });
+
+  it('handles dimension overrides across multiple dimension sets', () => {
+    // Prepare
+    const metrics = new Metrics({
+      namespace: DEFAULT_NAMESPACE,
+    });
+
+    // Act
+    // First add a single dimension
+    metrics.addDimension('d', '3');
+
+    // First dimension set
+    metrics.addDimensions({
+      a: '1',
+      b: '2',
+    });
+
+    // Second dimension set with some overlapping keys
+    metrics.addDimensions({
+      a: '3',
+      c: '5',
+      d: '8',
+    });
+
+    // Third dimension set with more overlapping keys
+    metrics.addDimensions({
+      b: '5',
+      d: '1',
+    });
+
+    metrics.addMetric('foo', MetricUnit.Count, 1);
+    metrics.publishStoredMetrics();
+
+    // Assess
+    expect(console.log).toHaveEmittedEMFWith(
+      expect.objectContaining({
+        a: '3', // Last value from second set
+        b: '5', // Last value from third set
+        c: '5', // Only value from second set
+        d: '1', // Last value from third set (overriding both the initial d:3 and second set's d:8)
+        foo: 1,
+      })
+    );
+    expect(console.log).toHaveEmittedMetricWith(
+      expect.objectContaining({
+        Dimensions: [
+          ['service', 'd'],
+          ['service', 'a', 'b'],
+          ['service', 'a', 'c', 'd'],
+          ['service', 'b', 'd'],
+        ],
+      })
+    );
+  });
+
   it.each([
     { value: undefined, name: 'undefined' },
     { value: null, name: 'null' },