@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 MetricCustomizations.
- 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

@@ -158,7 +158,7 @@ export class Alarm extends Construct {
             okActions: new Token(() => this.okActions),
 
             // Metric
-            ...props.metric.toAlarmJson()
+            ...props.metric.alarmInfo()
         });
 
         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));
     }
 
     /**

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

@@ -21,8 +21,13 @@ export class Dashboard extends Construct {
     constructor(parent: Construct, name: string, props?: DashboardProps) {
         super(parent, name);
 
+        // 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.
+        const dashboardName = (props && props.dashboardName) || this.generateDashboardName();
+
         new cloudwatch.DashboardResource(this, 'Resource', {
-            dashboardName: props && props.dashboardName,
+            dashboardName,
             dashboardBody: new Token(() => {
                 const column = new Column(...this.rows);
                 column.position(0, 0);
@@ -48,4 +53,12 @@ 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 {
+        // This will include the Stack name and is hence unique
+        return this.path.replace('/', '-');
+    }
 }

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

@@ -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 => m.graphJson('left')).concat(
+                         (this.props.right || []).map(m => m.graphJson('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 => m.graphJson('left'))
             }
         }];
     }

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

@@ -101,17 +101,22 @@ export class Metric {
         this.metricName = props.metricName;
         this.periodSec = props.periodSec !== undefined ? props.periodSec : 300;
         this.statistic = props.statistic || "Average";
+        this.label = props.label;
+        this.color = props.color;
         this.unit = props.unit;
 
         // Try parsing, this will throw if it's not a valid stat
         parseStatistic(this.statistic);
     }
 
     /**
-     * Return a copy of Metric with properties changed
-     * @param props Re
+     * Return a copy of Metric with properties changed.
+     *
+     * All properties except namespace and metricName can be changed.
+     *
+     * @param props The set of properties to change.
      */
