Skip to content

Commit 853e3d6

Browse files
authored
feat(cloudwatch): parse all metrics statistics and support long format (#23095)
### Description Previous PR added support for missing statistics #23074 This PR implements a proper parsing of all these statistics. - Support "short" format `ts99` - Support "long" format - `TS(10%:90%)` | `TS(10:90)` - `TS(:90)` | `TS(:90%)` - `TS(10:)` | `TS(10%:)` - Formats are case insensitive (no breaking changes) - If "long" format and only upper boundary% `TS(:90%)`, can be translated to "short" format `ts90` (`stat.asSingleStatStr`) ### Note I noticed that the following code expected the parsing to throw if it failed, but it actually will **not** fail in any case (it just return `GenericStatistic` if no format matched). I will **not** change this behavior here as I'm not willing to spend more effort testing if this breaks stuff elsewhere. https://github.com/aws/aws-cdk/blob/47943d206c8ff28923e19028acd5991d8e387ac9/packages/%40aws-cdk/aws-cloudwatch/lib/metric.ts#L295-L296 ### Followup work As is, this PR does not change any customer-facing logic. To make use of it, make the parsing throw if no format is recognized. At the end of the parser function, just replace ```ts return { type: 'generic', statistic: stat, } as GenericStatistic; ``` with ```ts throw new UnrecognizedStatisticFormatError() ``` --- You can see all tested inputs here: https://regexr.com/7351s ![2022-11-24_15-52-57](https://user-images.githubusercontent.com/26366184/204067982-05b7bd4f-1c56-4466-8c97-c626e8ac31b2.png) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 5f33a26 commit 853e3d6

12 files changed

+795
-98
lines changed

packages/@aws-cdk/aws-cloudwatch/README.md

+54
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,60 @@ below).
166166
> happen to know the Metric you want to alarm on makes sense as a rate
167167
> (`Average`) you can always choose to change the statistic.
168168
169+
### Available Aggregation Statistics
170+
171+
For your metrics aggregation, you can use the following statistics:
172+
173+
| Statistic | Short format | Long format | Factory name |
174+
| ------------------------ | :-----------------: | :------------------------------------------: | -------------------- |
175+
| SampleCount (n) ||| `Stats.SAMPLE_COUNT` |
176+
| Average (avg) ||| `Stats.AVERAGE` |
177+
| Sum ||| `Stats.SUM` |
178+
| Minimum (min) ||| `Stats.MINIMUM` |
179+
| Maximum (max) ||| `Stats.MAXIMUM` |
180+
| Interquartile mean (IQM) ||| `Stats.IQM` |
181+
| Percentile (p) | `p99` || `Stats.p(99)` |
182+
| Winsorized mean (WM) | `wm99` = `WM(:99%)` | `WM(x:y) \| WM(x%:y%) \| WM(x%:) \| WM(:y%)` | `Stats.wm(10, 90)` |
183+
| Trimmed count (TC) | `tc99` = `TC(:99%)` | `TC(x:y) \| TC(x%:y%) \| TC(x%:) \| TC(:y%)` | `Stats.tc(10, 90)` |
184+
| Trimmed sum (TS) | `ts99` = `TS(:99%)` | `TS(x:y) \| TS(x%:y%) \| TS(x%:) \| TS(:y%)` | `Stats.ts(10, 90)` |
185+
| Percentile rank (PR) || `PR(x:y) \| PR(x:) \| PR(:y)` | `Stats.pr(10, 5000)` |
186+
187+
The most common values are provided in the `cloudwatch.Stats` class. You can provide any string if your statistic is not in the class.
188+
189+
Read more at [CloudWatch statistics definitions](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Statistics-definitions.html).
190+
191+
```ts
192+
new cloudwatch.Metric({
193+
namespace: 'AWS/Route53',
194+
metricName: 'DNSQueries',
195+
dimensionsMap: {
196+
HostedZoneId: hostedZone.hostedZoneId
197+
},
198+
statistic: cloudwatch.Stats.SAMPLE_COUNT,
199+
period: cloudwatch.Duration.minutes(5)
200+
});
201+
202+
new cloudwatch.Metric({
203+
namespace: 'AWS/Route53',
204+
metricName: 'DNSQueries',
205+
dimensionsMap: {
206+
HostedZoneId: hostedZone.hostedZoneId
207+
},
208+
statistic: cloudwatch.Stats.p(99),
209+
period: cloudwatch.Duration.minutes(5)
210+
});
211+
212+
new cloudwatch.Metric({
213+
namespace: 'AWS/Route53',
214+
metricName: 'DNSQueries',
215+
dimensionsMap: {
216+
HostedZoneId: hostedZone.hostedZoneId
217+
},
218+
statistic: 'TS(7.5%:90%)',
219+
period: cloudwatch.Duration.minutes(5)
220+
});
221+
```
222+
169223
### Labels
170224

171225
Metric labels are displayed in the legend of graphs that include the metrics.

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

+4-9
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { IMetric, MetricExpressionConfig, MetricStatConfig } from './metric-type
99
import { dispatchMetric, metricPeriod } from './private/metric-util';
1010
import { dropUndefined } from './private/object';
1111
import { MetricSet } from './private/rendering';
12-
import { parseStatistic } from './private/statistic';
12+
import { normalizeStatistic, parseStatistic } from './private/statistic';
1313

1414
/**
1515
* Properties for Alarms
@@ -413,7 +413,7 @@ function renderIfSimpleStatistic(statistic?: string): string | undefined {
413413

414414
const parsed = parseStatistic(statistic);
415415
if (parsed.type === 'simple') {
416-
return parsed.statistic;
416+
return normalizeStatistic(parsed);
417417
}
418418
return undefined;
419419
}
@@ -422,14 +422,9 @@ function renderIfExtendedStatistic(statistic?: string): string | undefined {
422422
if (statistic === undefined) { return undefined; }
423423

424424
const parsed = parseStatistic(statistic);
425-
if (parsed.type === 'percentile') {
426-
// Already percentile. Avoid parsing because we might get into
427-
// floating point rounding issues, return as-is but lowercase the p.
428-
return statistic.toLowerCase();
429-
} else if (parsed.type === 'generic') {
430-
return statistic;
425+
if (parsed.type === 'single' || parsed.type === 'pair') {
426+
return normalizeStatistic(parsed);
431427
}
432-
433428
return undefined;
434429
}
435430

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

+40-18
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ import * as iam from '@aws-cdk/aws-iam';
22
import * as cdk from '@aws-cdk/core';
33
import { Construct, IConstruct } from 'constructs';
44
import { Alarm, ComparisonOperator, TreatMissingData } from './alarm';
5-
import { Dimension, IMetric, MetricAlarmConfig, MetricConfig, MetricGraphConfig, Unit } from './metric-types';
5+
import { Dimension, IMetric, MetricAlarmConfig, MetricConfig, MetricGraphConfig, Statistic, Unit } from './metric-types';
66
import { dispatchMetric, metricKey } from './private/metric-util';
7-
import { normalizeStatistic, parseStatistic } from './private/statistic';
7+
import { normalizeStatistic, pairStatisticToString, parseStatistic, singleStatisticToString } from './private/statistic';
8+
import { Stats } from './stats';
89

9-
export type DimensionHash = {[dim: string]: any};
10+
export type DimensionHash = { [dim: string]: any };
1011

1112
export type DimensionsMap = { [dim: string]: string };
1213

@@ -24,6 +25,8 @@ export interface CommonMetricOptions {
2425
/**
2526
* What function to use for aggregating.
2627
*
28+
* Use the `aws_cloudwatch.Stats` helper class to construct valid input strings.
29+
*
2730
* Can be one of the following:
2831
*
2932
* - "Minimum" | "min"
@@ -38,8 +41,6 @@ export interface CommonMetricOptions {
3841
* - "tcNN.NN" | "tc(NN.NN%:NN.NN%)"
3942
* - "tsNN.NN" | "ts(NN.NN%:NN.NN%)"
4043
*
41-
* Use the factory functions on the `Stats` object to construct valid input strings.
42-
*
4344
* @default Average
4445
*/
4546
readonly statistic?: string;
@@ -197,13 +198,13 @@ export interface MathExpressionOptions {
197198
readonly searchAccount?: string;
198199

199200
/**
200-
* Region to evaluate search expressions within.
201-
*
202-
* Specifying a searchRegion has no effect to the region used
203-
* for metrics within the expression (passed via usingMetrics).
204-
*
205-
* @default - Deployment region.
206-
*/
201+
* Region to evaluate search expressions within.
202+
*
203+
* Specifying a searchRegion has no effect to the region used
204+
* for metrics within the expression (passed via usingMetrics).
205+
*
206+
* @default - Deployment region.
207+
*/
207208
readonly searchRegion?: string;
208209
}
209210

