Skip to content

Commit

Permalink
feat(alarm): add minSampleCountToEvaluateDatapoint to CustomMonitoring (
Browse files Browse the repository at this point in the history
#458)

Fixes #452

Follow up to
#453:

* (feat) Exposing the new `minSampleCountToEvaluateDatapoint` through
CustomMonitoring
* (fix) Fixing the `minSampleCountToEvaluateDatapoint` MathExpression's
period as it defaults to 5 minutes. This didn't come up during testing
as I tested it using 5 minute period. Apparently, if we don't set the
period on MathExpression explicitly, it overrides all child metrics to 5
minute,
[reference](https://github.com/aws/aws-cdk/blob/db21fefc2dc76eb4ff306fa41652ab6a6cc95e42/packages/aws-cdk-lib/aws-cloudwatch/lib/metric.ts#L606).
To avoid similar situations in the future, extended the unit test to
cover custom periods.

---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
  • Loading branch information
miloszwatroba authored Nov 16, 2023
1 parent 59e7fa9 commit 3bd2b37
Show file tree
Hide file tree
Showing 6 changed files with 1,465 additions and 66 deletions.
1,456 changes: 1,400 additions & 56 deletions API.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/common/alarm/AlarmFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ export class AlarmFactory {
alarmMetric = new MathExpression({
label: `${adjustedMetric}`,
expression: `IF(sampleCount > ${props.minSampleCountToEvaluateDatapoint}, metric)`,
period: adjustedMetric.period,
usingMetrics: {
metric: adjustedMetric,
sampleCount: metricSampleCount,
Expand Down
15 changes: 15 additions & 0 deletions lib/common/alarm/CustomAlarmThreshold.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ export interface CustomAlarmThreshold {
*/
readonly alarmDescriptionOverride?: string;

/**
* Specifies how many samples (N) of the metric is needed in a datapoint to be evaluated for alarming.
* If this property is specified, your metric will be subject to MathExpression that will add an IF condition
* to your metric to make sure that each datapoint is evaluated only if it has sufficient number of samples.
* If the number of samples is not sufficient, the datapoint will be treated as missing data and will be evaluated
* according to the treatMissingData parameter.
* If specified, deprecated minMetricSamplesToAlarm has no effect.
*
* @default - default behaviour - no condition on sample count will be used
*/
readonly minSampleCountToEvaluateDatapoint?: number;

/**
* Specifies how many samples (N) of the metric is needed to trigger the alarm.
* If this property is specified, a composite alarm is created of the following:
Expand All @@ -48,6 +60,9 @@ export interface CustomAlarmThreshold {
* </ul>
* This composite alarm will be returned as a result and uses the specified alarm actions.
* @default - default behaviour - no condition on sample count will be added to the alarm
* @deprecated Use minSampleCountToEvaluateDatapoint instead. minMetricSamplesAlarm uses different evaluation
* period for its child alarms, so it doesn't guarantee that each datapoint in the evaluation period has
* sufficient number of samples
*/
readonly minMetricSamplesToAlarm?: number;

Expand Down
8 changes: 6 additions & 2 deletions test/common/alarm/AlarmFactory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,15 @@ test("addAlarm: check created alarms when minMetricSamplesToAlarm is used", () =
alarmNameSuffix: "none",
comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
minMetricSamplesToAlarm: 42,
period: Duration.minutes(15),
});

const template = Template.fromStack(stack);
template.hasResourceProperties("AWS::CloudWatch::Alarm", {
AlarmName: "DummyServiceAlarms-prefix-none",
MetricName: "DummyMetric1",
Statistic: "Average",
Period: 900,
});
template.hasResourceProperties("AWS::CloudWatch::Alarm", {
AlarmName: "DummyServiceAlarms-prefix-none-NoSamples",
Expand All @@ -304,6 +306,7 @@ test("addAlarm: check created alarms when minMetricSamplesToAlarm is used", () =
Statistic: "SampleCount",
Threshold: 42,
TreatMissingData: "breaching",
Period: 900,
});

const alarmRuleCapture = new Capture();
Expand Down Expand Up @@ -344,6 +347,7 @@ test("addAlarm: check created alarms when minSampleCountToEvaluateDatapoint is u
comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
minSampleCountToEvaluateDatapoint: 42,
minMetricSamplesToAlarm: 55, // not used if minSampleCountToEvaluateDatapoint defined
period: Duration.minutes(15),
});

const template = Template.fromStack(stack);
Expand All @@ -365,7 +369,7 @@ test("addAlarm: check created alarms when minSampleCountToEvaluateDatapoint is u
Metric: Match.objectLike({
MetricName: "DummyMetric1",
}),
Period: 300,
Period: 900,
Stat: "Average",
},
ReturnData: false,
Expand All @@ -376,7 +380,7 @@ test("addAlarm: check created alarms when minSampleCountToEvaluateDatapoint is u
Metric: Match.objectLike({
MetricName: "DummyMetric1",
}),
Period: 300,
Period: 900,
Stat: "SampleCount",
},
ReturnData: false,
Expand Down
1 change: 1 addition & 0 deletions test/monitoring/custom/CustomMonitoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ test("snapshot test", () => {
Warning: {
threshold: 10,
comparisonOperator: ComparisonOperator.GREATER_THAN_THRESHOLD,
minSampleCountToEvaluateDatapoint: 15,
},
Critical: {
threshold: 50,
Expand Down
50 changes: 42 additions & 8 deletions test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3bd2b37

Please sign in to comment.