Skip to content

Commit

Permalink
fix(cloudwatch): Render region and accountId when directly set on met…
Browse files Browse the repository at this point in the history
…rics

Closes aws#28731
  • Loading branch information
Trevor Burnham committed Apr 23, 2024
1 parent 31492c6 commit 5236824
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 16 deletions.
10 changes: 10 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric-types.ts
Expand Up @@ -323,6 +323,16 @@ export interface MetricStatConfig {
* @default Deployment account.
*/
readonly account?: string;

/**
* Region set directly on the metric, not inherited from the attached stack.
*/
readonly regionOverride?: string;

/**
* Account set directly on the metric, not inherited from the attached stack.
*/
readonly accountOverride?: string;
}

/**
Expand Down
60 changes: 46 additions & 14 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric.ts
Expand Up @@ -105,16 +105,26 @@ export interface CommonMetricOptions {
/**
* Account which this metric comes from.
*
* @default - Deployment account.
* @default - The account of the attached stack.
*/
readonly account?: string;

/**
* Region which this metric comes from.
*
* @default - Deployment region.
* @default - The region of the attached stack.
*/
readonly region?: 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;
}

/**
Expand Down Expand Up @@ -277,11 +287,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. */
private readonly stackAccount?: string;

/** Region of the stack this metric is attached to. */
private 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. */
private readonly accountOverride?: string;

/** Region set directly on the metric, taking precedence over the stack region. */
private readonly regionOverride?: string;

/**
* Warnings attached to this metric.
Expand Down Expand Up @@ -323,8 +339,10 @@ 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.stackAccount = props.stackAccount;
this.stackRegion = props.stackRegion;
this.accountOverride = props.account;
this.regionOverride = props.region;
}

/**
Expand All @@ -340,8 +358,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)
Expand All @@ -359,8 +379,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),
});
}

Expand All @@ -380,11 +402,19 @@ 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,
});
}

public get account(): string | undefined {
return this.accountOverride || this.stackAccount;
}

public get region(): string | undefined {
return this.regionOverride || this.stackRegion;
}

public toMetricConfig(): MetricConfig {
const dims = this.dimensionsAsList();
return {
Expand All @@ -397,6 +427,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,
Expand Down
12 changes: 10 additions & 2 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/private/rendering.ts
Expand Up @@ -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; }
},
Expand Down
13 changes: 13 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/test/cross-environment.test.ts
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 5236824

Please sign in to comment.