Skip to content

Commit f3596eb

Browse files
authored
fix(cloudwatch): math expressions incorrectly warn about search and metrics (#24313)
Closes [#20136](#20136). 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 SEARCH and METRICS queries, we can refer directly to metrics attribute values inside the query. Therefore we should not raise warnings. Change made based on work done in 55108b9 with regex extended to a few other expressions. Looks like integ requests will not be required based on that commit. I had some firsthand experience getting thousands of this warning message after upgrading to CDK 2 and decided it would be easier to fix than suppress the excessive warnings. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 5a0c627 commit f3596eb

File tree

2 files changed

+20
-2
lines changed

2 files changed

+20
-2
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -609,9 +609,9 @@ export class MathExpression implements IMetric {
609609
// we can add warnings.
610610
const missingIdentifiers = allIdentifiersInExpression(this.expression).filter(i => !this.usingMetrics[i]);
611611

612-
const warnings = [];
612+
const warnings: string[] = [];
613613

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

packages/@aws-cdk/aws-cloudwatch/test/metric-math.test.ts

+18
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,24 @@ describe('Metric Math', () => {
7575
expect(m.warnings).toContainEqual(expect.stringContaining("'m1 + m2' references unknown identifiers"));
7676
});
7777

78+
test('metrics METRICS expression does not produce warning for unknown identifier', () => {
79+
const m = new MathExpression({
80+
expression: 'SUM(METRICS())',
81+
usingMetrics: {},
82+
});
83+
84+
expect(m.warnings).toBeUndefined();
85+
});
86+
87+
test('metrics search expression does not produce warning for unknown identifier', () => {
88+
const m = new MathExpression({
89+
expression: "SEARCH('{dimension_one, dimension_two} my_metric', 'Average', 300)",
90+
usingMetrics: {},
91+
});
92+
93+
expect(m.warnings).toBeUndefined();
94+
});
95+
7896
test('metrics insights expression does not produce warning for unknown identifier', () => {
7997
const m = new MathExpression({
8098
expression: "SELECT AVG(CpuUsage) FROM EC2 WHERE Instance = '123456'",

0 commit comments

Comments
 (0)