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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alarm constructor statistic prop doesn't use Cfn extendedStatistic for percentile values #3845

Closed
mitchlloyd opened this issue Aug 28, 2019 · 0 comments 路 Fixed by #4253
Closed
Assignees
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch bug This issue is a bug.

Comments

@mitchlloyd
Copy link
Contributor

馃悰 Bug Report

What is the problem?

Creating cloudwatch Alarms via the metric.createAlarm api works with percentage values.

Also creating a metric with a value like p95 correctly uses the extendedStatistic Cfn field on a Metric:

new cloudwatch.Alarm(this, 'MyAlarm', {
  // ...
  metric: new cloudwatch.Metric({
    statistic: 'p95', // Works!
    namespace: 'Namespace',
    metricName: 'Duration',
    dimensions: { 'Operation': 'MyAPI', 'Region': 'us-west-2', 'Service': 'MyService' },
  }),
});

However using the statistic field on alarm will not work. It only works with "simple" metrics because it only uses the statistic and never the extendedStatistic field.

new cloudwatch.Alarm(this, 'MyAlarm', {
  // ...
  statistic: 'p95', // Cfn error!
  metric: new cloudwatch.Metric({
    namespace: 'Namespace',
    metricName: 'Duration',
    dimensions: { 'Operation': 'MyAPI', 'Region': 'us-west-2', 'Service': 'MyService' },
  }),
});

This is quite a gotcha given that a "simple" metric works:

new cloudwatch.Alarm(this, 'GetCategorySuccessSampleCount', {
  // ...
  statistic: 'SampleCount', // Works!
  metric: new cloudwatch.Metric({
    namespace: 'Namespace',
    metricName: 'Success',
    dimensions: { 'Operation': 'MyAPI', 'Region': 'us-west-2', 'Service': 'MyService' },
  }),
});

Reproduction Steps

This causes a Cfn error when deploying:

new cloudwatch.Alarm(this, 'MyAlarm', {
  // ...
  statistic: 'p95', // Cfn error!
  metric: new cloudwatch.Metric({
    namespace: 'Namespace',
    metricName: 'Duration',
    dimensions: { 'Operation': 'MyAPI', 'Region': 'us-west-2', 'Service': 'MyService' },
  }),
});

Environment

  • CDK CLI Version: 1.6.0
  • Module Version: 1.6.0
  • OS: OSX
  • Language: TypeScript
@mitchlloyd mitchlloyd added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2019
@shivlaks shivlaks added the @aws-cdk/aws-cloudwatch Related to Amazon CloudWatch label Aug 29, 2019
@rix0rrr rix0rrr removed the needs-triage This issue or PR still needs to be triaged. label Sep 4, 2019
rix0rrr added a commit that referenced this issue Sep 26, 2019
Correctly render percentile statistic to the `ExtendedStatistic` field, instead
of the `Statistic` field.

Fixes #3845.
@mergify mergify bot closed this as completed in #4253 Sep 27, 2019
mergify bot pushed a commit that referenced this issue Sep 27, 2019
Correctly render percentile statistic to the `ExtendedStatistic` field, instead
of the `Statistic` field.

Fixes #3845.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants