Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cloudwatch): parse all metrics statistics and support long format #23095

Merged
merged 27 commits into from Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2aea37e
Correctly parse metric statistic
rigwild Nov 26, 2022
d1e5845
Update usages
rigwild Nov 26, 2022
45b20f0
Add statistics parsing tests
rigwild Nov 26, 2022
c2dc018
Explain statistics in README
rigwild Nov 26, 2022
0d3882b
Fix build fail
rigwild Nov 26, 2022
d5bd361
Lint errors
rigwild Nov 26, 2022
eb18f5e
Lint errors lmeow
rigwild Nov 26, 2022
019733a
Remove breaking change, allow case insensitive
rigwild Nov 26, 2022
1b90c57
make it work lmeow
rigwild Nov 27, 2022
1bbbf35
Add integration tests
rigwild Nov 27, 2022
be2e11e
Add missing IQM
rigwild Dec 1, 2022
d356ea6
Use `Stats` instead of private `Statistic` - no breaking changes
rigwild Dec 1, 2022
70a2418
Add stats class alarm tests
rigwild Dec 1, 2022
b77e5a6
Add integration tests cdk.out (it disappeared somehow)
rigwild Dec 1, 2022
7e7c42a
Log warning message if unrecognized stat
rigwild Dec 1, 2022
e52fc5e
Remove readme trailing comma
rigwild Dec 1, 2022
997be52
Fix tests
rigwild Dec 1, 2022
729d145
Merge branch 'main' into main
rigwild Jan 19, 2023
6e7f950
Merge branch 'main' into main
rigwild Feb 4, 2023
6654d01
Merge branch 'main' of github.com:aws/aws-cdk into main
rigwild Feb 13, 2023
34843f9
Use proper warning
rigwild Feb 13, 2023
3b4dc19
Add warning check test
rigwild Feb 13, 2023
79b0bea
Merge branch 'main' of github.com:rigwild/aws-cdk into main
rigwild Feb 13, 2023
7c57780
Merge branch 'main' into main
rigwild Feb 13, 2023
699261d
Update packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
rigwild Feb 20, 2023
05a16a8
Update packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
rigwild Feb 20, 2023
2391b6a
Merge branch 'main' into main
TheRealAmazonKendra Feb 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Expand Up @@ -292,6 +292,8 @@ export class Metric implements IMetric {
if (periodSec !== 1 && periodSec !== 5 && periodSec !== 10 && periodSec !== 30 && periodSec % 60 !== 0) {
throw new Error(`'period' must be 1, 5, 10, 30, or a multiple of 60 seconds, received ${periodSec}`);
}

this.warnings = undefined;
this.dimensions = this.validateDimensions(props.dimensionsMap ?? props.dimensions);
this.namespace = props.namespace;
this.metricName = props.metricName;
Expand All @@ -300,12 +302,12 @@ export class Metric implements IMetric {
if (parsedStat.type === 'generic') {
// Unrecognized statistic, do not throw, just warn
// There may be a new statistic that this lib does not support yet
// eslint-disable-next-line no-console
console.warn(
`WARNING: Unrecognized statistic "${props.statistic}" for metric with namespace "${props.namespace}", label "${props.label}" and metric name "${props.metricName}".` +
' Use the `aws_cloudwatch.Stats` helper class to specify a statistic.' +
const label = props.label ? ` label "${props.label}"`: '';
rigwild marked this conversation as resolved.
Show resolved Hide resolved
this.warnings = [
`Unrecognized statistic "${props.statistic}" for metric with namespace "${props.namespace}",${label} and metric name "${props.metricName}".` +
rigwild marked this conversation as resolved.
Show resolved Hide resolved
' Preferably use the `aws_cloudwatch.Stats` helper class to specify a statistic.' +
' You can ignore this warning if your statistic is valid but not yet supported by the `aws_cloudwatch.Stats` helper class.',
);
];
}
this.statistic = normalizeStatistic(parsedStat);

Expand All @@ -314,7 +316,6 @@ export class Metric implements IMetric {
this.unit = props.unit;
this.account = props.account;
this.region = props.region;
this.warnings = undefined;
}

/**
Expand Down
22 changes: 21 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts
Expand Up @@ -324,7 +324,27 @@ describe('Alarm', () => {
});
});

test('metric warnings are added to Alarm', () => {
test('metric warnings are added to Alarm for unrecognized statistic', () => {
const stack = new Stack(undefined, 'MyStack');
const m = new Metric({
namespace: 'CDK/Test',
metricName: 'Metric',
statistic: 'invalid',
});

// WHEN
new Alarm(stack, 'MyAlarm', {
metric: m,
evaluationPeriods: 1,
threshold: 1,
});

// THEN
const template = Annotations.fromStack(stack);
template.hasWarning('/MyStack/MyAlarm', Match.stringLikeRegexp('Unrecognized statistic.*Preferably use the `aws_cloudwatch.Stats` helper class to specify a statistic'));
});

test('metric warnings are added to Alarm for math expressions', () => {
const stack = new Stack(undefined, 'MyStack');
const m = new MathExpression({ expression: 'oops' });

Expand Down