From 5292bd56ad4f7c2bf68767861c5e252b13282f34 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Mon, 13 Jan 2020 17:45:55 +0100
Subject: [PATCH] fix(cloudwatch): cross-account metrics in env-agnostic stack
 (#5775)

In order to avoid generating unnecessary diffs to currently-deployed
CloudWatch dashboards, in aws/aws-cdk#5628 when adding support for
cross-region/cross-account metrics, we only selectively render the new
attributes into the graph (only when we estimate it will make a difference).

The method chosen was:

    Render account/region if they're *definitely* different.

However, this has the side effect that the new region and account
attributes don't work at all in environment-agnostic stacks (because we
won't know whether they'll be different or not, and we assume they will
be).

Whether the original behavior was wrong or not can be debated,
but it's unintuitive for sure: users put in values that don't come
back out in the usual, getting-started case.

In this PR, change the decision to:

    Don't render account/region if they're *definitely* the same.

This will fix the case of manual input to `Metric`, and since
`attachTo()` won't take account and region from environment-agnostic
stacks anyway, it also won't introduce unwanted diffs in most cases.
---
 .../aws-cloudwatch/lib/private/env-tokens.ts  |  4 ++-
 .../test/test.cross-environment.ts            | 32 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/private/env-tokens.ts b/packages/@aws-cdk/aws-cloudwatch/lib/private/env-tokens.ts
index e0157393df489..15fcda4886791 100644
--- a/packages/@aws-cdk/aws-cloudwatch/lib/private/env-tokens.ts
+++ b/packages/@aws-cdk/aws-cloudwatch/lib/private/env-tokens.ts
@@ -26,7 +26,9 @@ class StackDependentToken implements cdk.IResolvable {
   public resolve(context: cdk.IResolveContext) {
     const stackValue = this.fn(cdk.Stack.of(context.scope));
 
-    if (cdk.Token.isUnresolved(stackValue) || stackValue === this.originalValue) {
+    // Don't render if the values are definitely the same. If the stack
+    // is unresolved we don't know, better output the value.
+    if (!cdk.Token.isUnresolved(stackValue) && stackValue === this.originalValue) {
       return undefined;
     }
 
diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.cross-environment.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.cross-environment.ts
index a35f34d953304..3a970d80bb20e 100644
--- a/packages/@aws-cdk/aws-cloudwatch/test/test.cross-environment.ts
+++ b/packages/@aws-cdk/aws-cloudwatch/test/test.cross-environment.ts
@@ -46,6 +46,38 @@ export = {
 
       test.done();
     },
+
+    'metric with explicit account and region will render in environment agnostic stack'(test: Test) {
+      // GIVEN
+      const graph = new GraphWidget({
+        left: [
+          a.with({ account: '1234', region: 'us-north-5' })
+        ],
+      });
+
+      // THEN
+      graphMetricsAre(test, new Stack(), graph, [
+        [ 'Test', 'ACount', { accountId: '1234', region: 'us-north-5' }],
+      ]);
+
+      test.done();
+    },
+
+    'metric attached to agnostic stack will not render in agnostic stack'(test: Test) {
+      // GIVEN
+      const graph = new GraphWidget({
+        left: [
+          a.attachTo(new Stack()),
+        ],
+      });
+
+      // THEN
+      graphMetricsAre(test, new Stack(), graph, [
+        [ 'Test', 'ACount' ],
+      ]);
+
+      test.done();
+    },
   },
 
   'in alarms': {