-    public with(props: ChangeMetricProps): Metric {
+    public with(props: MetricCustomizations): Metric {
         return new Metric({
             dimensions: ifUndefined(props.dimensions, this.dimensions),
             namespace: this.namespace,
@@ -149,8 +154,10 @@ export class Metric {
 
     /**
      * Return the JSON structure which represents this metric in an alarm
+     *
+     * This will be called by Alarm, no need for clients to call this.
      */
-    public toAlarmJson(): MetricAlarmJson {
+    public alarmInfo(): AlarmMetricInfo {
         const stat = parseStatistic(this.statistic);
 
         return {
@@ -166,8 +173,10 @@ export class Metric {
 
     /**
      * Return the JSON structure which represents this metric in a graph
+     *
+     * This will be called by GraphWidget, no need for clients to call this.
      */
-    public toGraphJson(yAxis: string): any[] {
+    public graphJson(yAxis: string): any[] {
         // Namespace and metric Name
         const ret: any[] = [
             this.namespace,
@@ -196,7 +205,7 @@ export class Metric {
 /**
  * Properties used to construct the Metric identifying part of an Alarm
  */
-export interface MetricAlarmJson {
+export interface AlarmMetricInfo {
     /**
      * The dimensions to apply to the alarm
      */
@@ -295,7 +304,7 @@ export enum Unit {
 /**
  * Properties of a metric that can be changed
  */
-export interface ChangeMetricProps {
+export interface MetricCustomizations {
     /**
      * Dimensions of the metric
      *

packages/@aws-cdk/cloudwatch/test/integ.alarm-and-dashboard.expected.json

@@ -29,6 +29,7 @@
     "DashCCD7F836": {
       "Type": "AWS::CloudWatch::Dashboard",
       "Properties": {
+        "DashboardName": "aws-cdk-cloudwatch-Dash",
         "DashboardBody": {
           "Fn::Sub": [
             "{\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"# This is my dashboard\"}},{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":6,\"y\":0,\"properties\":{\"markdown\":\"you like?\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":2,\"properties\":{\"view\":\"timeSeries\",\"title\":\"Messages in queue\",\"region\":\"${ref0}\",\"annotations\":{\"alarms\":[\"${ref1}\"]},\"yAxis\":{\"left\":{\"min\":0}}}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":8,\"properties\":{\"view\":\"timeSeries\",\"title\":\"More messages in queue with alarm annotation\",\"region\":\"${ref0}\",\"metrics\":[[\"AWS/SQS\",\"ApproximateNumberOfMessagesVisible\",\"QueueName\",\"${ref2}\",{\"yAxis\":\"left\",\"period\":300,\"stat\":\"Average\"}]],\"annotations\":{\"horizontal\":[{\"label\":\"ApproximateNumberOfMessagesVisible >= 100 for 3 datapoints within 15 minutes\",\"value\":100,\"yAxis\":\"left\"}]},\"yAxis\":{\"left\":{\"min\":0},\"right\":{\"min\":0}}}},{\"type\":\"metric\",\"width\":6,\"height\":3,\"x\":0,\"y\":14,\"properties\":{\"view\":\"singleValue\",\"title\":\"Current messages in queue\",\"region\":\"${ref0}\",\"metrics\":[[\"AWS/SQS\",\"ApproximateNumberOfMessagesVisible\",\"QueueName\",\"${ref2}\",{\"yAxis\":\"left\",\"period\":300,\"stat\":\"Average\"}]]}}]}",

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

@@ -1,5 +1,5 @@
 import { expect, haveResource, isSuperObject } from '@aws-cdk/assert';
-import { Stack } from '@aws-cdk/core';
+import { App, Stack } from '@aws-cdk/core';
 import { Test } from 'nodeunit';
 import { Dashboard, GraphWidget, TextWidget } from '../lib';
 
@@ -94,6 +94,22 @@ export = {
 
         test.done();
     },
+
+    'work around CloudFormation bug'(test: Test) {
+        // GIVEN
+        const app = new App();
+        const stack = new Stack(app, 'MyStack');
+
+        // WHEN
+        new Dashboard(stack, 'MyDashboard');
+
+        // THEN
+        expect(stack).to(haveResource('AWS::CloudWatch::Dashboard', {
+            DashboardName: 'MyStack-MyDashboard'
+        }));
+
+        test.done();
+    }
 };
 
 /**

packages/@aws-cdk/cloudwatch/test/test.graphs.ts

@@ -36,6 +36,31 @@ export = {
         test.done();
     },
 
+    'label and color are respected in constructor'(test: Test) {
+        // WHEN
+        const widget = new GraphWidget({
+            left: [new Metric({ namespace: 'CDK', metricName: 'Test', label: 'MyMetric', color: '000000' }) ],
+        });
+
+        // THEN
+        test.deepEqual(resolve(widget.toJson()), [{
+            type: 'metric',
+            width: 6,
+            height: 6,
+            properties: {
+                view: 'timeSeries',
+                region: { Ref: 'AWS::Region' },
+                metrics: [
+                    ['CDK', 'Test', { yAxis: 'left', period: 300, stat: 'Average', label: 'MyMetric', color: '000000' }],
+                ],
+                annotations: { horizontal: [] },
+                yAxis: { left: { min: 0 }, right: { min: 0 } }
+            }
+        }]);
+
+        test.done();
+    },
+
     'singlevalue widget'(test: Test) {
         // GIVEN
         const metric = new Metric({ namespace: 'CDK', metricName: 'Test' });

packages/@aws-cdk/lambda/lib/lambda-ref.ts

@@ -1,4 +1,4 @@
-import { ChangeMetricProps, Metric } from '@aws-cdk/cloudwatch';
+import { Metric, MetricCustomizations } from '@aws-cdk/cloudwatch';
 import { AccountPrincipal, Arn, Construct, FnSelect, FnSplit, PolicyPrincipal,
          PolicyStatement, resolve, ServicePrincipal, Token } from '@aws-cdk/core';
 import { EventRuleTarget, IEventRuleTarget } from '@aws-cdk/events';
@@ -42,7 +42,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
     /**
      * Return the given named metric for this Lambda
      */
-    public static metricAll(metricName: string, props?: ChangeMetricProps): Metric {
+    public static metricAll(metricName: string, props?: MetricCustomizations): Metric {
         return new Metric({
             namespace: 'AWS/Lambda',
             metricName,
@@ -54,7 +54,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default sum over 5 minutes
      */
-    public static metricAllErrors(props?: ChangeMetricProps): Metric {
+    public static metricAllErrors(props?: MetricCustomizations): Metric {
         return LambdaRef.metricAll('Errors', { statistic: 'sum', ...props });
     }
 
@@ -63,7 +63,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default average over 5 minutes
      */
-    public static metricAllDuration(props?: ChangeMetricProps): Metric {
+    public static metricAllDuration(props?: MetricCustomizations): Metric {
         return LambdaRef.metricAll('Duration', props);
     }
 
@@ -72,7 +72,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default sum over 5 minutes
      */
-    public static metricAllInvocations(props?: ChangeMetricProps): Metric {
+    public static metricAllInvocations(props?: MetricCustomizations): Metric {
         return LambdaRef.metricAll('Invocations', { statistic: 'sum', ...props });
     }
 
@@ -81,7 +81,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default sum over 5 minutes
      */
-    public static metricAllThrottles(props?: ChangeMetricProps): Metric {
+    public static metricAllThrottles(props?: MetricCustomizations): Metric {
         return LambdaRef.metricAll('Throttles', { statistic: 'sum', ...props });
     }
 
@@ -167,7 +167,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
     /**
      * Return the given named metric for this Lambda
      */
-    public metric(metricName: string, props?: ChangeMetricProps): Metric {
+    public metric(metricName: string, props?: MetricCustomizations): Metric {
         return new Metric({
             namespace: 'AWS/Lambda',
             metricName,
@@ -181,7 +181,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default sum over 5 minutes
      */
-    public metricErrors(props?: ChangeMetricProps): Metric {
+    public metricErrors(props?: MetricCustomizations): Metric {
         return this.metric('Errors', { statistic: 'sum', ...props });
     }
 
@@ -190,7 +190,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default average over 5 minutes
      */
-    public metricDuration(props?: ChangeMetricProps): Metric {
+    public metricDuration(props?: MetricCustomizations): Metric {
         return this.metric('Duration', props);
     }
 
@@ -199,7 +199,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default sum over 5 minutes
      */
-    public metricInvocations(props?: ChangeMetricProps): Metric {
+    public metricInvocations(props?: MetricCustomizations): Metric {
         return this.metric('Invocations', { statistic: 'sum', ...props });
     }
 
@@ -208,7 +208,7 @@ export abstract class LambdaRef extends Construct implements IEventRuleTarget {
      *
      * @default sum over 5 minutes
      */
-    public metricThrottles(props?: ChangeMetricProps): Metric {
+    public metricThrottles(props?: MetricCustomizations): Metric {
         return this.metric('Throttles', { statistic: 'sum', ...props });
     }