Add `treatMissingData` as an alias for `treatMissingDataOverride`

[dimakuv]

Version

ALL

Steps and/or minimal code example to reproduce

I noticed that treatMissingData is silently ignored when (incorrectly) creating custom monitoring, smth like this:

const snsMetricGroup = {
    metrics: [
        {
            metric: snsMetric,
            addAlarm: {
                Warning: {
                    ...
                    treatMissingData: TreatMissingData.NOT_BREACHING,
                    comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
                }
            }
        }
    ],
};

this.monitoring.monitorCustom({
    ...
    metricGroups: [snsMetricGroup]
});

The interesting part is that comparisonOperator is not silently ignored.

This happens because:

• The object we create for the Warning record is of type CustomThreshold.
• CustomThreshold is an interface that defines the comparisonOperator field and extends another interface CustomAlarmThreshold, see here.
• CustomAlarmThreshold defines two fields: comparisonOperatorOverride and treatMissingDataOverride.

Now if we look at the implementation of CustomAlarmFactory::addCustomAlarm() which is being called by monitorCustom() from my code snippet, we see this non-uniform behavior:

cdk-monitoring-constructs/lib/common/monitoring/alarms/CustomAlarmFactory.ts

Lines 29 to 42 in a9d1dfc

	return this.alarmFactory.addAlarm(metric, {
	...props,
	disambiguator,
	treatMissingData:
	props.treatMissingDataOverride ?? TreatMissingData.MISSING,
	threshold: props.threshold,
	comparisonOperator:
	props.comparisonOperatorOverride ?? props.comparisonOperator,
	alarmDedupeStringSuffix: props.dedupeString,
	alarmNameSuffix,
	alarmDescription:
	props.additionalDescription ??
	`Threshold of ${props.threshold} has been breached.`,
	});

• Notice how comparisonOperator can be any of props.comparisonOperatorOverride and props.comparisonOperator defined by the user.
• Notice how treatMissingData can only be overriden by props.treatMissingDataOverride, otherwise it defaults to the constant MISSING.

This is not a bug per se, but this is something that is easy to get wrong. It would make sense for developers (for readability) to allow only two options:

• both comparisonOperator and treatMissingData
• both comparisonOperatorOverride and treatMissingDataOverride
• but not a mix of comparisonOperator and treatMissingDataOverride

Looking at git blame, I see that this inconsistency was introduced when this public GitHub repo was created: 8d7ce03. So this happens in all versions of CDK Monitoring Constructs.

Expected behavior

I would argue that the easiest fix would be to add the treatMissingData to CustomThreshold interface, and modify the CustomAlarmFactory::addCustomAlarm() implementation to fall back to this new field if no treatMissingDataOverride is set.

Actual behavior

Silent bug in our alarms, as discussed above (the alarms used TreatMissingData.MISSING instead of the requested TreatMissingData.NOT_BREACHING).

Other details

Slightly related issue: #669

[dimakuv]

I noticed one more thing: CustomThreshold::comparisonOperator is a required field. So I can't write code like this:

                Warning: {
                    ...
                    treatMissingDataOverride: TreatMissingData.NOT_BREACHING,
                    comparisonOperatorOverride: ComparisonOperator.LESS_THAN_THRESHOLD,
                }

Because the build then complains something like this:

error TS2741: Property 'comparisonOperator' is missing in type 
'{ additionalDescription: string; treatMissingDataOverride: TreatMissingData.NOT_BREACHING; 
  comparisonOperatorOverride: ComparisonOperator.LESS_THAN_THRESHOLD; ... }'
but required in type 'CustomThreshold'.

---

At this point, the only combination that seems to work is this:

                Warning: {
                    ...
                    treatMissingDataOverride: TreatMissingData.NOT_BREACHING,
                    comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
                }