Skip to content

Commit

Permalink
fix(cloudwatch): cloudwatch ec2 alarm action with multiple dimension …
Browse files Browse the repository at this point in the history
…results in error (#29364)

### Issue # (if applicable)

Closes #29331

### Reason for this change

While trying to create a Custom Metric with multiple dimension, and adding EC2 action, the CDK synth fails.

### Description of changes

As long as there's instance id in dimension, we should accept it instead of raising exception.

### Description of how you validated changes

new tests and existing tests pass.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
GavinZZ committed Mar 14, 2024
1 parent bc9d0b4 commit cc37778
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-cloudwatch/lib/alarm.ts
Expand Up @@ -267,7 +267,7 @@ export class Alarm extends AlarmBase {
if (ec2ActionsRegexp.test(actionArn)) {
// Check per-instance metric
const metricConfig = this.metric.toMetricConfig();
if (metricConfig.metricStat?.dimensions?.length != 1 || metricConfig.metricStat?.dimensions![0].name != 'InstanceId') {
if (metricConfig.metricStat?.dimensions?.length != 1 || !metricConfig.metricStat?.dimensions?.some(dimension => dimension.name === 'InstanceId')) {
throw new Error(`EC2 alarm actions requires an EC2 Per-Instance Metric. (${JSON.stringify(metricConfig)} does not have an 'InstanceId' dimension)`);
}
}
Expand Down
63 changes: 62 additions & 1 deletion packages/aws-cdk-lib/aws-cloudwatch/test/alarm.test.ts
@@ -1,6 +1,8 @@
import { Construct } from 'constructs';
import { Match, Template, Annotations } from '../../assertions';
import { Duration, Stack } from '../../core';
import { Ec2Action, Ec2InstanceAction } from '../../aws-cloudwatch-actions/lib';
import { Duration, Stack, App } from '../../core';
import { ENABLE_PARTITION_LITERALS } from '../../cx-api';
import { Alarm, IAlarm, IAlarmAction, Metric, MathExpression, IMetric, Stats } from '../lib';

const testMetric = new Metric({
Expand Down Expand Up @@ -232,6 +234,65 @@ describe('Alarm', () => {
});
});

test('EC2 alarm actions with InstanceId dimension', () => {
// GIVEN
const app = new App({ context: { [ ENABLE_PARTITION_LITERALS]: true } });
const stack = new Stack(app, 'EC2AlarmStack', { env: { region: 'us-west-2', account: '123456789012' } });

// WHEN
const metric = new Metric({
namespace: 'CWAgent',
metricName: 'disk_used_percent',
dimensionsMap: {
InstanceId: 'instance-id',
},
period: Duration.minutes(5),
statistic: 'Average',
});

const sev3Alarm = new Alarm(stack, 'DISK_USED_PERCENT_SEV3', {
alarmName: 'DISK_USED_PERCENT_SEV3',
actionsEnabled: true,
metric: metric,
threshold: 1,
evaluationPeriods: 1,
});

expect(() => {
sev3Alarm.addAlarmAction(new Ec2Action(Ec2InstanceAction.REBOOT));
}).not.toThrow();
});

test('EC2 alarm actions without InstanceId dimension', () => {
// GIVEN
const app = new App({ context: { [ ENABLE_PARTITION_LITERALS]: true } });
const stack = new Stack(app, 'EC2AlarmStack', { env: { region: 'us-west-2', account: '123456789012' } });

// WHEN
const metric = new Metric({
namespace: 'CWAgent',
metricName: 'disk_used_percent',
dimensionsMap: {
ImageId: 'image-id',
InstanceType: 't2.micro',
},
period: Duration.minutes(5),
statistic: 'Average',
});

const sev3Alarm = new Alarm(stack, 'DISK_USED_PERCENT_SEV3', {
alarmName: 'DISK_USED_PERCENT_SEV3',
actionsEnabled: true,
metric: metric,
threshold: 1,
evaluationPeriods: 1,
});

expect(() => {
sev3Alarm.addAlarmAction(new Ec2Action(Ec2InstanceAction.REBOOT));
}).toThrow(/EC2 alarm actions requires an EC2 Per-Instance Metric/);
});

test('can use percentile string to make alarm', () => {
// GIVEN
const stack = new Stack();
Expand Down

0 comments on commit cc37778

Please sign in to comment.