feat(cloudwatch): add PromQL Alarm L2 construct#37793
Conversation
|
PRs without a linked issue will receive lower priority for review and merging. Please update the description to follow the PR template and include a line like |
|
|
||||||||||||||
|
|
||||||||||||||
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
kumsmrit
left a comment
There was a problem hiding this comment.
Thank you for your contribution; I have reviewed and added some comments.
| import type { IAlarm, IAlarmAction } from '../lib'; | ||
| import { PromQLAlarm } from '../lib/promql-alarm'; | ||
|
|
||
| describe('PromQLAlarm', () => { |
There was a problem hiding this comment.
The construct validates evaluationInterval, pendingPeriod, recoveryPeriod, and query length, but there are no tests verifying these validations throw.
Can we add tests for each of the validation paths?
| ## PromQL Alarms | ||
|
|
||
| PromQL alarms allow you to create CloudWatch alarms using PromQL query expressions. This is useful when working with Prometheus-compatible metrics in CloudWatch. | ||
|
|
There was a problem hiding this comment.
Can we add detail as how is it different from standard CloudWatch alarms?
| /** | ||
| * Import an existing CloudWatch alarm provided an ARN. | ||
| */ | ||
| public static fromAlarmArn(scope: Construct, id: string, alarmArn: string): IAlarm { |
There was a problem hiding this comment.
| public static fromAlarmArn(scope: Construct, id: string, alarmArn: string): IAlarm { | |
| public static fromPromQLAlarmArn(scope: Construct, id: string, alarmArn: string): IAlarm { |
| /** | ||
| * Import an existing CloudWatch alarm provided a Name. | ||
| */ | ||
| public static fromAlarmName(scope: Construct, id: string, alarmName: string): IAlarm { |
There was a problem hiding this comment.
| public static fromAlarmName(scope: Construct, id: string, alarmName: string): IAlarm { | |
| public static fromPromQLAlarmName(scope: Construct, id: string, alarmName: string): IAlarm { |
|
|
||
| /** | ||
| * A CloudWatch Alarm based on a PromQL query expression. | ||
| * |
There was a problem hiding this comment.
nit:
| * | |
| * @see https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/alarm-promql.html |
| alarmDescription: props.alarmDescription, | ||
| alarmName: this.physicalName, | ||
| actionsEnabled: props.actionsEnabled, | ||
| alarmActions: Lazy.list({ produce: () => this.alarmActionArns }), |
There was a problem hiding this comment.
AlarmBase._alarmActionArns is now an IArrayBox<string> which already implements IResolvable directly, so this can be passed to the L1 without an additional Lazy.list(...) wrapper. Use Token.asList(...) instead.
| alarmName: this.physicalName, | ||
| actionsEnabled: props.actionsEnabled, | ||
| alarmActions: Lazy.list({ produce: () => this.alarmActionArns }), | ||
| insufficientDataActions: Lazy.list({ produce: () => this.insufficientDataActionArns }), |
There was a problem hiding this comment.
Please use Token.asList(...) instead of Lazy.list(...). Applicable at other places as well.
|
|
||
| new IntegTest(app, 'PromQLAlarmIntegTest', { | ||
| testCases: [stack], | ||
| }); |
There was a problem hiding this comment.
Consider adding assertions to verify if the alarm is created correctly after deployment; That would help validate that the deployed alarm contains the expected EvaluationCriteria.PromQLCriteria.
There was a problem hiding this comment.
As discussed, since the parameters are newly added and the sdk version is not updated to have them yet. We'll add the validation once the sdk version is updated later
Add PromQLAlarm construct to aws-cloudwatch module enabling customers to define PromQL-based alarms in their CDK applications.
2ef5635 to
4a9a9d0
Compare
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status
This pull request spent 45 minutes 12 seconds in the queue, including 35 minutes 4 seconds running CI. Required conditions to merge
ReasonThe merge conditions cannot be satisfied due to failing checks HintYou may have to fix your CI before adding the pull request to the queue again. |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status
This pull request spent 1 hour 22 minutes 3 seconds in the queue, including 33 minutes 49 seconds running CI. Required conditions to merge
|
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Reason for this change
Add a new PromQLAlarm L2 construct to the aws-cdk-lib/aws-cloudwatch module that enables users to create CloudWatch alarms based on PromQL instant queries. PromQL alarms monitor metrics ingested through the CloudWatch OTLP endpoint and use duration-based state transitions (pending/recovery periods) instead of the evaluation-period/threshold model used by standard CloudWatch alarms.
Description of changes
Describe any new or updated permissions being added
N/A
Description of how you validated changes
Unit tests + Integ tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license