Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(cloudwatch): render region and accountId when directly set on metrics #29935

Closed
wants to merge 1 commit into from
Closed

fix(cloudwatch): render region and accountId when directly set on metrics #29935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Loading