fix(cloudwatch): render region and accountId when directly set on met…

…rics (#32325)

### Issue #

Closes #28731

### Reason for this change

Currently, if a user creates a metric that includes `region` and `accountId`, those fields are omitted when the metric renders in a stack with those same values. The intended behavior is to omit those fields when they're implicitly set via `metric.attachTo(stack)`, not to omit them when set directly by the user.

### Description of changes

This is a second attempt at #29935, which was auto-closed after the pull request linter complained that there were no integration test changes. This time around I've tried to satisfy the linter. Otherwise, the changes are the same as before:

The key changes here are on the `Metric` class. Previously, `region` and `account` were public properties that were set by `metric.attachTo(stack)`, making it impossible to differentiate whether they were set directly or via `attachTo`. To differentiate them while maintaining backward compatibility, I took this approach:

1. `attachTo` sets internal properties called `stackRegion` and `stackAccount`. Setting `region` and `account` directly sets internal properties called `regionOverride` and `accountOverride`.
2. The public `region` and `account` properties are now getters that return the override (if set) and fall back on the stack properties.
3. The `toMetricConfig()` method returns the `region` and `account` from the getters, but also includes the `regionOverride` and `accountOverride`.

That way, everything that looks at `region` and `account` works the same way it did before, except in `metricGraphJson`, which skips the "if different from stack" tokenization if the overrides are set.

### Description of how you validated changes

1. Confirmed that all existing unit tests pass.
2. Added a new unit test to show that `region` and `account` are now preserved when set directly on a metric.
3. Modified an integration test to show how setting `region` and `account` on a metric affects the snapshot.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.dashboard-and-widget-with-start-and-end.js.snapshot/DashboardAndWidgetWithStartAndEnd.template.json

@@ -11,15 +11,15 @@
        {
         "Ref": "AWS::Region"
        },
-       "\",\"metrics\":[[\"CDK/Test\",\"Metric\"]],\"start\":\"-P7D\",\"end\":\"2018-12-17T06:00:00.000Z\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":3,\"properties\":{\"view\":\"timeSeries\",\"region\":\"",
+       "\",\"metrics\":[[\"CDK/Test\",\"Metric\",{\"accountId\":\"1234\",\"region\":\"us-north-5\"}]],\"start\":\"-P7D\",\"end\":\"2018-12-17T06:00:00.000Z\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":3,\"properties\":{\"view\":\"timeSeries\",\"region\":\"",
        {
         "Ref": "AWS::Region"
        },
-       "\",\"metrics\":[[\"CDK/Test\",\"Metric\"]],\"yAxis\":{},\"start\":\"-P7D\",\"end\":\"2018-12-17T06:00:00.000Z\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":9,\"properties\":{\"view\":\"gauge\",\"region\":\"",
+       "\",\"metrics\":[[\"CDK/Test\",\"Metric\",{\"accountId\":\"1234\",\"region\":\"us-north-5\"}]],\"yAxis\":{},\"start\":\"-P7D\",\"end\":\"2018-12-17T06:00:00.000Z\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":9,\"properties\":{\"view\":\"gauge\",\"region\":\"",
        {
         "Ref": "AWS::Region"
        },
-       "\",\"metrics\":[[\"CDK/Test\",\"Metric\"]],\"yAxis\":{\"left\":{\"min\":0,\"max\":100}},\"start\":\"-P7D\",\"end\":\"2018-12-17T06:00:00.000Z\"}}]}"
+       "\",\"metrics\":[[\"CDK/Test\",\"Metric\",{\"accountId\":\"1234\",\"region\":\"us-north-5\"}]],\"yAxis\":{\"left\":{\"min\":0,\"max\":100}},\"start\":\"-P7D\",\"end\":\"2018-12-17T06:00:00.000Z\"}}]}"
       ]
      ]
     }

packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.dashboard-and-widget-with-start-and-end.ts

