cdklabs / cdk-monitoring-constructs

Easy-to-use CDK constructs for monitoring your AWS infrastructure
https://constructs.dev/packages/cdk-monitoring-constructs
Apache License 2.0
444 stars 56 forks source link

feat(route53): Support Route53 Health Checks #411

Closed marcogrcr closed 10 months ago

marcogrcr commented 10 months ago

WHAT?

Introduce interface IMetricAdjuster which allows to adjust a metric before AlarmFactory creates an alarm from it.

Additionally created three implementations of IMetricAdjuster:

WHY?

Route53 Health Checks have strict requirements about which CloudWatch alarms can be used. By default, alarms created by MonitoringFacade use metrics with the label property set which results in alarms that are incompatible with Route53 Health Checks. This commit introdues an opt-in Route53HealthCheckMetricAdjuster class that allows users to mark alarms to be suitable for Route53 Health Checks.

HOW?

npx yarn build succeeds all the way to package:js.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

marcogrcr commented 10 months ago

I debated internally whether customTags + PredefinedCustomTags was the most appropriate way to implement this, or whether I should've added a new field to:

https://github.com/cdklabs/cdk-monitoring-constructs/blob/f23fd6736b90bbb601c87d7222b7438d9e8003fa/lib/common/alarm/CustomAlarmThreshold.ts#L12

I ultimately decided to go with the former because the field would also have to be added to:

https://github.com/cdklabs/cdk-monitoring-constructs/blob/f23fd6736b90bbb601c87d7222b7438d9e8003fa/lib/common/alarm/AlarmFactory.ts#L66

And it would've had to be populated on every addSomeAlarm method. While this would be mostly mitigated by the usage of the spread operator in the lib/common/monitoring/alarms/ factory implementations, for example:

https://github.com/cdklabs/cdk-monitoring-constructs/blob/f23fd6736b90bbb601c87d7222b7438d9e8003fa/lib/common/monitoring/alarms/LatencyAlarmFactory.ts#L127-L144

Any user-defined subclass of MonitoringFacade that does not use the spread operator, would miss propagating the property accordingly. Additionally, using customTags makes it easy later to query all the appropriate alarms for creating the health checks as follows:

monitoring.createdAlarmsWithTag(PredefinedCustomTags.ROUTE_53_HEALTH_CHECK);
marcogrcr commented 10 months ago

OK, I've taken your suggestion and added a new implementation. I've also updated https://github.com/cdklabs/cdk-monitoring-constructs/pull/411#issue-1850911095 to reflect the change. Hope this is in line with what you had in mind!