Skip to content

Commit c29dc17

Browse files
kaizencckaizencc
authored
fix(cloudwatch): remove false positive warning for CDK tokens in MathExpression (#36882)
### Issue # (if applicable) Closes #34977. ### Reason for this change When using CDK tokens (from custom resources or other dynamic values) in CloudWatch MathExpression, users receive a false positive warning message. The `allIdentifiersInExpression` function incorrectly parses token strings like `${Token[TOKEN.81]}` and extracts "oken" as an identifier, triggering a confusing warning that suggests adding "oken" to the `usingMetrics` map. This degrades user experience and creates confusion, particularly for users leveraging custom resources or dynamic values in CloudWatch dashboards. ### Description of changes Modified the `allIdentifiersInExpression` function in `packages/aws-cdk-lib/aws-cloudwatch/lib/metric.ts` to filter out CDK token patterns before extracting metric identifiers: - Added regex replacement to remove CDK token patterns (`${Token[TOKEN.N]}`) from expressions before identifier extraction - Token pattern regex: `/\$\{Token\[[^\]]+\]\}/g` matches the standard CDK token format - Preserves existing validation behavior for actual unknown identifiers - Added 4 comprehensive unit tests covering single tokens, multiple tokens, mixed expressions, and complex patterns **Technical Details:** ```typescript // Before: Incorrectly extracted "oken" from "${Token[TOKEN.81]}" function allIdentifiersInExpression(x: string) { return Array.from(matchAll(x, FIND_VARIABLE)).map(m => m[0]); } // After: Filters out tokens first, then extracts identifiers function allIdentifiersInExpression(x: string) { // Remove CDK token patterns before extracting identifiers // Token format: ${Token[TOKEN.123]} or ${Token[TOKEN.456]} const withoutTokens = x.replace(/\$\{Token\[[^\]]+\]\}/g, ''); return Array.from(matchAll(withoutTokens, FIND_VARIABLE)).map(m => m[0]); } ``` This approach follows the same pattern used for special functions (SEARCH, METRICS, SELECT, INSIGHT_RULE_METRIC) where certain syntax patterns are filtered or handled specially before validation. **No breaking changes** - This fix only removes false positive warnings and does not affect CloudFormation template generation, resource behavior, or public APIs. ### Describe any new or updated permissions being added N/A - No IAM permissions or resource access changes. This is a validation-only fix that does not affect CloudFormation templates or runtime behavior. ### Description of how you validated changes - **Unit tests**: Added 4 new unit tests in `packages/aws-cdk-lib/aws-cloudwatch/test/metric-math.test.ts`: 1. Verified single token in expression doesn't produce warning 2. Verified multiple tokens in expression don't produce warning 3. Verified mixed token + real identifier warns only about the real identifier (not "oken") 4. Verified complex token patterns (arrays, functions) don't produce warning - **Regression testing**: All 40 existing unit tests pass without modification, confirming: - Warnings for actual unknown identifiers still work correctly - Special function handling (METRICS, SEARCH, etc.) unchanged - Nested expression validation preserved - No impact on other CloudWatch functionality - **Build validation**: - Linting passed with no violations (`yarn lint --fix`) - TypeScript compilation successful - No JSII compatibility issues ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) [comment]: <> (cdk-contribution-power preview)
1 parent dedf99b commit c29dc17

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,10 @@ function validVariableName(x: string) {
11401140
* Return all variable names used in an expression
11411141
*/
11421142
function allIdentifiersInExpression(x: string) {
1143-
return Array.from(matchAll(x, FIND_VARIABLE)).map(m => m[0]);
1143+
// Remove CDK token patterns before extracting identifiers
1144+
// Token format: ${Token[TOKEN.123]} or ${Token[TOKEN.456]}
1145+
const withoutTokens = x.replace(/\$\{Token\[[^\]]+\]\}/g, '');
1146+
return Array.from(matchAll(withoutTokens, FIND_VARIABLE)).map(m => m[0]);
11441147
}
11451148

11461149
/**

‎packages/aws-cdk-lib/aws-cloudwatch/test/metric-math.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,48 @@ describe('Metric Math', () => {
110110
});
111111
});
112112

113+
test('math expression with CDK token does not produce warning', () => {
114+
const token = Lazy.string({ produce: () => 'metric1' });
115+
const m = new MathExpression({
116+
expression: `TIME_SERIES(${token})`,
117+
});
118+
119+
expect(m.warningsV2).toBeUndefined();
120+
});
121+
122+
test('math expression with multiple CDK tokens does not produce warning', () => {
123+
const token1 = Lazy.string({ produce: () => 'metric1' });
124+
const token2 = Lazy.string({ produce: () => 'metric2' });
125+
const m = new MathExpression({
126+
expression: `${token1} + ${token2}`,
127+
});
128+
129+
expect(m.warningsV2).toBeUndefined();
130+
});
131+
132+
test('math expression with CDK token and real identifier warns only about real identifier', () => {
133+
const token = Lazy.string({ produce: () => 'metric1' });
134+
const m = new MathExpression({
135+
expression: `m1 + ${token}`,
136+
});
137+
138+
expect(m.warningsV2).toMatchObject({
139+
'CloudWatch:Math:UnknownIdentifier': expect.stringContaining('references unknown identifiers: m1'),
140+
});
141+
// Should NOT mention "oken" as an unknown identifier (only "m1" should be listed)
142+
expect(m.warningsV2?.['CloudWatch:Math:UnknownIdentifier']).not.toMatch(/identifiers:.*oken/);
143+
});
144+
145+
test('math expression with complex token pattern does not produce warning', () => {
146+
const token1 = Lazy.string({ produce: () => 'metric1' });
147+
const token2 = Lazy.string({ produce: () => 'metric2' });
148+
const m = new MathExpression({
149+
expression: `SUM([${token1}, ${token2}])`,
150+
});
151+
152+
expect(m.warningsV2).toBeUndefined();
153+
});
154+
113155
test('metrics METRICS expression does not produce warning for unknown identifier', () => {
114156
const m = new MathExpression({
115157
expression: 'SUM(METRICS())',

0 commit comments

Comments
 (0)