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

(cloudwatch): add validation for metric id field on CfnAlarm #26674

Closed
2 tasks
wilhen01 opened this issue Aug 8, 2023 · 2 comments
Closed
2 tasks

(cloudwatch): add validation for metric id field on CfnAlarm #26674

wilhen01 opened this issue Aug 8, 2023 · 2 comments
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch feature-request A feature should be added or improved.

Comments

@wilhen01
Copy link
Contributor

wilhen01 commented Aug 8, 2023

Describe the feature

When creating alarms via the L1 construct, it's possible to create an alarm which is totally valid according to CDK but fails to deploy with a cryptic error as follows:

Invalid metrics list (Service: AmazonCloudWatch; Status Code: 400; Error Code: ValidationError; Request ID: 980ecb87-eb02-4eb8-80f3-dfe53d25f442; Proxy: null)

This is caused by metric Ids failing validation (see CloudFormation docs here)

Id
A short name used to tie this object to the results in the response. This name must be unique within a single call to GetMetricData. If you are performing math expressions on this set of data, this name represents that data and can serve as a variable in the mathematical expression. The valid characters are letters, numbers, and underscore. The first character must be a lowercase letter.

It would be great if CDK validated this field and caught bad IDs up front.

Example CDK code which causes this kind of deployment error:

   new CfnAlarm(this, '4xxErrorAlarm', {
      alarmDescription: 'High percentage of 4xx errors vs. total calls',
      metrics: [
        {
          id: 'percentage-of-4xx-errors', // id includes hyphens - invalid
          label: '% of total calls that receive 4xx response',
          expression: '(4xxErrors / totalCalls) * 100',
          returnData: true
        },
        {
          id: '4xxErrors', // id doesn't start with a lowercase letter - invalid
          metricStat: {
            metric: {
              namespace: 'AWS/ApiGateway',
              metricName: '4XXError',
              dimensions: [{ name: 'ApiName', value: this.restApi.restApiName }]
            },
            stat: 'Average',
            period: 300
          },
          returnData: false
        },
        {
          id: 'totalCalls',
          metricStat: {
            metric: {
              namespace: 'AWS/ApiGateway',
              metricName: 'Count',
              dimensions: [{ name: 'ApiName', value: this.restApi.restApiName }]
            },
            stat: 'Average',
            period: 300
          },
          returnData: false
        }
      ],
      comparisonOperator: ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
      evaluationPeriods: 1,
      datapointsToAlarm: 1,
      threshold: 10,
      treatMissingData: TreatMissingData.IGNORE
    });

Use Case

Better developer experience when creating alarms

Proposed Solution

Add validation in CDK

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.87.0

Environment details (OS name and version, etc.)

Mac OS

@wilhen01 wilhen01 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2023
@github-actions github-actions bot added the @aws-cdk/aws-cloudwatch Related to Amazon CloudWatch label Aug 8, 2023
@peterwoodworth
Copy link
Contributor

L1 resources are defining L1 resources exactly as you'd like them in the CloudFormation template. If it follows the API model that CloudFormation allows, then we will allow it. We can't do any validation on these since we don't own these resources and cannot programmatically know details like this

@peterwoodworth peterwoodworth removed the needs-triage This issue or PR still needs to be triaged. label Aug 8, 2023
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

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 feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants