Skip to content

Commit 3991ec8

Browse files
authored
fix(dynamo): adjusts DynamoTableGlobalSecondaryIndexMonitoring to respect localAlarmNamePrefixOverride (#629)
Fixes #628 This change passes the BaseMonitoringProps passed into DynamoTableGlobalSecondaryIndexMonitoring along to the super class constructor, which exposes `localAlarmNamePrefixOverride` to be consumed by Monitoring.createAlarmFactory. Additionally, this changes the construction of the alarm factory to use `createAlarmFactory` from the parent class (thus using `localAlarmNamePrefixOverride`, rather than that from the parent scope (MonitoringFacade, which doesn't have `localAlarmNamePrefixOverride` available). By doing so, we achieve the more expected behavior of having `localAlarmNamePrefixOverride` influence the local portion of the alarm name. By default, that portion is just the GSI name itself, and there are cases where this can cause collisions (e.g. two GSIs with the same name on different tables). Having `localAlarmNamePrefixOverride` function as expected provides an escape hatch for this corner case without changing the default naming behavior. (i.e. including table name in the alarm name). --- _By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license_
1 parent 0b2bee6 commit 3991ec8

File tree

3 files changed

+317
-2
lines changed

3 files changed

+317
-2
lines changed

lib/monitoring/aws-dynamo/DynamoTableGlobalSecondaryIndexMonitoring.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class DynamoTableGlobalSecondaryIndexMonitoring extends Monitoring {
5959
scope: MonitoringScope,
6060
props: DynamoTableGlobalSecondaryIndexMonitoringProps,
6161
) {
62-
super(scope);
62+
super(scope, props);
6363

6464
const namingStrategy = new MonitoringNamingStrategy({
6565
...props,
@@ -92,7 +92,7 @@ export class DynamoTableGlobalSecondaryIndexMonitoring extends Monitoring {
9292
this.indexThrottleCountMetric =
9393
metricFactory.metricThrottledIndexRequestCount();
9494

95-
const alarmFactory = scope.createAlarmFactory(
95+
const alarmFactory = this.createAlarmFactory(
9696
namingStrategy.resolveAlarmFriendlyName(),
9797
);
9898
this.gsiAlarmFactory = new DynamoAlarmFactory(alarmFactory);

test/monitoring/aws-dynamo/DynamoTableGlobalSecondaryIndexMonitoring.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,63 @@ test("snapshot test: all alarms", () => {
7878
expect(numAlarmsCreated).toStrictEqual(2);
7979
expect(Template.fromStack(stack)).toMatchSnapshot();
8080
});
81+
82+
test("test: localAlarmNamePrefixOverride is properly applied", () => {
83+
const stack = new Stack();
84+
const scope = new TestMonitoringScope(stack, "Scope");
85+
86+
const table = new Table(stack, "Table", {
87+
tableName: "DummyTable",
88+
partitionKey: {
89+
name: "id",
90+
type: AttributeType.STRING,
91+
},
92+
});
93+
94+
const customPrefix = "CustomPrefix";
95+
const indexName = "non-existing-index";
96+
const monitoring = new DynamoTableGlobalSecondaryIndexMonitoring(scope, {
97+
table,
98+
globalSecondaryIndexName: indexName,
99+
localAlarmNamePrefixOverride: customPrefix,
100+
addReadThrottledEventsCountAlarm: {
101+
Warning: {
102+
maxThrottledEventsThreshold: 5,
103+
},
104+
},
105+
});
106+
107+
addMonitoringDashboardsToStack(stack, monitoring);
108+
109+
const template = Template.fromStack(stack);
110+
const alarms = template.findResources("AWS::CloudWatch::Alarm");
111+
const alarmLogicalIds = Object.keys(alarms);
112+
113+
expect(alarmLogicalIds.length).toStrictEqual(1);
114+
115+
const alarmNames = alarmLogicalIds.map(
116+
(id) => alarms[id].Properties.AlarmName,
117+
);
118+
119+
const alarmNameHasCustomPrefix = alarmNames.every((name) =>
120+
name.includes(customPrefix),
121+
);
122+
123+
const alarmIdHasCustomPrefix = alarmLogicalIds.every((id) =>
124+
id.includes(customPrefix),
125+
);
126+
127+
const alarmNameHasDefaultPrefix = alarmNames.some((name) =>
128+
name.includes(indexName),
129+
);
130+
131+
const alarmIdHasDefaultPrefix = alarmLogicalIds.some((id) =>
132+
id.includes(indexName),
133+
);
134+
135+
expect(alarmNameHasCustomPrefix).toBe(true);
136+
expect(alarmIdHasCustomPrefix).toBe(true);
137+
expect(alarmNameHasDefaultPrefix).toBe(false);
138+
expect(alarmIdHasDefaultPrefix).toBe(false);
139+
expect(Template.fromStack(stack)).toMatchSnapshot();
140+
});

test/monitoring/aws-dynamo/__snapshots__/DynamoTableGlobalSecondaryIndexMonitoring.test.ts.snap

Lines changed: 255 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)