cloudwatch: Explicitly set region and accountId fields are removed from metrics when they match the stack

[TrevorBurnham]

Describe the bug

In my dashboard created from the CDK, I noticed that the region property was missing from metrics when that metric matched the stack. For example, for a stack in us-east-1, the metrics would look like this:

[
  {
    "label": "My metric [us-west-2]",
    "region": "us-west-2",
    "period": 60,
    "stat": "Maximum"
  },
  {
    "label": "My metric [us-east-1]",
    "period": 60,
    "stat": "Maximum"
  },
  // ...
]

This broke some region-based filtering logic in my UI application.

Expected Behavior

I expected the region field to exist for all of my metrics, since I was setting it explicitly:

new Metric({
  label: `My metric [${region}]`,
  region: region,
  period: 60,
  stat: 'Maximum'
})

Current Behavior

During serialization, the region and accountId fields are removed if they match the corresponding region and account values on the stack.

Reproduction Steps

You can confirm the behavior by adding this unit test to cross-environment.test.ts:

test('metric with explicit account and region will render as-is in stack with same region and account', () => {
  // GIVEN
  const graph = new GraphWidget({
    left: [
      a.with({ account: '1234', region: 'us-north-5' }),
    ],
  });

  // THEN
  // Assertion fails. It would pass if the stack had different region and account values.
  graphMetricsAre(new Stack(undefined, undefined, { env: { region: 'us-north-5', account: '1234' } }), graph, [
    ['Test', 'ACount', { accountId: '1234', region: 'us-north-5' }],
  ]);
});

Possible Solution

The problem arises from these two lines in the metricGraphJson function:

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

Lines 50 to 51 in 109b2ab

	if (stat.account) { options.accountId = accountIfDifferentFromStack(stat.account); }
	if (stat.region) { options.region = regionIfDifferentFromStack(stat.region); }

I'm not sure, but I think the intent here is to prevent the region and account from being included in the output if they were set implicitly by the attachTo function. As that function's description says:

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.

I'm not sure why it's so important to prevent the region and account fields from being included when they match the stack, but if it is, you could have attachTo attach a placeholder that metricGraphJson can process appropriately (e.g. "${ifDifferentFromStack(us-east-1)}") rather than attaching a value that's indistinguishable from one that's been explicitly set.

Additional Information/Context

No response

CDK CLI Version

2.121.1

Framework Version

No response

Node.js Version

18.15.0

OS

macOS 13.6.3

Language

TypeScript

Language Version

No response

Other information

Possibly related to #18951?

[pahud]

Looks like all conditions here have to be satisfied

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

Lines 339 to 349 in 20ad55e

	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.dimensionsMap === undefined)
	&& (props.period === undefined || props.period.toSeconds() === this.period.toSeconds())) {

Otherwise a new Matric will be created and region would be undefined here:

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

Line 363 in 20ad55e

region: ifUndefined(props.region, this.region),

IMO, yes, if you specify the region explicitly, it should be passed and defined explicitly.

I am making it a p2 but we welcome and appreciate any pull requests.

[TrevorBurnham]

@pahud I've submitted a pull request to fix this: #29935

[TrevorBurnham]

@pahud I've made another attempt to fix this: #32325

[AlbyM-dev]

I am in the same situation, when the 2 metrics are rendered, the second metric doesn't have a region specified and so the region for the above metric is used, breaking the second metric

this is the same graph generated manually without CDK:

I guess that happens because when a metric doesn't have a specific region and the one above has one, the metric inherits that region and not the stack region, so the logic around how cdk hides the region if not required (because it's in the environment) doesn't really follow how cloudwatch dashboards work.

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.