fix(cloudwatch): Alarm can't use MathExpression without submetrics

`MathExpression`s without submetrics (like for example, `INSIGHT_RULE_METRIC`) will end up without a `period`, which is not allowed. Add a `period` field to the schema (it's not in the upstream schema yet), and render it out when submetrics are missing. Fixes #7155.
aws · Apr 30, 2020 · b59aed0 · b59aed0
1 parent aa9dd83
commit b59aed0
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 2 deletions.
diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
@@ -3,7 +3,7 @@ import { IAlarmAction } from './alarm-action';
 import { CfnAlarm, CfnAlarmProps } from './cloudwatch.generated';
 import { HorizontalAnnotation } from './graph';
 import { CreateAlarmOptions } from './metric';
-import { IMetric, MetricStatConfig } from './metric-types';
+import { IMetric, MetricExpressionConfig, MetricStatConfig } from './metric-types';
 import { dispatchMetric, metricPeriod } from './private/metric-util';
 import { dropUndefined } from './private/object';
 import { MetricSet } from './private/rendering';
@@ -326,6 +326,7 @@ export class Alarm extends Resource implements IAlarm {
                 expression: expr.expression,
                 id: entry.id || uniqueMetricId(),
                 label: conf.renderingProperties?.label,
+                period: mathExprHasSubmetrics(expr) ? undefined : expr.period,
                 returnData: entry.tag ? undefined : false, // Tag stores "primary" attribute, default is "true"
               };
             },
@@ -388,4 +389,8 @@ function renderIfExtendedStatistic(statistic?: string): string | undefined {
   return undefined;
 }
 
+function mathExprHasSubmetrics(expr: MetricExpressionConfig) {
+  return Object.keys(expr.usingMetrics).length > 0;
+}
+
 type Writeable<T> = { -readonly [P in keyof T]: T[P] };
diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts
@@ -315,6 +315,11 @@ export interface MetricExpressionConfig {
    * Metrics used in the math expression
    */
   readonly usingMetrics: Record<string, IMetric>;
+
+  /**
+   * How many seconds to aggregate over
+   */
+  readonly period: number;
 }
 
 /**

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
@@ -485,6 +485,7 @@ export class MathExpression implements IMetric {
   public toMetricConfig(): MetricConfig {
     return {
       mathExpression: {
+        period: this.period.toSeconds(),
         expression: this.expression,
         usingMetrics: this.usingMetrics,
       },

diff --git a/packages/@aws-cdk/aws-cloudwatch/package.json b/packages/@aws-cdk/aws-cloudwatch/package.json
@@ -98,7 +98,8 @@
       "props-default-doc:@aws-cdk/aws-cloudwatch.MetricGraphConfig.unit",
       "props-default-doc:@aws-cdk/aws-cloudwatch.MetricRenderingProperties.color",
       "props-default-doc:@aws-cdk/aws-cloudwatch.MetricRenderingProperties.label",
-      "props-default-doc:@aws-cdk/aws-cloudwatch.MetricRenderingProperties.stat"
+      "props-default-doc:@aws-cdk/aws-cloudwatch.MetricRenderingProperties.stat",
+      "duration-prop-type:@aws-cdk/aws-cloudwatch.MetricExpressionConfig.period"
     ]
   },
   "engines": {

diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metric-math.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metric-math.ts
@@ -507,6 +507,28 @@ export = {
       test.done();
     },
 
+    'MathExpression without inner metrics emits its own period'(test: Test) {
+      // WHEN
+      new Alarm(stack, 'Alarm', {
+        threshold: 1, evaluationPeriods: 1,
+        metric: new MathExpression({
+          expression: 'INSIGHT_RULE_METRIC("SomeId", UniqueContributors)',
+          usingMetrics: {},
+        }),
+      });
+
+      // THEN
+      alarmMetricsAre([
+        {
+          Expression: 'INSIGHT_RULE_METRIC("SomeId", UniqueContributors)',
+          Id: 'expr_1',
+          Period: 300,
+        },
+      ]);
+
+      test.done();
+    },
+
     'annotation for a mathexpression alarm is calculated based upon constituent metrics'(test: Test) {
       // GIVEN
       const alarm = new Alarm(stack, 'Alarm', {

diff --git a/packages/@aws-cdk/cfnspec/spec-source/500_CloudWatch_Alarm_MetricDataQuery_Period_patch.json b/packages/@aws-cdk/cfnspec/spec-source/500_CloudWatch_Alarm_MetricDataQuery_Period_patch.json
@@ -0,0 +1,19 @@
+{
+  "PropertyTypes": {
+    "AWS::CloudWatch::Alarm.MetricDataQuery": {
+      "patch": {
+        "description": "Add Period to AWS::CloudWatch::Alarm.MetricDataQuery (#7155)",
+        "operations": [
+          {
+            "op": "add",
+            "path": "/Properties/Period",
+            "value": {
+              "PrimitiveType": "Integer",
+              "UpdateType": "Mutable"
+            }
+          }
+        ]
+      }
+    }
+  }
+}