From c165138fa7c3456e530ffeab9b7a038914cc2dca Mon Sep 17 00:00:00 2001 From: Leah Hirst Date: Mon, 27 Sep 2021 16:26:34 +0100 Subject: [PATCH] feat(cloudwatch): support cross-environment search expressions (#16539) Fixes #9039 (also some prior discussion on https://github.com/aws/aws-cdk/pull/9034) Currently CDK gives no way to specify account or region on MathExpressions which means it is not possible to run search expressions against environments other than the deployment environment. The CloudWatch console currently has this capability, however [the documentation](https://docs.aws.amazon.com/en_us/AmazonCloudWatch/latest/APIReference/CloudWatch-Dashboard-Body-Structure.html#CloudWatch-Dashboard-Properties-Metric-Widget-Object) still states that `accountId` or `region` should only be used on metric widgets. I've left feedback to ask whether this documentation is out of date or whether it is an undocumented feature. This change adds in the `searchAccount` and `searchRegion` property which can be optionally specified for the Dashboards use case. When a MathExpression with a searchAccount or searchRegion set is attempted to be used within an Alarm, the change throws an error as Alarms do not support search expressions. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-cloudwatch/README.md | 22 +++++- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 12 +++ .../aws-cloudwatch/lib/metric-types.ts | 14 ++++ .../@aws-cdk/aws-cloudwatch/lib/metric.ts | 55 ++++++++++++- .../aws-cloudwatch/lib/private/metric-util.ts | 6 ++ .../aws-cloudwatch/lib/private/rendering.ts | 2 + .../test/cross-environment.test.ts | 79 +++++++++++++++++++ 7 files changed, 183 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/README.md b/packages/@aws-cdk/aws-cloudwatch/README.md index bdcf58e61024f..450fc8ca9821a 100644 --- a/packages/@aws-cdk/aws-cloudwatch/README.md +++ b/packages/@aws-cdk/aws-cloudwatch/README.md @@ -43,7 +43,7 @@ const metric = new Metric({ dimensionsMap: { HostedZoneId: hostedZone.hostedZoneId } -}) +}); ``` ### Instantiating a new Metric object @@ -73,7 +73,7 @@ const allProblems = new MathExpression({ errors: myConstruct.metricErrors(), faults: myConstruct.metricFaults(), } -}) +}); ``` You can use `MathExpression` objects like any other metric, including using @@ -86,9 +86,25 @@ const problemPercentage = new MathExpression({ problems: allProblems, invocations: myConstruct.metricInvocations() } -}) +}); +``` + +### Search Expressions + +Math expressions also support search expressions. For example, the following +search expression returns all CPUUtilization metrics that it finds, with the +graph showing the Average statistic with an aggregation period of 5 minutes: + +```ts +const cpuUtilization = new MathExpression({ + expression: "SEARCH('{AWS/EC2,InstanceId} MetricName=\"CPUUtilization\"', 'Average', 300)" +}); ``` +Cross-account and cross-region search expressions are also supported. Use +the `searchAccount` and `searchRegion` properties to specify the account +and/or region to evaluate the search expression against. + ### Aggregation To graph or alarm on metrics you must aggregate them first, using a function diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index e0692478c8457..e09b5d714af69 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -334,6 +334,8 @@ export class Alarm extends AlarmBase { assertSubmetricsCount(expr); } + self.validateMetricExpression(expr); + return { expression: expr.expression, id: entry.id || uniqueMetricId(), @@ -358,6 +360,16 @@ export class Alarm extends AlarmBase { throw new Error(`Cannot create an Alarm in region '${stack.region}' based on metric '${metric}' in '${stat.region}'`); } } + + /** + * Validates that the expression config does not specify searchAccount or searchRegion props + * as search expressions are not supported by Alarms. + */ + private validateMetricExpression(expr: MetricExpressionConfig) { + if (expr.searchAccount !== undefined || expr.searchRegion !== undefined) { + throw new Error('Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion'); + } + } } function definitelyDifferent(x: string | undefined, y: string) { diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts index 296400ee7f910..e8506fde53140 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts @@ -322,6 +322,20 @@ export interface MetricExpressionConfig { * How many seconds to aggregate over */ readonly period: number; + + /** + * Account to evaluate search expressions within. + * + * @default - Deployment account. + */ + readonly searchAccount?: string; + + /** + * Region to evaluate search expressions within. + * + * @default - Deployment region. + */ + readonly searchRegion?: string; } /** diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts index cf4051f74ffd4..d306978c93733 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts @@ -149,6 +149,26 @@ export interface MathExpressionOptions { * @default Duration.minutes(5) */ readonly period?: cdk.Duration; + + /** + * Account to evaluate search expressions within. + * + * Specifying a searchAccount has no effect to the account used + * for metrics within the expression (passed via usingMetrics). + * + * @default - Deployment account. + */ + readonly searchAccount?: string; + + /** + * Region to evaluate search expressions within. + * + * Specifying a searchRegion has no effect to the region used + * for metrics within the expression (passed via usingMetrics). + * + * @default - Deployment region. + */ + readonly searchRegion?: string; } /** @@ -157,6 +177,9 @@ export interface MathExpressionOptions { export interface MathExpressionProps extends MathExpressionOptions { /** * The expression defining the metric. + * + * When an expression contains a SEARCH function, it cannot be used + * within an Alarm. */ readonly expression: string; @@ -165,8 +188,10 @@ export interface MathExpressionProps extends MathExpressionOptions { * * The key is the identifier that represents the given metric in the * expression, and the value is the actual Metric object. + * + * @default - Empty map. */ - readonly usingMetrics: Record; + readonly usingMetrics?: Record; } /** @@ -451,6 +476,10 @@ function asString(x?: unknown): string | undefined { * It makes sense to embed this in here, so that compound constructs can attach * that metadata to metrics they expose. * + * MathExpression can also be used for search expressions. In this case, + * it also optionally accepts a searchRegion and searchAccount property for cross-environment + * search expressions. + * * This class does not represent a resource, so hence is not a construct. Instead, * MathExpression is an abstraction that makes it easy to specify metrics for use in both * alarms and graphs. @@ -482,14 +511,26 @@ export class MathExpression implements IMetric { */ public readonly period: cdk.Duration; + /** + * Account to evaluate search expressions within. + */ + public readonly searchAccount?: string; + + /** + * Region to evaluate search expressions within. + */ + public readonly searchRegion?: string; + constructor(props: MathExpressionProps) { this.period = props.period || cdk.Duration.minutes(5); this.expression = props.expression; - this.usingMetrics = changeAllPeriods(props.usingMetrics, this.period); + this.usingMetrics = changeAllPeriods(props.usingMetrics ?? {}, this.period); this.label = props.label; this.color = props.color; + this.searchAccount = props.searchAccount; + this.searchRegion = props.searchRegion; - const invalidVariableNames = Object.keys(props.usingMetrics).filter(x => !validVariableName(x)); + const invalidVariableNames = Object.keys(this.usingMetrics).filter(x => !validVariableName(x)); if (invalidVariableNames.length > 0) { throw new Error(`Invalid variable names in expression: ${invalidVariableNames}. Must start with lowercase letter and only contain alphanumerics.`); } @@ -508,7 +549,9 @@ export class MathExpression implements IMetric { // Short-circuit creating a new object if there would be no effective change if ((props.label === undefined || props.label === this.label) && (props.color === undefined || props.color === this.color) - && (props.period === undefined || props.period.toSeconds() === this.period.toSeconds())) { + && (props.period === undefined || props.period.toSeconds() === this.period.toSeconds()) + && (props.searchAccount === undefined || props.searchAccount === this.searchAccount) + && (props.searchRegion === undefined || props.searchRegion === this.searchRegion)) { return this; } @@ -518,6 +561,8 @@ export class MathExpression implements IMetric { label: ifUndefined(props.label, this.label), color: ifUndefined(props.color, this.color), period: ifUndefined(props.period, this.period), + searchAccount: ifUndefined(props.searchAccount, this.searchAccount), + searchRegion: ifUndefined(props.searchRegion, this.searchRegion), }); } @@ -541,6 +586,8 @@ export class MathExpression implements IMetric { period: this.period.toSeconds(), expression: this.expression, usingMetrics: this.usingMetrics, + searchAccount: this.searchAccount, + searchRegion: this.searchRegion, }, renderingProperties: { label: this.label, diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/private/metric-util.ts b/packages/@aws-cdk/aws-cloudwatch/lib/private/metric-util.ts index 589e3d99ad474..8988148c16b3b 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/private/metric-util.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/private/metric-util.ts @@ -25,6 +25,12 @@ export function metricKey(metric: IMetric): string { parts.push(id); parts.push(metricKey(conf.mathExpression.usingMetrics[id])); } + if (conf.mathExpression.searchRegion) { + parts.push(conf.mathExpression.searchRegion); + } + if (conf.mathExpression.searchAccount) { + parts.push(conf.mathExpression.searchAccount); + } } if (conf.metricStat) { parts.push(conf.metricStat.namespace); diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/private/rendering.ts b/packages/@aws-cdk/aws-cloudwatch/lib/private/rendering.ts index 8553d9ad5c486..9223695a96c67 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/private/rendering.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/private/rendering.ts @@ -55,6 +55,8 @@ function metricGraphJson(metric: IMetric, yAxis?: string, id?: string) { withExpression(expr) { options.expression = expr.expression; + if (expr.searchAccount) { options.accountId = accountIfDifferentFromStack(expr.searchAccount); } + if (expr.searchRegion) { options.region = regionIfDifferentFromStack(expr.searchRegion); } if (expr.period && expr.period !== 300) { options.period = expr.period; } }, }); 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 50278d9a83d54..668807c89bfef 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/cross-environment.test.ts @@ -78,6 +78,35 @@ describe('cross environment', () => { }); + + test('math expressions with explicit account and region will render in environment agnostic stack', () => { + // GIVEN + const expression = 'SEARCH(\'MetricName="ACount"\', \'Sum\', 300)'; + + const b = new MathExpression({ + expression, + usingMetrics: {}, + label: 'Test label', + searchAccount: '5678', + searchRegion: 'mars', + }); + + const graph = new GraphWidget({ + left: [ + b, + ], + }); + + // THEN + graphMetricsAre(new Stack(), graph, [ + [{ + expression, + accountId: '5678', + region: 'mars', + label: 'Test label', + }], + ]); + }); }); describe('in alarms', () => { @@ -234,6 +263,56 @@ describe('cross environment', () => { ], }); }); + + test('math expression with different searchAccount will throw', () => { + // GIVEN + const b = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: '1234', + }); + + const c = new MathExpression({ + expression: 'a + b', + usingMetrics: { a: a.attachTo(stack3), b }, + period: Duration.minutes(1), + searchAccount: '5678', + }); + + // THEN + expect(() => { + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: c, + }); + }).toThrow(/Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion/); + }); + + test('match expression with different searchRegion will throw', () => { + // GIVEN + const b = new Metric({ + namespace: 'Test', + metricName: 'ACount', + account: '1234', + }); + + const c = new MathExpression({ + expression: 'a + b', + usingMetrics: { a: a.attachTo(stack3), b }, + period: Duration.minutes(1), + searchRegion: 'mars', + }); + + // THEN + expect(() => { + new Alarm(stack1, 'Alarm', { + threshold: 1, + evaluationPeriods: 1, + metric: c, + }); + }).toThrow(/Cannot create an Alarm based on a MathExpression which specifies a searchAccount or searchRegion/); + }); }); });