Skip to content

Commit

Permalink
fix(aws-cloudwatch): unable to use generic extended statistics for cl…
Browse files Browse the repository at this point in the history
…oudwatch alarms (#15720)

This change adds support for alarming on custom statistics and extends the testing done for metrics to ensure if a custom statistic is passed it preserves the case


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
filipay authored Jul 22, 2021
1 parent 2c4ef01 commit f593311
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 5 deletions.
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,10 @@ function renderIfExtendedStatistic(statistic?: string): string | undefined {
// Already percentile. Avoid parsing because we might get into
// floating point rounding issues, return as-is but lowercase the p.
return statistic.toLowerCase();
} else if (parsed.type === 'generic') {
return statistic;
}

return undefined;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/private/statistic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ export function parseStatistic(stat: string): SimpleStatistic | PercentileStatis

return {
type: 'generic',
statistic: lowerStat,
statistic: stat,
};
}

export function normalizeStatistic(stat: string): string {
const parsed = parseStatistic(stat);
if (parsed.type === 'simple') {
if (parsed.type === 'simple' || parsed.type === 'generic') {
return parsed.statistic;
} else {
// Already percentile. Avoid parsing because we might get into
Expand Down
20 changes: 20 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,26 @@ describe('Alarm', () => {


});

test('can use a generic string for extended statistic to make alarm', () => {
// GIVEN
const stack = new Stack();

// WHEN
testMetric.createAlarm(stack, 'Alarm', {
threshold: 1000,
evaluationPeriods: 2,
statistic: 'tm99.9999999999',
});

// THEN
expect(stack).toHaveResource('AWS::CloudWatch::Alarm', {
Statistic: ABSENT,
ExtendedStatistic: 'tm99.9999999999',
});

});

});

class TestAlarmAction implements IAlarmAction {
Expand Down
7 changes: 4 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/test/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,13 @@ describe('Metrics', () => {
});

test('metric accepts a variety of statistics', () => {
new Metric({
const customStat = 'myCustomStatistic';
const metric = new Metric({
namespace: 'Test',
metricName: 'Metric',
statistic: 'myCustomStatistic',
statistic: customStat,
});


expect(metric.statistic).toEqual(customStat);
});
});

0 comments on commit f593311

Please sign in to comment.