From 340f1df7096fcb8d47628772008ed9f0686a1946 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 11 Aug 2021 17:25:54 -0400 Subject: [PATCH 1/7] pipe account to metricdataquery if necessary --- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 12 +++---- .../test/cross-environment.test.ts | 33 ++++++++++++++++++- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index 6b102d567125f..b360204f21ff0 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -1,11 +1,12 @@ import { Lazy, Stack, Token } from '@aws-cdk/core'; -import { Construct } from 'constructs'; +import { Construct, Node } from 'constructs'; import { IAlarmAction } from './alarm-action'; import { AlarmBase, IAlarm } from './alarm-base'; import { CfnAlarm, CfnAlarmProps } from './cloudwatch.generated'; import { HorizontalAnnotation } from './graph'; import { CreateAlarmOptions } from './metric'; import { IMetric, MetricExpressionConfig, MetricStatConfig } from './metric-types'; +import { accountIfDifferentFromStack } from './private/env-tokens'; import { dispatchMetric, metricPeriod } from './private/metric-util'; import { dropUndefined } from './private/object'; import { MetricSet } from './private/rendering'; @@ -257,7 +258,8 @@ export class Alarm extends AlarmBase { return dispatchMetric(metric, { withStat(stat, conf) { self.validateMetricStat(stat, metric); - if (conf.renderingProperties?.label == undefined) { + if (conf.renderingProperties?.label == undefined && + (stat.account == undefined || Stack.of(self).account == stat.account )) { return dropUndefined({ dimensions: stat.dimensions, namespace: stat.namespace, @@ -283,6 +285,7 @@ export class Alarm extends AlarmBase { unit: stat.unitFilter, }, id: 'm1', + accountId: stat.account, label: conf.renderingProperties?.label, returnData: true, } as CfnAlarm.MetricDataQueryProperty, @@ -344,7 +347,7 @@ export class Alarm extends AlarmBase { } /** - * Validate that if a region and account are in the given stat config, they match the Alarm + * Validate that if a region is in the given stat config, they match the Alarm */ private validateMetricStat(stat: MetricStatConfig, metric: IMetric) { const stack = Stack.of(this); @@ -352,9 +355,6 @@ export class Alarm extends AlarmBase { if (definitelyDifferent(stat.region, stack.region)) { throw new Error(`Cannot create an Alarm in region '${stack.region}' based on metric '${metric}' in '${stat.region}'`); } - if (definitelyDifferent(stat.account, stack.account)) { - throw new Error(`Cannot create an Alarm in account '${stack.account}' based on metric '${metric}' in '${stat.account}'`); - } } } diff --git a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts index 7c7ed7d04bf6f..c2783b38e0a0b 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts @@ -6,11 +6,12 @@ const a = new Metric({ namespace: 'Test', metricName: 'ACount' }); let stack1: Stack; let stack2: Stack; +let stack3: Stack; describe('cross environment', () => { beforeEach(() => { stack1 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '1234' } }); stack2 = new Stack(undefined, undefined, { env: { region: 'mars', account: '5678' } }); - + stack3 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '0000'} }); }); describe('in graphs', () => { @@ -112,6 +113,36 @@ describe('cross environment', () => { }); + + test('metric attached to stack3 will render in stack1 ', () => { + //Cross-account metrics are supported in Alarms + + // GIVEN + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: a.attachTo(stack3), + }); + + // THEN + Template.fromStack(stack1).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: [ + { + AccountId: "0000", + Id: "m1", + MetricStat: { + Metric: { + MetricName: "ACount", + Namespace: "Test", + }, + Period: 300, + Stat: "Average", + }, + ReturnData: true, + } + ] + }); + }) }); }); From 0821355b954c2bd30fdc451488b97850f3cd6b48 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 11 Aug 2021 17:42:05 -0400 Subject: [PATCH 2/7] fix accidental imports --- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index b360204f21ff0..83dfce92442c8 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -1,12 +1,11 @@ import { Lazy, Stack, Token } from '@aws-cdk/core'; -import { Construct, Node } from 'constructs'; +import { Construct } from 'constructs'; import { IAlarmAction } from './alarm-action'; import { AlarmBase, IAlarm } from './alarm-base'; import { CfnAlarm, CfnAlarmProps } from './cloudwatch.generated'; import { HorizontalAnnotation } from './graph'; import { CreateAlarmOptions } from './metric'; import { IMetric, MetricExpressionConfig, MetricStatConfig } from './metric-types'; -import { accountIfDifferentFromStack } from './private/env-tokens'; import { dispatchMetric, metricPeriod } from './private/metric-util'; import { dropUndefined } from './private/object'; import { MetricSet } from './private/rendering'; From d128fcc72662e0b40bf6c2149e45808ea49a2eac Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 11 Aug 2021 17:49:47 -0400 Subject: [PATCH 3/7] satisfy linter --- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 2 +- .../test/cross-environment.test.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index 83dfce92442c8..8602d6a1f03d0 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -257,7 +257,7 @@ export class Alarm extends AlarmBase { return dispatchMetric(metric, { withStat(stat, conf) { self.validateMetricStat(stat, metric); - if (conf.renderingProperties?.label == undefined && + if (conf.renderingProperties?.label == undefined && (stat.account == undefined || Stack.of(self).account == stat.account )) { return dropUndefined({ dimensions: stat.dimensions, diff --git a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts index c2783b38e0a0b..3778a9a462e87 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts @@ -11,7 +11,7 @@ describe('cross environment', () => { beforeEach(() => { stack1 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '1234' } }); stack2 = new Stack(undefined, undefined, { env: { region: 'mars', account: '5678' } }); - stack3 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '0000'} }); + stack3 = new Stack(undefined, undefined, { env: { region: 'pluto', account: '0000' } }); }); describe('in graphs', () => { @@ -128,21 +128,21 @@ describe('cross environment', () => { Template.fromStack(stack1).hasResourceProperties('AWS::CloudWatch::Alarm', { Metrics: [ { - AccountId: "0000", - Id: "m1", + AccountId: '0000', + Id: 'm1', MetricStat: { Metric: { - MetricName: "ACount", - Namespace: "Test", + MetricName: 'ACount', + Namespace: 'Test', }, Period: 300, - Stat: "Average", + Stat: 'Average', }, ReturnData: true, - } - ] + }, + ], }); - }) + }); }); }); From 749fd0a724b7f10d8c5b9e8cbff3724c10a1c927 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 11 Aug 2021 18:03:50 -0400 Subject: [PATCH 4/7] update readme for cross-account alarms --- packages/@aws-cdk/aws-cloudwatch/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@aws-cdk/aws-cloudwatch/README.md b/packages/@aws-cdk/aws-cloudwatch/README.md index ff1f1997489f4..cb6cd38bf8e65 100644 --- a/packages/@aws-cdk/aws-cloudwatch/README.md +++ b/packages/@aws-cdk/aws-cloudwatch/README.md @@ -160,6 +160,8 @@ The most important properties to set while creating an Alarms are: - `evaluationPeriods`: how many consecutive periods the metric has to be breaching the the threshold for the alarm to trigger. +To create a cross-account alarm, make sure you have enabled [cross-account functionality](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Cross-Account-Cross-Region.html) in CloudWatch. Then, set the `account` property in the `Metric` object either manually or via the `metric.attachTo()` function. + ### Alarm Actions To add actions to an alarm, use the integration classes from the From ed1fd3a79614cb137ddbc41a0003bdce3d7398c1 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Fri, 13 Aug 2021 15:12:00 -0400 Subject: [PATCH 5/7] add renderAsLegacyMetric --- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index 8602d6a1f03d0..41928d0af7193 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -257,8 +257,10 @@ export class Alarm extends AlarmBase { return dispatchMetric(metric, { withStat(stat, conf) { self.validateMetricStat(stat, metric); - if (conf.renderingProperties?.label == undefined && - (stat.account == undefined || Stack.of(self).account == stat.account )) { + const canRenderAsLegacyMetric = conf.renderingProperties?.label == undefined && + (stat.account == undefined || Stack.of(self).account == stat.account); + // Do this to disturb existing templates as little as possible + if (canRenderAsLegacyMetric) { return dropUndefined({ dimensions: stat.dimensions, namespace: stat.namespace, From f1451ffff7c90449e503d46d53d495e814ab8b37 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Mon, 23 Aug 2021 13:40:17 -0400 Subject: [PATCH 6/7] add new test and more readme docs --- packages/@aws-cdk/aws-cloudwatch/README.md | 4 +- .../test/cross-environment.test.ts | 37 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/README.md b/packages/@aws-cdk/aws-cloudwatch/README.md index cb6cd38bf8e65..bdcf58e61024f 100644 --- a/packages/@aws-cdk/aws-cloudwatch/README.md +++ b/packages/@aws-cdk/aws-cloudwatch/README.md @@ -28,6 +28,8 @@ represents the amount of errors reported by that Lambda function: const errors = fn.metricErrors(); ``` +`Metric` objects can be account and region aware. You can specify `account` and `region` as properties of the metric, or use the `metric.attachTo(Construct)` method. `metric.attachTo()` will automatically copy the `region` and `account` fields of the `Construct`, which can come from anywhere in the Construct tree. + You can also instantiate `Metric` objects to reference any [published metric](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/aws-services-cloudwatch-metrics.html) that's not exposed using a convenience method on the CDK construct. @@ -160,7 +162,7 @@ The most important properties to set while creating an Alarms are: - `evaluationPeriods`: how many consecutive periods the metric has to be breaching the the threshold for the alarm to trigger. -To create a cross-account alarm, make sure you have enabled [cross-account functionality](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Cross-Account-Cross-Region.html) in CloudWatch. Then, set the `account` property in the `Metric` object either manually or via the `metric.attachTo()` function. +To create a cross-account alarm, make sure you have enabled [cross-account functionality](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Cross-Account-Cross-Region.html) in CloudWatch. Then, set the `account` property in the `Metric` object either manually or via the `metric.attachTo()` method. ### Alarm Actions diff --git a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts index 3778a9a462e87..8680383721274 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts @@ -114,7 +114,7 @@ describe('cross environment', () => { }); - test('metric attached to stack3 will render in stack1 ', () => { + test('metric attached to stack3 will render in stack1', () => { //Cross-account metrics are supported in Alarms // GIVEN @@ -143,6 +143,41 @@ describe('cross environment', () => { ], }); }); + + test('metric can render in a different account', () => { + // GIVEN + + const b = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: '0000', + }); + + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: b, + }); + + // THEN + Template.fromStack(stack1).hasResourceProperties('AWS::CloudWatch::Alarm', { + Metrics: [ + { + AccountId: '0000', + Id: 'm1', + MetricStat: { + Metric: { + MetricName: 'ACount', + Namespace: 'Test', + }, + Period: 300, + Stat: 'Average', + }, + ReturnData: true, + }, + ], + }); + }); }); }); From a7666f178d3a67102aae74cce615366818b39c1b Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Mon, 23 Aug 2021 13:42:29 -0400 Subject: [PATCH 7/7] remove 1 line --- packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts index 8680383721274..f17391591a8c5 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts @@ -146,7 +146,6 @@ describe('cross environment', () => { test('metric can render in a different account', () => { // GIVEN - const b = new Metric({ namespace: 'Test', metricName: 'ACount',