@@ -11,6 +11,8 @@ class TestStack extends Stack {
     const testMetric = new Metric({
       namespace: 'CDK/Test',
       metricName: 'Metric',
+      account: '1234',
+      region: 'us-north-5',
     });
 
     const singleValueWidget = new SingleValueWidget({

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

@@ -323,6 +323,20 @@ export interface MetricStatConfig {
    * @default Deployment account.
    */
   readonly account?: string;
+
+  /**
+   * Region set directly on the metric, not inherited from the attached stack.
+   *
+   * @default No override.
+   */
+  readonly regionOverride?: string;
+
+  /**
+   * Account set directly on the metric, not inherited from the attached stack.
+   *
+   * @default No override.
+   */
+  readonly accountOverride?: string;
 }
 
 /**

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

@@ -6,6 +6,7 @@ import { normalizeStatistic, pairStatisticToString, parseStatistic, singleStatis
 import { Stats } from './stats';
 import * as iam from '../../aws-iam';
 import * as cdk from '../../core';
+import { makeEnumerable } from './private/make-enumerable';
 
 export type DimensionHash = { [dim: string]: any };
 
@@ -115,6 +116,20 @@ export interface CommonMetricOptions {
    * @default - Deployment region.
    */
   readonly region?: string;
+
+  /**
+   * Account of the stack this metric is attached to.
+   *
+   * @default - Deployment account.
+   */
+  readonly stackAccount?: string;
+
+  /**
+   * Region of the stack this metric is attached to.
+   *
+   * @default - Deployment region.
+   */
+  readonly stackRegion?: string;
 }
 
 /**
@@ -306,11 +321,17 @@ export class Metric implements IMetric {
   /** Unit of the metric. */
   public readonly unit?: Unit;
 
-  /** Account which this metric comes from */
-  public readonly account?: string;
+  /** Account of the stack this metric is attached to. */
+  readonly #stackAccount?: string;
+
+  /** Region of the stack this metric is attached to. */
+  readonly #stackRegion?: string;
 
-  /** Region which this metric comes from. */
-  public readonly region?: string;
+  /** Account set directly on the metric, taking precedence over the stack account. */
+  readonly #accountOverride?: string;
+
+  /** Region set directly on the metric, taking precedence over the stack region. */
+  readonly #regionOverride?: string;
 
   /**
    * Warnings attached to this metric.
@@ -352,8 +373,14 @@ export class Metric implements IMetric {
     this.label = props.label;
     this.color = props.color;
     this.unit = props.unit;
-    this.account = props.account;
-    this.region = props.region;
+    this.#accountOverride = props.account;
+    this.#regionOverride = props.region;
+    this.#stackAccount = props.stackAccount;
+    this.#stackRegion = props.stackRegion;
+
+    // Make getters enumerable.
+    makeEnumerable(Metric.prototype, this, 'account');
+    makeEnumerable(Metric.prototype, this, 'region');
   }
 
   /**
@@ -369,8 +396,10 @@ export class Metric implements IMetric {
       && (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)
+      && (props.account === undefined || props.account === this.#accountOverride)
+      && (props.region === undefined || props.region === this.#regionOverride)
+      && (props.stackAccount === undefined || props.stackAccount === this.#stackAccount)
+      && (props.stackRegion === undefined || props.stackRegion === this.#stackRegion)
       // For these we're not going to do deep equality, misses some opportunity for optimization
       // but that's okay.
       && (props.dimensions === undefined)
@@ -388,8 +417,10 @@ export class Metric implements IMetric {
       unit: ifUndefined(props.unit, this.unit),
       label: ifUndefined(props.label, this.label),
       color: ifUndefined(props.color, this.color),
-      account: ifUndefined(props.account, this.account),
-      region: ifUndefined(props.region, this.region),
+      account: ifUndefined(props.account, this.#accountOverride),
+      region: ifUndefined(props.region, this.#regionOverride),
+      stackAccount: ifUndefined(props.stackAccount, this.#stackAccount),
+      stackRegion: ifUndefined(props.stackRegion, this.#stackRegion),
     });
   }
 
@@ -409,11 +440,25 @@ export class Metric implements IMetric {
     const stack = cdk.Stack.of(scope);
 
     return this.with({
-      region: cdk.Token.isUnresolved(stack.region) ? undefined : stack.region,
-      account: cdk.Token.isUnresolved(stack.account) ? undefined : stack.account,
+      stackAccount: cdk.Token.isUnresolved(stack.account) ? undefined : stack.account,
+      stackRegion: cdk.Token.isUnresolved(stack.region) ? undefined : stack.region,
     });
   }
 
+  /**
+   * Account which this metric comes from.
+   */
+  public get account(): string | undefined {
+    return this.#accountOverride || this.#stackAccount;
+  }
+
+  /**
+   * Region which this metric comes from.
+   */
+  public get region(): string | undefined {
+    return this.#regionOverride || this.#stackRegion;
+  }
+
   public toMetricConfig(): MetricConfig {
     const dims = this.dimensionsAsList();
     return {
@@ -426,6 +471,8 @@ export class Metric implements IMetric {
         unitFilter: this.unit,
         account: this.account,
         region: this.region,
+        accountOverride: this.#accountOverride,
+        regionOverride: this.#regionOverride,
       },
       renderingProperties: {
         color: this.color,

packages/aws-cdk-lib/aws-cloudwatch/lib/private/make-enumerable.ts

@@ -0,0 +1,9 @@
+/**
+ * Make a property from the specified prototype enumerable on the specific instance.
+ */
+export function makeEnumerable(prototype: object, instance: object, propertyKey: string) {
+  Object.defineProperty(instance, propertyKey, {
+    ...Object.getOwnPropertyDescriptor(prototype, propertyKey),
+    enumerable: true,
+  });
+}

packages/aws-cdk-lib/aws-cloudwatch/lib/private/rendering.ts

@@ -47,8 +47,16 @@ function metricGraphJson(metric: IMetric, yAxis?: string, id?: string) {
       }
 
       // Metric attributes that are rendered to graph options
-      if (stat.account) { options.accountId = accountIfDifferentFromStack(stat.account); }
-      if (stat.region) { options.region = regionIfDifferentFromStack(stat.region); }
+      if (stat.accountOverride) {
+        options.accountId = stat.accountOverride;
+      } else if (stat.account) {
+        options.accountId = accountIfDifferentFromStack(stat.account);
+      }
+      if (stat.regionOverride) {
+        options.region = stat.regionOverride;
+      } else if (stat.region) {
+        options.region = regionIfDifferentFromStack(stat.region);
+      }
       if (stat.period && stat.period.toSeconds() !== 300) { options.period = stat.period.toSeconds(); }
       if (stat.statistic && stat.statistic !== 'Average') { options.stat = stat.statistic; }
     },

packages/aws-cdk-lib/aws-cloudwatch/test/cross-environment.test.ts

@@ -59,7 +59,20 @@ describe('cross environment', () => {
       graphMetricsAre(new Stack(), graph, [
         ['Test', 'ACount', { accountId: '1234', region: 'us-north-5' }],
       ]);
+    });
 
+    test('metric with explicit account and region that match stack will render as-is', () => {
+      // GIVEN
+      const graph = new GraphWidget({
+        left: [
+          a.with({ account: '1234', region: 'us-north-5' }),
+        ],
+      });
+
+      // THEN
+      graphMetricsAre(new Stack(undefined, undefined, { env: { region: 'us-north-5', account: '1234' } }), graph, [
+        ['Test', 'ACount', { accountId: '1234', region: 'us-north-5' }],
+      ]);
     });
 
     test('metric attached to agnostic stack will not render in agnostic stack', () => {

packages/aws-cdk-lib/aws-cloudwatch/test/metrics.test.ts

@@ -273,6 +273,35 @@ describe('Metrics', () => {
     expect(metric.statistic).toEqual(customStat);
   });
 
+  test('region and account getters are enumerable', () => {
+    const metric = new Metric({
+      namespace: 'Test',
+      metricName: 'Metric',
+      period: cdk.Duration.minutes(10),
+      region: 'test-region',
+      account: 'test-account',
+    });
+
+    expect(metric.region).toBe('test-region');
+    expect(metric.account).toBe('test-account');
+
+    const metricObject = { ...metric };
+    expect(metricObject).toEqual(expect.objectContaining({
+      region: 'test-region',
+      account: 'test-account',
+    }));
+
+    // Check that private fields are not included.
+    // @ts-expect-error
+    expect(metricObject.accountOverride).toBeUndefined();
+    // @ts-expect-error
+    expect(metricObject.stackAccount).toBeUndefined();
+    // @ts-expect-error
+    expect(metricObject.regionOverride).toBeUndefined();
+    // @ts-expect-error
+    expect(metricObject.stackRegion).toBeUndefined();
+  });
+
   test('statistic is properly parsed', () => {
     const checkParsingSingle = (statistic: string, statPrefix: string, statName: string, value: number) => {
       const parsed = parseStatistic(statistic);