Skip to content

Commit adc1a13

Browse files
authored
fix(cloudwatch): p100 statistic is no longer recognized (#24981)
Since #23095 (which was released in 2.66.0), the `p100` statistic is no longer rendered into Alarms. Fixes #24976. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 6171101 commit adc1a13

File tree

4 files changed

+86
-92
lines changed

4 files changed

+86
-92
lines changed

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

+10-1
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,19 @@ function renderIfExtendedStatistic(statistic?: string): string | undefined {
422422
if (statistic === undefined) { return undefined; }
423423

424424
const parsed = parseStatistic(statistic);
425+
if (parsed.type === 'simple') {
426+
// This statistic will have been rendered by renderIfSimpleStatistic
427+
return undefined;
428+
}
429+
425430
if (parsed.type === 'single' || parsed.type === 'pair') {
426431
return normalizeStatistic(parsed);
427432
}
428-
return undefined;
433+
434+
// We can't not render anything here. Just put whatever we got as input into
435+
// the ExtendedStatistic and hope it's correct. Either that, or we throw
436+
// an error.
437+
return parsed.statistic;
429438
}
430439

431440
function mathExprHasSubmetrics(expr: MetricExpressionConfig) {

packages/aws-cdk-lib/aws-cloudwatch/lib/private/statistic.ts

+49-88
Original file line numberDiff line numberDiff line change
@@ -60,118 +60,79 @@ function parseSingleStatistic(statistic: string, prefix: string): Omit<SingleSta
6060
return undefined;
6161
}
6262

63-
let r: RegExpExecArray | null = null;
63+
// A decimal positive number regex (1, 1.2, 99.999, etc)
64+
const reDecimal = '\\d+(?:\\.\\d+)?';
6465

6566
// p99.99
66-
// /^p(\d{1,2}(?:\.\d+)?)$/
67-
r = new RegExp(`^${prefixLower}(\\d{1,2}(?:\\.\\d+)?)$`).exec(statistic);
68-
if (r) {
69-
return {
70-
type: 'single',
71-
rawStatistic: statistic,
72-
statPrefix: prefixLower,
73-
value: parseFloat(r[1]),
74-
};
67+
// /^p(\d+(?:\.\d+)?)$/
68+
const r = new RegExp(`^${prefixLower}(${reDecimal})$`).exec(statistic);
69+
if (!r) {
70+
return undefined;
7571
}
7672

77-
return undefined;
73+
const value = parseFloat(r[1]);
74+
if (value < 0 || value > 100) {
75+
return undefined;
76+
}
77+
return {
78+
type: 'single',
79+
rawStatistic: statistic,
80+
statPrefix: prefixLower,
81+
value,
82+
};
7883
}
7984

85+
/**
86+
* Parse a statistic that looks like `tm( LOWER : UPPER )`.
87+
*/
8088
function parsePairStatistic(statistic: string, prefix: string): Omit<PairStatistic, 'statName'> | undefined {
81-
const prefixUpper = prefix.toUpperCase();
82-
83-
// Allow `tm(10%:90%)` lowercase
84-
statistic = statistic.toUpperCase();
85-
86-
if (!statistic.startsWith(prefixUpper)) {
89+
const r = new RegExp(`^${prefix}\\(([^)]+)\\)$`, 'i').exec(statistic);
90+
if (!r) {
8791
return undefined;
8892
}
8993

9094
const common: Omit<PairStatistic, 'statName' | 'isPercent'> = {
9195
type: 'pair',
9296
canBeSingleStat: false,
9397
rawStatistic: statistic,
94-
statPrefix: prefixUpper,
98+
statPrefix: prefix.toUpperCase(),
9599
};
96100

97-
let r: RegExpExecArray | null = null;
98-
99-
// TM(99.999:)
100-
// /TM\((\d{1,2}(?:\.\d+)?):\)/
101-
r = new RegExp(`^${prefixUpper}\\((\\d+(?:\\.\\d+)?)\\:\\)$`).exec(statistic);
102-
if (r) {
103-
return {
104-
...common,
105-
lower: parseFloat(r[1]),
106-
upper: undefined,
107-
isPercent: false,
108-
};
109-
}
110-
111-
// TM(99.999%:)
112-
// /TM\((\d{1,2}(?:\.\d+)?)%:\)/
113-
r = new RegExp(`^${prefixUpper}\\((\\d{1,2}(?:\\.\\d+)?)%\\:\\)$`).exec(statistic);
114-
if (r) {
115-
return {
116-
...common,
117-
lower: parseFloat(r[1]),
118-
upper: undefined,
119-
isPercent: true,
120-
};
101+
const [lhs, rhs] = r[1].split(':');
102+
if (rhs === undefined) {
103+
// Doesn't have 2 parts
104+
return undefined;
121105
}
122106

123-
// TM(:99.999)
124-
// /TM\(:(\d{1,2}(?:\.\d+)?)\)/
125-
r = new RegExp(`^${prefixUpper}\\(\\:(\\d+(?:\\.\\d+)?)\\)$`).exec(statistic);
126-
if (r) {
127-
return {
128-
...common,
129-
lower: undefined,
130-
upper: parseFloat(r[1]),
131-
isPercent: false,
132-
};
133-
}
107+
const parseNumberAndPercent = (x: string): [number | undefined | 'fail', boolean] => {
108+
x = x.trim();
109+
if (!x) {
110+
return [undefined, false];
111+
}
112+
const value = parseFloat(x.replace(/%$/, ''));
113+
const percent = x.endsWith('%');
114+
if (isNaN(value) || value < 0 || (percent && value > 100)) {
115+
return ['fail', false];
116+
}
117+
return [value, percent];
118+
};
134119

135-
// TM(:99.999%)
136-
// /TM\(:(\d{1,2}(?:\.\d+)?)%\)/
137-
// Note: this can be represented as a single stat! TM(:90%) = tm90
138-
r = new RegExp(`^${prefixUpper}\\(\\:(\\d{1,2}(?:\\.\\d+)?)%\\)$`).exec(statistic);
139-
if (r) {
140-
return {
141-
...common,
142-
canBeSingleStat: true,
143-
asSingleStatStr: `${prefix.toLowerCase()}${r[1]}`,
144-
lower: undefined,
145-
upper: parseFloat(r[1]),
146-
isPercent: true,
147-
};
120+
const [lower, lhsPercent] = parseNumberAndPercent(lhs);
121+
const [upper, rhsPercent] = parseNumberAndPercent(rhs);
122+
if (lower === 'fail' || upper === 'fail' || (lower === undefined && upper === undefined)) {
123+
return undefined;
148124
}
149125

150-
// TM(99.999:99.999)
151-
// /TM\((\d{1,2}(?:\.\d+)?):(\d{1,2}(?:\.\d+)?)\)/
152-
r = new RegExp(`^${prefixUpper}\\((\\d+(?:\\.\\d+)?)\\:(\\d+(?:\\.\\d+)?)\\)$`).exec(statistic);
153-
if (r) {
154-
return {
155-
...common,
156-
lower: parseFloat(r[1]),
157-
upper: parseFloat(r[2]),
158-
isPercent: false,
159-
};
126+
if (lower !== undefined && upper !== undefined && lhsPercent !== rhsPercent) {
127+
// If one value is a percentage, the other one must be too
128+
return undefined;
160129
}
161130

162-
// TM(99.999%:99.999%)
163-
// /TM\((\d{1,2}(?:\.\d+)?)%:(\d{1,2}(?:\.\d+)?)%\)/
164-
r = new RegExp(`^${prefixUpper}\\((\\d{1,2}(?:\\.\\d+)?)%\\:(\\d{1,2}(?:\\.\\d+)?)%\\)$`).exec(statistic);
165-
if (r) {
166-
return {
167-
...common,
168-
lower: parseFloat(r[1]),
169-
upper: parseFloat(r[2]),
170-
isPercent: true,
171-
};
172-
}
131+
const isPercent = lhsPercent || rhsPercent;
132+
const canBeSingleStat = lower === undefined && isPercent;
133+
const asSingleStatStr = canBeSingleStat ? `${prefix.toLowerCase()}${upper}` : undefined;
173134

174-
return undefined;
135+
return { ...common, lower, upper, isPercent, canBeSingleStat, asSingleStatStr };
175136
}
176137

177138
export function singleStatisticToString(parsed: SingleStatistic): string {

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

+24-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
import { Construct } from 'constructs';
12
import { Match, Template, Annotations } from '../../assertions';
23
import { Duration, Stack } from '../../core';
3-
import { Construct } from 'constructs';
44
import { Alarm, IAlarm, IAlarmAction, Metric, MathExpression, IMetric, Stats } from '../lib';
55

66
const testMetric = new Metric({
@@ -359,6 +359,29 @@ describe('Alarm', () => {
359359
const template = Annotations.fromStack(stack);
360360
template.hasWarning('/MyStack/MyAlarm', Match.stringLikeRegexp("Math expression 'oops' references unknown identifiers"));
361361
});
362+
363+
test('check alarm for p100 statistic', () => {
364+
const stack = new Stack(undefined, 'MyStack');
365+
new Alarm(stack, 'MyAlarm', {
366+
metric: new Metric({
367+
dimensionsMap: {
368+
Boop: 'boop',
369+
},
370+
metricName: 'MyMetric',
371+
namespace: 'MyNamespace',
372+
period: Duration.minutes(1),
373+
statistic: Stats.p(100),
374+
}),
375+
evaluationPeriods: 1,
376+
threshold: 1,
377+
});
378+
379+
// THEN
380+
const template = Template.fromStack(stack);
381+
template.hasResourceProperties('AWS::CloudWatch::Alarm', {
382+
ExtendedStatistic: 'p100',
383+
});
384+
});
362385
});
363386

364387
class TestAlarmAction implements IAlarmAction {

packages/aws-cdk-lib/aws-cloudwatch/test/metrics.test.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ describe('Metrics', () => {
288288
checkParsingSingle('p99', 'p', 'percentile', 99);
289289
checkParsingSingle('P99', 'p', 'percentile', 99);
290290
checkParsingSingle('p99.99', 'p', 'percentile', 99.99);
291+
checkParsingSingle('p100', 'p', 'percentile', 100);
291292
checkParsingSingle('tm99', 'tm', 'trimmedMean', 99);
292293
checkParsingSingle('wm99', 'wm', 'winsorizedMean', 99);
293294
checkParsingSingle('tc99', 'tc', 'trimmedCount', 99);
@@ -337,8 +338,8 @@ describe('Metrics', () => {
337338
expect(parseStatistic('TM(10%:1500)').type).toEqual('generic');
338339
expect(parseStatistic('TM(10)').type).toEqual('generic');
339340
expect(parseStatistic('TM()').type).toEqual('generic');
340-
expect(parseStatistic('TM(0.:)').type).toEqual('generic');
341-
expect(parseStatistic('TM(:0.)').type).toEqual('generic');
341+
expect(parseStatistic('TM(0.:)').type).toEqual('pair');
342+
expect(parseStatistic('TM(:0.)').type).toEqual('pair');
342343
expect(parseStatistic('()').type).toEqual('generic');
343344
expect(parseStatistic('(:)').type).toEqual('generic');
344345
expect(parseStatistic('TM(:)').type).toEqual('generic');

0 commit comments

Comments
 (0)