Merge branch 'master' into sfn-tasks-aws-sdk

packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts

@@ -257,8 +257,7 @@ export class Alarm extends AlarmBase {
     return dispatchMetric(metric, {
       withStat(stat, conf) {
         self.validateMetricStat(stat, metric);
-        const canRenderAsLegacyMetric = conf.renderingProperties?.label == undefined &&
-          (stat.account == undefined || Stack.of(self).account == stat.account);
+        const canRenderAsLegacyMetric = conf.renderingProperties?.label == undefined && !self.requiresAccountId(stat);
         // Do this to disturb existing templates as little as possible
         if (canRenderAsLegacyMetric) {
           return dropUndefined({
@@ -286,7 +285,7 @@ export class Alarm extends AlarmBase {
                 unit: stat.unitFilter,
               },
               id: 'm1',
-              accountId: stat.account,
+              accountId: self.requiresAccountId(stat) ? stat.account : undefined,
               label: conf.renderingProperties?.label,
               returnData: true,
             } as CfnAlarm.MetricDataQueryProperty,
@@ -321,7 +320,7 @@ export class Alarm extends AlarmBase {
                   unit: stat.unitFilter,
                 },
                 id: entry.id || uniqueMetricId(),
-                accountId: stat.account,
+                accountId: self.requiresAccountId(stat) ? stat.account : undefined,
                 label: conf.renderingProperties?.label,
                 returnData: entry.tag ? undefined : false, // entry.tag evaluates to true if the metric is the math expression the alarm is based on.
               };
@@ -370,6 +369,32 @@ export class Alarm extends AlarmBase {
       throw new Error('Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion');
     }
   }
+
+  /**
+   * Determine if the accountId property should be included in the metric.
+   */
+  private requiresAccountId(stat: MetricStatConfig): boolean {
+    const stackAccount = Stack.of(this).account;
+
+    // if stat.account is undefined, it's by definition in the same account
+    if (stat.account === undefined) {
+      return false;
+    }
+
+    // if this is a region-agnostic stack, we can't assume anything about stat.account
+    // and therefore we assume its a cross-account call
+    if (Token.isUnresolved(stackAccount)) {
+      return true;
+    }
+
+    // ok, we can compare the two concrete values directly - if they are the same we
+    // can omit the account ID from the metric.
+    if (stackAccount === stat.account) {
+      return false;
+    }
+
+    return true;
+  }
 }
 
 function definitelyDifferent(x: string | undefined, y: string) {

packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts

@@ -9,9 +9,7 @@ const testMetric = new Metric({
 });
 
 describe('Alarm', () => {
-
   test('alarm does not accept a math expression with more than 10 metrics', () => {
-
     const stack = new Stack();
 
     const usingMetrics: Record<string, IMetric> = {};
@@ -30,19 +28,15 @@ describe('Alarm', () => {
     });
 
     expect(() => {
-
       new Alarm(stack, 'Alarm', {
         metric: math,
         threshold: 1000,
         evaluationPeriods: 3,
       });
-
     }).toThrow(/Alarms on math expressions cannot contain more than 10 individual metrics/);
-
-
   });
-  test('non ec2 instance related alarm does not accept EC2 action', () => {
 
+  test('non ec2 instance related alarm does not accept EC2 action', () => {
     const stack = new Stack();
     const alarm = new Alarm(stack, 'Alarm', {
       metric: testMetric,
@@ -53,8 +47,8 @@ describe('Alarm', () => {
     expect(() => {
       alarm.addAlarmAction(new Ec2TestAlarmAction('arn:aws:automate:us-east-1:ec2:reboot'));
     }).toThrow(/EC2 alarm actions requires an EC2 Per-Instance Metric. \(.+ does not have an 'InstanceId' dimension\)/);
-
   });
+
   test('can make simple alarm', () => {
     // GIVEN
     const stack = new Stack();
@@ -76,8 +70,6 @@ describe('Alarm', () => {
       Statistic: 'Average',
       Threshold: 1000,
     });
-
-
   });
 
   test('override metric period in Alarm', () => {
@@ -102,8 +94,6 @@ describe('Alarm', () => {
       Statistic: 'Average',
       Threshold: 1000,
     });
-
-
   });
 
   test('override statistic Alarm', () => {
@@ -129,8 +119,6 @@ describe('Alarm', () => {
       ExtendedStatistic: Match.absent(),
       Threshold: 1000,
     });
-
-
   });
 
   test('can use percentile in Alarm', () => {
@@ -156,8 +144,6 @@ describe('Alarm', () => {
       ExtendedStatistic: 'p99',
       Threshold: 1000,
     });
-
-
   });
 
   test('can set DatapointsToAlarm', () => {
@@ -183,8 +169,6 @@ describe('Alarm', () => {
       Statistic: 'Average',
       Threshold: 1000,
     });
-
-
   });
 
   test('can add actions to alarms', () => {
@@ -208,8 +192,6 @@ describe('Alarm', () => {
       InsufficientDataActions: ['B'],
       OKActions: ['C'],
     });
-
-
   });
 
   test('can make alarm directly from metric', () => {
@@ -234,8 +216,6 @@ describe('Alarm', () => {
       Statistic: 'Minimum',
       Threshold: 1000,
     });
-
-
   });
 
   test('can use percentile string to make alarm', () => {
@@ -253,8 +233,6 @@ describe('Alarm', () => {
     Template.fromStack(stack).hasResourceProperties('AWS::CloudWatch::Alarm', {
       ExtendedStatistic: 'p99.9',
     });
-
-
   });
 
   test('can use a generic string for extended statistic to make alarm', () => {
@@ -273,9 +251,7 @@ describe('Alarm', () => {
       Statistic: Match.absent(),
       ExtendedStatistic: 'tm99.9999999999',
     });
-
   });
-
 });
 
 class TestAlarmAction implements IAlarmAction {