Skip to content

Commit

Permalink
fix(cloudwatch): math expressions incorrectly warn about metricsinsig…
Browse files Browse the repository at this point in the history
…hts variable names (#23316)

It is intended that all metric identifiers referenced in a MathExpression are included in the usingMetrics map and we will raise warnings if the customer does not follow this contract. 
However for Metricsinsights queries, we can refer directly to metrics attribute values inside the query. Therefore we do not raise warnings for Metricsinsights queries for not referencing metrics

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
caoxy98 authored Dec 12, 2022
1 parent 6a28b7f commit 55108b9
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ export class MathExpression implements IMetric {

const warnings = [];

if (missingIdentifiers.length > 0) {
if (!this.expression.toUpperCase().match('\\s*SELECT\\s.*') && missingIdentifiers.length > 0) {
warnings.push(`Math expression '${this.expression}' references unknown identifiers: ${missingIdentifiers.join(', ')}. Please add them to the 'usingMetrics' map.`);
}

Expand Down
8 changes: 8 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/test/metric-math.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ describe('Metric Math', () => {
expect(m.warnings).toContainEqual(expect.stringContaining("'m1 + m2' references unknown identifiers"));
});

test('metrics insights expression does not produce warning for unknown identifier', () => {
const m = new MathExpression({
expression: "SELECT AVG(CpuUsage) FROM EC2 WHERE Instance = '123456'",
});

expect(m.warnings).toBeUndefined();
});

test('math expression referring to unknown expressions produces a warning, even when nested', () => {
const m = new MathExpression({
expression: 'e1 + 5',
Expand Down

0 comments on commit 55108b9

Please sign in to comment.