fix(cloudwatch): unhelpful error when reusing metric IDs (#6892)

Reuse of the same metric ID was already checked upon construction of the `MathExpression` instance and so we thought this error couldn't occur. However, reuse can also occur (and is also prohibited) between different `MathExpression`s, even on different axes. Make the error message describe the situation in a way that is actionable.
aws · Mar 20, 2020 · 60253a3 · 60253a3
1 parent f50b876
commit 60253a3
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 4 deletions.
diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/graph.ts b/packages/@aws-cdk/aws-cloudwatch/lib/graph.ts
@@ -168,8 +168,11 @@ export class GraphWidget extends ConcreteWidget {
   }
 
   public toJson(): any[] {
-    const horizontalAnnoations =  (this.props.leftAnnotations || []).map(mapAnnotation('left')).concat(
-      (this.props.rightAnnotations || []).map(mapAnnotation('right')));
+    const horizontalAnnotations = [
+      ...(this.props.leftAnnotations || []).map(mapAnnotation('left')),
+      ...(this.props.rightAnnotations || []).map(mapAnnotation('right')),
+    ];
+
     const metrics = allMetricsGraphJson(this.props.left || [], this.props.right || []);
     return [{
       type: 'metric',
@@ -183,7 +186,7 @@ export class GraphWidget extends ConcreteWidget {
         region: this.props.region || cdk.Aws.REGION,
         stacked: this.props.stacked,
         metrics: metrics.length > 0 ? metrics : undefined,
-        annotations: horizontalAnnoations.length > 0 ? { horizontal: horizontalAnnoations } : undefined,
+        annotations: horizontalAnnotations.length > 0 ? { horizontal: horizontalAnnotations } : undefined,
         yAxis: {
           left: this.props.leftYAxis !== undefined ? this.props.leftYAxis : undefined,
           right: this.props.rightYAxis !== undefined ? this.props.rightYAxis : undefined,

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/private/rendering.ts b/packages/@aws-cdk/aws-cloudwatch/lib/private/rendering.ts
@@ -142,7 +142,7 @@ export class MetricSet<A> {
     if (id) {
       existingEntry = this.metricById.get(id);
       if (existingEntry && metricKey(existingEntry.metric) !== key) {
-        throw new Error(`Can't happen, already checked elsewhere`);
+        throw new Error(`Cannot have two different metrics share the same id ('${id}') in one Alarm or Graph. Rename one of them.`);
       }
     }
 

diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metric-math.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metric-math.ts
@@ -268,6 +268,58 @@ export = {
 
       test.done();
     },
+
+    'can reuse the same metric between left and right axes'(test: Test) {
+      // GIVEN
+      const graph = new GraphWidget({
+        left: [
+          new MathExpression({
+            expression: 'a + 1',
+            usingMetrics: { a }
+          })
+        ],
+        right: [
+          new MathExpression({
+            expression: 'a + 2',
+            usingMetrics: { a }
+          })
+        ]
+      });
+
+      // THEN
+      graphMetricsAre(test, graph, [
+        [ { label: 'a + 1', expression: 'a + 1' } ],
+        [ 'Test', 'ACount', { visible: false, id: 'a' } ],
+        [ { label: 'a + 2', expression: 'a + 2', yAxis: 'right' } ]
+      ]);
+
+      test.done();
+    },
+
+    'detect name conflicts between left and right axes'(test: Test) {
+      // GIVEN
+      const graph = new GraphWidget({
+        left: [
+          new MathExpression({
+            expression: 'm1 + 1',
+            usingMetrics: { m1: a }
+          })
+        ],
+        right: [
+          new MathExpression({
+            expression: 'm1 + 1',
+            usingMetrics: { m1: b }
+          })
+        ]
+      });
+
+      // THEN
+      test.throws(() => {
+        graphMetricsAre(test, graph, []);
+      }, /Cannot have two different metrics share the same id \('m1'\)/);
+
+      test.done();
+    },
   },
 
   'in alarms': {