@@ -291,17 +292,30 @@ export class Metric implements IMetric {
291292
if (periodSec !== 1 && periodSec !== 5 && periodSec !== 10 && periodSec !== 30 && periodSec % 60 !== 0) {
292293
throw new Error(`'period' must be 1, 5, 10, 30, or a multiple of 60 seconds, received ${periodSec}`);
293294
}
295+
296+
this.warnings = undefined;
294297
this.dimensions = this.validateDimensions(props.dimensionsMap ?? props.dimensions);
295298
this.namespace = props.namespace;
296299
this.metricName = props.metricName;
297-
// Try parsing, this will throw if it's not a valid stat
298-
this.statistic = normalizeStatistic(props.statistic || 'Average');
300+
301+
const parsedStat = parseStatistic(props.statistic || Stats.AVERAGE);
302+
if (parsedStat.type === 'generic') {
303+
// Unrecognized statistic, do not throw, just warn
304+
// There may be a new statistic that this lib does not support yet
305+
const label = props.label ? `, label "${props.label}"`: '';
306+
this.warnings = [
307+
`Unrecognized statistic "${props.statistic}" for metric with namespace "${props.namespace}"${label} and metric name "${props.metricName}".` +
308+
' Preferably use the `aws_cloudwatch.Stats` helper class to specify a statistic.' +
309+
' You can ignore this warning if your statistic is valid but not yet supported by the `aws_cloudwatch.Stats` helper class.',
310+
];
311+
}
312+
this.statistic = normalizeStatistic(parsedStat);
313+
299314
this.label = props.label;
300315
this.color = props.color;
301316
this.unit = props.unit;
302317
this.account = props.account;
303318
this.region = props.region;
304-
this.warnings = undefined;
305319
}
306320

307321
/**
@@ -389,14 +403,22 @@ export class Metric implements IMetric {
389403
throw new Error('Using a math expression is not supported here. Pass a \'Metric\' object instead');
390404
}
391405

392-
const stat = parseStatistic(metricConfig.metricStat.statistic);
406+
const parsed = parseStatistic(metricConfig.metricStat.statistic);
407+
408+
let extendedStatistic: string | undefined = undefined;
409+
if (parsed.type === 'single') {
410+
extendedStatistic = singleStatisticToString(parsed);
411+
} else if (parsed.type === 'pair') {
412+
extendedStatistic = pairStatisticToString(parsed);
413+
}
414+
393415
return {
394416
dimensions: metricConfig.metricStat.dimensions,
395417
namespace: metricConfig.metricStat.namespace,
396418
metricName: metricConfig.metricStat.metricName,
397419
period: metricConfig.metricStat.period.toSeconds(),
398-
statistic: stat.type === 'simple' ? stat.statistic : undefined,
399-
extendedStatistic: stat.type === 'percentile' ? 'p' + stat.percentile : undefined,
420+
statistic: parsed.type === 'simple' ? parsed.statistic as Statistic : undefined,
421+
extendedStatistic,
400422
unit: this.unit,
401423
};
402424
}

0 commit comments

Comments
 (0)