Skip to content

Commit

Permalink
fix(alarm): ensure each datapoint has sufficient number of samples fo…
Browse files Browse the repository at this point in the history
…r minMetricSamplesToAlarm
  • Loading branch information
miloszwatroba committed Nov 9, 2023
1 parent 84609a4 commit 58a1cc2
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 164 deletions.
10 changes: 4 additions & 6 deletions API.md

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

94 changes: 37 additions & 57 deletions lib/common/alarm/AlarmFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
CompositeAlarm,
HorizontalAnnotation,
IAlarmRule,
MathExpression,
TreatMissingData,
} from "aws-cdk-lib/aws-cloudwatch";
import { Construct } from "constructs";
Expand Down Expand Up @@ -188,12 +189,11 @@ export interface AddAlarmProps {

/**
* Specifies how many samples (N) of the metric is needed to trigger the alarm.
* If this property is specified, an artificial composite alarm is created of the following:
* <ul>
* <li>The original alarm, created without this property being used; this alarm will have no actions set.</li>
* <li>A secondary alarm, which will monitor the same metric with the N (SampleCount) statistic, checking the sample count.</li>
* </ul>
* The newly created composite alarm will be returned as a result, and it will take the original alarm actions.
* If this property is specified, your metric will be subject to MetricMath 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.
*
* @default - default behaviour - no condition on sample count will be added to the alarm
*/
readonly minMetricSamplesToAlarm?: number;
Expand Down Expand Up @@ -511,7 +511,10 @@ export class AlarmFactory {
props
);

// prepare primary alarm properties
// metric that will be ultimately used to create the alarm
let alarmMetric: MetricWithAlarmSupport = adjustedMetric;

// prepare alarm properties

const actionsEnabled = this.determineActionsEnabled(
props.actionsEnabled,
Expand Down Expand Up @@ -549,63 +552,40 @@ export class AlarmFactory {
);
}

// create primary alarm

const primaryAlarm = adjustedMetric.createAlarm(
this.alarmScope,
alarmName,
{
alarmName,
alarmDescription,
threshold: props.threshold,
comparisonOperator: props.comparisonOperator,
treatMissingData: props.treatMissingData,
// default value (undefined) means "evaluate"
evaluateLowSampleCountPercentile: evaluateLowSampleCountPercentile
? undefined
: "ignore",
datapointsToAlarm,
evaluationPeriods,
actionsEnabled,
}
);

let alarm: AlarmBase = primaryAlarm;

// create composite alarm for min metric samples (if defined)
// apply metric math for minimum metric samples

if (props.minMetricSamplesToAlarm) {
const metricSampleCount = adjustedMetric.with({
statistic: MetricStatistic.N,
});
const noSamplesAlarm = metricSampleCount.createAlarm(
this.alarmScope,
`${alarmName}-NoSamples`,
{
alarmName: `${alarmName}-NoSamples`,
alarmDescription: `The metric (${adjustedMetric}) does not have enough samples to alarm. Must have at least ${props.minMetricSamplesToAlarm}.`,
threshold: props.minMetricSamplesToAlarm,
comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
treatMissingData: TreatMissingData.BREACHING,
datapointsToAlarm: 1,
evaluationPeriods: 1,
actionsEnabled,
}
);
alarm = new CompositeAlarm(this.alarmScope, `${alarmName}-WithSamples`, {
actionsEnabled,
compositeAlarmName: `${alarmName}-WithSamples`,
alarmDescription: this.joinDescriptionParts(
alarmDescription,
`Min number of samples to alarm: ${props.minMetricSamplesToAlarm}`
),
alarmRule: AlarmRule.allOf(
AlarmRule.fromAlarm(primaryAlarm, AlarmState.ALARM),
AlarmRule.not(AlarmRule.fromAlarm(noSamplesAlarm, AlarmState.ALARM))
),

alarmMetric = new MathExpression({
label: `${adjustedMetric}-WithSamples`,
expression: `IF(sampleCount > ${props.minMetricSamplesToAlarm}, metric)`,
usingMetrics: {
metric: adjustedMetric,
sampleCount: metricSampleCount,
},
});
}

// create alarm

const alarm = alarmMetric.createAlarm(this.alarmScope, alarmName, {
alarmName,
alarmDescription,
threshold: props.threshold,
comparisonOperator: props.comparisonOperator,
treatMissingData: props.treatMissingData,
// default value (undefined) means "evaluate"
evaluateLowSampleCountPercentile: evaluateLowSampleCountPercentile
? undefined
: "ignore",
datapointsToAlarm,
evaluationPeriods,
actionsEnabled,
});

// attach alarm actions

action.addAlarmActions({
Expand All @@ -620,7 +600,7 @@ export class AlarmFactory {
// create annotation for the primary alarm

const annotation = this.createAnnotation({
alarm: primaryAlarm,
alarm,
action,
metric: adjustedMetric,
evaluationPeriods,
Expand Down
69 changes: 32 additions & 37 deletions test/common/alarm/AlarmFactory.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Duration, Stack } from "aws-cdk-lib";
import { Capture, Template } from "aws-cdk-lib/assertions";
import { Match, Template } from "aws-cdk-lib/assertions";
import {
Alarm,
CfnAlarm,
Expand Down Expand Up @@ -289,43 +289,38 @@ test("addAlarm: check created alarms when minMetricSamplesToAlarm is used", () =
const template = Template.fromStack(stack);
template.hasResourceProperties("AWS::CloudWatch::Alarm", {
AlarmName: "DummyServiceAlarms-prefix-none",
MetricName: "DummyMetric1",
Statistic: "Average",
});
template.hasResourceProperties("AWS::CloudWatch::Alarm", {
AlarmName: "DummyServiceAlarms-prefix-none-NoSamples",
AlarmDescription:
"The metric (DummyMetric1) does not have enough samples to alarm. Must have at least 42.",
AlarmDescription: "Description",
ComparisonOperator: "LessThanThreshold",
DatapointsToAlarm: 1,
EvaluationPeriods: 1,
MetricName: "DummyMetric1",
Statistic: "SampleCount",
Threshold: 42,
TreatMissingData: "breaching",
});

const alarmRuleCapture = new Capture();
template.hasResourceProperties("AWS::CloudWatch::CompositeAlarm", {
AlarmName: "DummyServiceAlarms-prefix-none-WithSamples",
AlarmRule: alarmRuleCapture,
});
const expectedPrimaryAlarmArn = {
"Fn::GetAtt": ["DummyServiceAlarmsprefixnoneF01556DA", "Arn"],
};
const expectedSecondaryAlarmArn = {
"Fn::GetAtt": ["DummyServiceAlarmsprefixnoneNoSamples414211DB", "Arn"],
};
expect(alarmRuleCapture.asObject()).toStrictEqual({
["Fn::Join"]: [
"",
[
'(ALARM("',
expectedPrimaryAlarmArn,
'") AND (NOT (ALARM("',
expectedSecondaryAlarmArn,
'"))))',
],
DatapointsToAlarm: 10,
EvaluationPeriods: 10,
TreatMissingData: "notBreaching",
Metrics: [
Match.objectLike({
Expression: "IF(sampleCount > 42, metric)",
Label: "DummyMetric1-WithSamples",
}),
{
Id: "metric",
MetricStat: {
Metric: Match.objectLike({
MetricName: "DummyMetric1",
}),
Period: 300,
Stat: "Average",
},
ReturnData: false,
},
{
Id: "sampleCount",
MetricStat: {
Metric: Match.objectLike({
MetricName: "DummyMetric1",
}),
Period: 300,
Stat: "SampleCount",
},
ReturnData: false,
},
],
});
});
Expand Down
107 changes: 43 additions & 64 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 58a1cc2

Please sign in to comment.