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 authored and TrevorBurnham committed Jan 2, 2025
1 parent 3b162fc commit dc87654
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 30 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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\"}}]}"
]
]
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
14 changes: 14 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
71 changes: 59 additions & 12 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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');
}

/**
Expand All @@ -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)
Expand All @@ -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),
});
}

Expand All @@ -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 {
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
});
}
12 changes: 10 additions & 2 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/private/rendering.ts
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
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
29 changes: 29 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/test/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit dc87654

Please sign in to comment.