@aws-cdk/cloudwatch: bugfixes, small changes changes and workaround

- bugfix: label/color were ignored when passed directly to the Metric
  constructor.
- BREAKING CHANGE: ChangeMetricProps renamed to MetricCustomization.
- BREAKING CHANGE: removed public helper methods from Metric.
- change: onAlarm()/onOk() now take variadic arguments
- workaround: generate a dashboard name if user did not supply one.
  This fixes #192.

packages/@aws-cdk/assert/lib/assertions/have-resource.ts

@@ -7,7 +7,7 @@ import { StackInspector } from "../inspector";
  * Properties can be:
  *
  * - An object, in which case its properties will be compared to those of the actual resource found
- * - A callablage, in which case it will be treated as a predicate that is applied to the Properties of the found resources.
+ * - A callable, in which case it will be treated as a predicate that is applied to the Properties of the found resources.
  */
 export function haveResource(resourceType: string, properties?: any): Assertion<StackInspector> {
     return new HaveResourceAssertion(resourceType, properties);

packages/@aws-cdk/cloudwatch/lib/alarm.ts

@@ -1,7 +1,7 @@
 import { Arn, Construct, Token } from '@aws-cdk/core';
 import { cloudwatch } from '@aws-cdk/resources';
 import { HorizontalAnnotation } from './graph';
-import { Metric } from './metric';
+import { Dimension, Metric, parseStatistic, Statistic, Unit } from './metric';
 
 /**
  * Properties for Alarms
@@ -158,7 +158,7 @@ export class Alarm extends Construct {
             okActions: new Token(() => this.okActions),
 
             // Metric
-            ...props.metric.toAlarmJson()
+            ...metricJson(props.metric)
         });
 
         this.alarmArn = alarm.alarmArn;
@@ -175,38 +175,38 @@ export class Alarm extends Construct {
      *
      * Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
      */
-    public onAlarm(action: IAlarmAction) {
+    public onAlarm(...actions: IAlarmAction[]) {
         if (this.alarmActions === undefined) {
             this.alarmActions = [];
         }
 
-        this.alarmActions.push(action.alarmActionArn);
+        this.alarmActions.push(...actions.map(a => a.alarmActionArn));
     }
 
     /**
      * Trigger this action if there is insufficient data to evaluate the alarm
      *
      * Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
      */
-    public onInsufficientData(action: IAlarmAction) {
+    public onInsufficientData(...actions: IAlarmAction[]) {
         if (this.insufficientDataActions === undefined) {
             this.insufficientDataActions = [];
         }
 
-        this.insufficientDataActions.push(action.alarmActionArn);
+        this.insufficientDataActions.push(...actions.map(a => a.alarmActionArn));
     }
 
     /**
      * Trigger this action if the alarm returns from breaching state into ok state
      *
      * Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
      */
-    public onOk(action: IAlarmAction) {
+    public onOk(...actions: IAlarmAction[]) {
         if (this.okActions === undefined) {
             this.okActions = [];
         }
 
-        this.okActions.push(action.alarmActionArn);
+        this.okActions.push(...actions.map(a => a.alarmActionArn));
     }
 
     /**
@@ -256,4 +256,63 @@ export interface IAlarmAction {
      * Return the ARN that should be used for a CloudWatch Alarm action
      */
     readonly alarmActionArn: Arn;
-}
+}
+
+/**
+ * Return the JSON structure which represents the given metric in an alarm.
+ */
+function metricJson(metric: Metric): AlarmMetricJson {
+    const stat = parseStatistic(metric.statistic);
+
+    const dims = metric.dimensionsAsList();
+
+    return {
+        dimensions: dims.length > 0 ? dims : undefined,
+        namespace: metric.namespace,
+        metricName: metric.metricName,
+        period: metric.periodSec,
+        statistic: stat.type === 'simple' ? stat.statistic : undefined,
+        extendedStatistic: stat.type === 'percentile' ? 'p' + stat.percentile : undefined,
+        unit: metric.unit
+    };
+}
+
+/**
+ * Properties used to construct the Metric identifying part of an Alarm
+ */
+export interface AlarmMetricJson {
+    /**
+     * The dimensions to apply to the alarm
+     */
+    dimensions?: Dimension[];
+
+    /**
+     * Namespace of the metric
+     */
+    namespace: string;
+
+    /**
+     * Name of the metric
+     */
+    metricName: string;
+
+    /**
+     * How many seconds to aggregate over
+     */
+    period: number;
+
+    /**
+     * Simple aggregation function to use
+     */
+    statistic?: Statistic;
+
+    /**
+     * Percentile aggregation function to use
+     */
+    extendedStatistic?: string;
+
+    /**
+     * The unit of the alarm
+     */
+    unit?: Unit;
+}

packages/@aws-cdk/cloudwatch/lib/dashboard.ts

@@ -1,4 +1,4 @@
-import { Construct, Token, tokenAwareJsonify } from "@aws-cdk/core";
+import { Construct, Stack, Token, tokenAwareJsonify } from "@aws-cdk/core";
 import { cloudwatch } from "@aws-cdk/resources";
 import { Column, Row } from "./layout";
 import { IWidget } from "./widget";
@@ -17,12 +17,19 @@ export interface DashboardProps {
  */
 export class Dashboard extends Construct {
     private readonly rows: IWidget[] = [];
+    private readonly dashboard: cloudwatch.DashboardResource;
 
     constructor(parent: Construct, name: string, props?: DashboardProps) {
         super(parent, name);
 
-        new cloudwatch.DashboardResource(this, 'Resource', {
-            dashboardName: props && props.dashboardName,
+        // WORKAROUND -- Dashboard cannot be updated if the DashboardName is missing.
+        // This is a bug in CloudFormation, but we don't want CDK users to have a bad
+        // experience. We'll generate a name here if you did not supply one.
+        // See: https://github.com/awslabs/aws-cdk/issues/213
+        const dashboardName = (props && props.dashboardName) || new Token(() => this.generateDashboardName());
+
+        this.dashboard = new cloudwatch.DashboardResource(this, 'Resource', {
+            dashboardName,
             dashboardBody: new Token(() => {
                 const column = new Column(...this.rows);
                 column.position(0, 0);
@@ -48,4 +55,13 @@ export class Dashboard extends Construct {
         const w = widgets.length > 1 ? new Row(...widgets) : widgets[0];
         this.rows.push(w);
     }
+
+    /**
+     * Generate a unique dashboard name in case the user didn't supply one
+     */
+    private generateDashboardName(): string {
+        // Combination of stack name and LogicalID, which are guaranteed to be unique.
+        const stack = Stack.find(this);
+        return stack.name + '-' + this.dashboard.logicalId;
+    }
 }

packages/@aws-cdk/cloudwatch/lib/graph.ts

@@ -1,6 +1,6 @@
 import { AwsRegion, Token } from "@aws-cdk/core";
 import { Alarm } from "./alarm";
-import { Metric } from "./metric";
+import { Metric, parseStatistic } from "./metric";
 import { ConcreteWidget } from "./widget";
 
 /**
@@ -156,8 +156,8 @@ export class GraphWidget extends ConcreteWidget {
                 view: 'timeSeries',
                 title: this.props.title,
                 region: this.props.region || new AwsRegion(),
-                metrics: (this.props.left || []).map(m => m.toGraphJson('left')).concat(
-                         (this.props.right || []).map(m => m.toGraphJson('right'))),
+                metrics: (this.props.left || []).map(m => metricJson(m, 'left')).concat(
+                         (this.props.right || []).map(m => metricJson(m, 'right'))),
                 annotations: {
                     horizontal: (this.props.leftAnnotations || []).map(mapAnnotation('left')).concat(
                                 (this.props.rightAnnotations || []).map(mapAnnotation('right')))
@@ -203,7 +203,7 @@ export class SingleValueWidget extends ConcreteWidget {
                 view: 'singleValue',
                 title: this.props.title,
                 region: this.props.region || new AwsRegion(),
-                metrics: this.props.metrics.map(m => m.toGraphJson('left'))
+                metrics: this.props.metrics.map(m => metricJson(m, 'left'))
             }
         }];
     }
@@ -287,4 +287,34 @@ function mapAnnotation(yAxis: string): ((x: HorizontalAnnotation) => any) {
     return (a: HorizontalAnnotation) => {
         return { ...a, yAxis };
     };
+}
+
+/**
+ * Return the JSON structure which represents this metric in a graph
+ *
+ * This will be called by GraphWidget, no need for clients to call this.
+ */
+function metricJson(metric: Metric, yAxis: string): any[] {
+    // Namespace and metric Name
+    const ret: any[] = [
+        metric.namespace,
+        metric.metricName,
+    ];
+
+    // Dimensions
+    for (const dim of metric.dimensionsAsList()) {
+        ret.push(dim.name, dim.value);
+    }
+
+    // Options
+    const stat = parseStatistic(metric.statistic);
+    ret.push({
+        yAxis,
+        label: metric.label,
+        color: metric.color,
+        period: metric.periodSec,
+        stat: stat.type === 'simple' ? stat.statistic : 'p' + stat.percentile.toString(),
+    });
+
+    return ret;
 }