feat(cloudwatch): make Metric objects region-aware

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

@@ -1,11 +1,13 @@
-import { Construct, IResource, Lazy, Resource, Stack } from '@aws-cdk/core';
+import { Construct, IResource, Lazy, Resource, Stack, Token } from '@aws-cdk/core';
 import { IAlarmAction } from './alarm-action';
 import { CfnAlarm } from './cloudwatch.generated';
 import { HorizontalAnnotation } from './graph';
 import { CreateAlarmOptions } from './metric';
-import { IMetric } from './metric-types';
-import { dispatchMetric, dropUndefined, metricPeriod, MetricSet } from './metric-util';
-import { parseStatistic } from './util.statistic';
+import { IMetric, MetricStatConfig } from './metric-types';
+import { dispatchMetric, metricPeriod } from './private/metric-util';
+import { dropUndefined } from './private/object';
+import { MetricSet } from './private/rendering';
+import { parseStatistic } from './private/statistic';
 
 export interface IAlarm extends IResource {
   /**
@@ -142,7 +144,7 @@ export class Alarm extends Resource implements IAlarm {
       okActions: Lazy.listValue({ produce: () => this.okActionArns }),
 
       // Metric
-      ...renderAlarmMetric(props.metric),
+      ...this.renderMetric(props.metric),
       ...dropUndefined({
         // Alarm overrides
         period: props.period && props.period.toSeconds(),
@@ -225,63 +227,86 @@ export class Alarm extends Resource implements IAlarm {
   public toAnnotation(): HorizontalAnnotation {
     return this.annotation;
   }
-}
 
-function renderAlarmMetric(metric: IMetric) {
-  return dispatchMetric(metric, {
-    withStat(st) {
-      return dropUndefined({
-        dimensions: st.dimensions,
-        namespace: st.namespace,
-        metricName: st.metricName,
-        period: st.period?.toSeconds(),
-        statistic: renderIfSimpleStatistic(st.statistic),
-        extendedStatistic: renderIfExtendedStatistic(st.statistic),
-        unit: st.unitFilter,
-      });
-    },
-
-    withExpression() {
-      // Expand the math expression metric into a set
-      const mset = new MetricSet<boolean>();
-      mset.addTopLevel(true, metric);
-
-      let eid = 0;
-      function uniqueMetricId() {
-        return `expr_${++eid}`;
+  private renderMetric(metric: IMetric) {
+    const self = this;
+    return dispatchMetric(metric, {
+      withStat(st) {
+        self.validateMetricStat(st, metric);
+
+        return dropUndefined({
+          dimensions: st.dimensions,
+          namespace: st.namespace,
+          metricName: st.metricName,
+          period: st.period?.toSeconds(),
+          statistic: renderIfSimpleStatistic(st.statistic),
+          extendedStatistic: renderIfExtendedStatistic(st.statistic),
+          unit: st.unitFilter,
+        });
+      },
+
+      withExpression() {
+        // Expand the math expression metric into a set
+        const mset = new MetricSet<boolean>();
+        mset.addTopLevel(true, metric);
+
+        let eid = 0;
+        function uniqueMetricId() {
+          return `expr_${++eid}`;
+        }
+
+        return {
+          metrics: mset.entries.map(entry => dispatchMetric(entry.metric, {
+            withStat(stat, conf) {
+              self.validateMetricStat(stat, entry.metric);
+
+              return {
+                metricStat: {
+                  metric: {
+                    metricName: stat.metricName,
+                    namespace: stat.namespace,
+                    dimensions: stat.dimensions,
+                  },
+                  period: stat.period.toSeconds(),
+                  stat: stat.statistic,
+                  unit: stat.unitFilter,
+                },
+                id: entry.id || uniqueMetricId(),
+                label: conf.renderingProperties?.label,
+                returnData: entry.tag ? undefined : false, // Tag stores "primary" attribute, default is "true"
+              };
+            },
+            withExpression(expr, conf) {
+              return {
+                expression: expr.expression,
+                id: entry.id || uniqueMetricId(),
+                label: conf.renderingProperties?.label,
+                returnData: entry.tag ? undefined : false, // Tag stores "primary" attribute, default is "true"
+              };
+            },
+          }) as CfnAlarm.MetricDataQueryProperty)
+        };
       }
+    });
+  }
 
-      return {
-        metrics: mset.entries.map(entry => dispatchMetric(entry.metric, {
-          withStat(stat, conf) {
-            return {
-              metricStat: {
-                metric: {
-                  metricName: stat.metricName,
-                  namespace: stat.namespace,
-                  dimensions: stat.dimensions,
-                },
-                period: stat.period.toSeconds(),
-                stat: stat.statistic,
-                unit: stat.unitFilter,
-              },
-              id: entry.id ?? uniqueMetricId(),
-              label: conf.renderingProperties?.label,
-              returnData: entry.tag ? undefined : false, // Tag stores "primary" attribute, default is "true"
-            };
-          },
-          withExpression(expr, conf) {
-            return {
-              expression: expr.expression,
-              id: entry.id ?? uniqueMetricId(),
-              label: conf.renderingProperties?.label,
-              returnData: entry.tag ? undefined : false, // Tag stores "primary" attribute, default is "true"
-            };
-          },
-        }) as CfnAlarm.MetricDataQueryProperty)
-      };
+  /**
+   * Validate that if a region and account are in the given stat config, they match the Alarm
+   */
+  private validateMetricStat(stat: MetricStatConfig, metric: IMetric) {
+    const stack = Stack.of(this);
+
+    if (definitelyDifferent(stat.region, stack.region)) {
+      throw new Error(`Cannot create an Alarm in region '${stack.region}' based on metric '${metric}' in '${stat.region}'`);
+    }
+    if (definitelyDifferent(stat.account, stack.account)) {
+      throw new Error(`Cannot create an Alarm in account '${stack.account}' based on metric '${metric}' in '${stat.account}'`);
     }
-  });
+  }
+}
+
+function definitelyDifferent(x: string | undefined, y: string) {
+  return x && !Token.isUnresolved(y) && x !== y;
 }
 
 /**

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

@@ -1,7 +1,7 @@
 import * as cdk from '@aws-cdk/core';
 import { IAlarm } from "./alarm";
 import { IMetric } from "./metric-types";
-import { allMetricsGraphJson } from './metric-util';
+import { allMetricsGraphJson } from './private/rendering';
 import { ConcreteWidget } from "./widget";
 
 /**

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

@@ -2,8 +2,8 @@ import * as iam from '@aws-cdk/aws-iam';
 import * as cdk from '@aws-cdk/core';
 import { Alarm, ComparisonOperator, TreatMissingData } from './alarm';
 import { Dimension, IMetric, MetricAlarmConfig, MetricConfig, MetricGraphConfig, Unit } from './metric-types';
-import { metricKey, validateNoIdConflicts } from './metric-util';
-import { normalizeStatistic, parseStatistic } from './util.statistic';
+import { dispatchMetric, metricKey } from './private/metric-util';
+import { normalizeStatistic, parseStatistic } from './private/statistic';
 
 export type DimensionHash = {[dim: string]: any};
 
@@ -234,7 +234,21 @@ export class Metric implements IMetric {
    * @param props The set of properties to change.
    */
   public with(props: MetricOptions): Metric {
-    const ret = new Metric({
+    // Short-circuit creating a new object if there would be no effective change
+    if ((props.label === undefined || props.label === this.label)
+      && (props.color === undefined || props.color === this.color)
+      && (props.statistic === undefined || props.statistic === this.statistic)
+      && (props.unit === undefined || props.unit === this.unit)
+      && (props.account === undefined || props.account === this.account)
+      && (props.region === undefined || props.region === this.region)
+      // For these we're not going to do deep equality, misses some opportunity for optimization
+      // but that's okay.
+      && (props.dimensions === undefined)
+      && (props.period === undefined || props.period.toSeconds() === this.period.toSeconds())) {
+      return this;
+    }
+
+    return new Metric({
       dimensions: ifUndefined(props.dimensions, this.dimensions),
       namespace: this.namespace,
       metricName: this.metricName,
@@ -246,12 +260,27 @@ export class Metric implements IMetric {
       account: ifUndefined(props.account, this.account),
       region: ifUndefined(props.region, this.region)
     });
+  }
 
-    // Save on objects: if the returned object is the same as the current
-    // object, just return ourselves.
-    if (metricKey(ret) === metricKey(this) && ret.color === this.color && ret.label === this.label) { return this; }
+  /**
+   * Attach the metric object to the given construct scope
+   *
+   * Returns a Metric object that uses the account and region from the Stack
+   * the given construct is defined in. If the metric is subsequently used
+   * in a Dashboard or Alarm in a different Stack defined in a different
+   * account or region, the appropriate 'region' and 'account' fields
+   * will be added to it.
+   *
+   * If the scope we attach to is in an environment-agnostic stack,
+   * nothing is done and the same Metric object is returned.
+   */
+  public attachTo(scope: cdk.Construct): Metric {
+    const stack = cdk.Stack.of(scope);
 
-    return ret;
+    return this.with({
+      region: cdk.Token.isUnresolved(stack.region) ? undefined : stack.region,
+      account: cdk.Token.isUnresolved(stack.account) ? undefined : stack.account,
+    });
   }
 
   public toMetricConfig(): MetricConfig {
@@ -419,7 +448,7 @@ export class MathExpression implements IMetric {
       throw new Error(`Invalid variable names in expression: ${invalidVariableNames}. Must start with lowercase letter and only contain alphanumerics.`);
     }
 
-    validateNoIdConflicts(this);
+    this.validateNoIdConflicts();
   }
 
   /**
@@ -433,7 +462,7 @@ export class MathExpression implements IMetric {
     // Short-circuit creating a new object if there would be no effective change
     if ((props.label === undefined || props.label === this.label)
       && (props.color === undefined || props.color === this.color)
-      && (props.period === undefined)) {
+      && (props.period === undefined || props.period.toSeconds() === this.period.toSeconds())) {
       return this;
     }
 
@@ -493,6 +522,30 @@ export class MathExpression implements IMetric {
   public toString() {
     return this.label || this.expression;
   }
+
+  private validateNoIdConflicts() {
+    const seen = new Map<string, IMetric>();
+    visit(this);
+
+    function visit(metric: IMetric) {
+      dispatchMetric(metric, {
+        withStat() {
+          // Nothing
+        },
+        withExpression(expr) {
+          for (const [id, subMetric] of Object.entries(expr.usingMetrics)) {
+            const existing = seen.get(id);
+            if (existing && metricKey(existing) !== metricKey(subMetric)) {
+              throw new Error(`The ID '${id}' used for two metrics in the expression: '${subMetric}' and '${existing}'. Rename one.`);
+            }
+            seen.set(id, subMetric);
+            visit(subMetric);
+          }
+        }
+      });
+    }
+  }
+
 }
 
 const VALID_VARIABLE = new RegExp('^[a-z][a-zA-Z0-9_]*$');

packages/@aws-cdk/aws-cloudwatch/lib/private/drop-empty-object-at-the-end-of-an-array-token.ts

@@ -0,0 +1,33 @@
+import * as cdk from '@aws-cdk/core';
+import { dropUndefined } from './object';
+
+/**
+ * A Token object that will drop the last element of an array if it is an empty object
+ *
+ * Necessary to prevent options objects that only contain "region" and "account" keys
+ * that evaluate to "undefined" from showing up in the rendered JSON.
+ */
+export class DropEmptyObjectAtTheEndOfAnArray implements cdk.IResolvable, cdk.IPostProcessor {
+  public readonly creationStack: string[];
+
+  constructor(private readonly value: any) {
+    this.creationStack = cdk.captureStackTrace();
+  }
+
+  public resolve(context: cdk.IResolveContext) {
+    context.registerPostProcessor(this);
+    return context.resolve(this.value);
+  }
+
+  public postProcess(o: any, _context: cdk.IResolveContext): any {
+    if (!Array.isArray(o)) { return o; }
+
+    const lastEl = o[o.length - 1];
+
+    if (typeof lastEl === 'object' && lastEl !== null && Object.keys(dropUndefined(lastEl)).length === 0) {
+      return o.slice(0, o.length - 1);
+    }
+
+    return o;
+  }
+}

packages/@aws-cdk/aws-cloudwatch/lib/private/env-tokens.ts

@@ -0,0 +1,43 @@
+import * as cdk from '@aws-cdk/core';
+
+/**
+ * Make a Token that renders to given region if used in a different stack, otherwise undefined
+ */
+export function regionIfDifferentFromStack(region: string): string {
+  return cdk.Token.asString(new StackDependentToken(region, stack => stack.region));
+}
+
+/**
+ * Make a Token that renders to given account if used in a different stack, otherwise undefined
+ */
+export function accountIfDifferentFromStack(account: string): string {
+  return cdk.Token.asString(new StackDependentToken(account, stack => stack.account));
+}
+
+/**
+ * A lazy token that requires an instance of Stack to evaluate
+ */
+class StackDependentToken implements cdk.IResolvable {
+  public readonly creationStack: string[];
+  constructor(private readonly originalValue: string, private readonly fn: (stack: cdk.Stack) => string) {
+    this.creationStack = cdk.captureStackTrace();
+  }
+
+  public resolve(context: cdk.IResolveContext) {
+    const stackValue = this.fn(cdk.Stack.of(context.scope));
+
+    if (cdk.Token.isUnresolved(stackValue) || stackValue === this.originalValue) {
+      return undefined;
+    }
+
+    return this.originalValue;
+  }
+
+  public toString() {
+    return cdk.Token.asString(this);
+  }
+
+  public toJSON() {
+    return this.originalValue;
+  }
+}