You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hi all! I'm wondering if it's possible to allow generic alarm creation for any MetricWithAlarmSupport on a given Monitoring scope.
Context for this question is that I'm setting up some monitoring/alarming for a Kinesis Data Firehose, and the KinesisFirehoseMonitoring class only allows passing addRecordsThrottledAlarm to the constructor, but it creates ~10 metrics that could be alarmed on. It doesnt seem possible/simple to access this without going in a roundabout way that guesses about some internal patterns by calling MonitoringFacade.createdMonitorings() and then for each entry in the returned array checking for some unique public property like title and re-constructing in where you call MonitoringFacade.monitorKinesisFirehose.
This feels like a pretty fragile way to add alarms for metrics that have been otherwise created, so I was wondering if a feature could be added that did something like take a generic { [key: string]: <GenericAlarmConfigInterface> } prop where each key is one of the MetricWithAlarmSupport fields on a Monitoring instance like incomingBytesMetric on a KinesisFirehoseMonitoring instance.
This might end up needing to be a 2.0 of this library, but unless I'm missing something in the code it's currently easy to create dashboards, but extremely hard to create alarms for many of the things on those dashboards so it may be worth it.
I can take a crack at a PR if this is a feature that there's appetite for, but I wanted to at least raise an issue/feature request to gauge appetite before opening a likely large PR.
An alternate implementation option could be to return segment; in the functions like monitorApiGateway instead of return this; which would allow people to easily access the MetricWithAlarmSupport properties for the resource they are monitoring, but that obviously removes chainability of that function which is also a breaking change.
The text was updated successfully, but these errors were encountered:
Feature scope
Alarms
Describe your suggested feature
Hi all! I'm wondering if it's possible to allow generic alarm creation for any
MetricWithAlarmSupport
on a givenMonitoring
scope.Context for this question is that I'm setting up some monitoring/alarming for a Kinesis Data Firehose, and the KinesisFirehoseMonitoring class only allows passing
addRecordsThrottledAlarm
to the constructor, but it creates ~10 metrics that could be alarmed on. It doesnt seem possible/simple to access this without going in a roundabout way that guesses about some internal patterns by calling MonitoringFacade.createdMonitorings() and then for each entry in the returned array checking for some unique public property liketitle
and re-constructing in where you callMonitoringFacade.monitorKinesisFirehose
.This feels like a pretty fragile way to add alarms for metrics that have been otherwise created, so I was wondering if a feature could be added that did something like take a generic
{ [key: string]: <GenericAlarmConfigInterface> }
prop where eachkey
is one of theMetricWithAlarmSupport
fields on aMonitoring
instance likeincomingBytesMetric
on aKinesisFirehoseMonitoring
instance.This might end up needing to be a 2.0 of this library, but unless I'm missing something in the code it's currently easy to create dashboards, but extremely hard to create alarms for many of the things on those dashboards so it may be worth it.
I can take a crack at a PR if this is a feature that there's appetite for, but I wanted to at least raise an issue/feature request to gauge appetite before opening a likely large PR.
An alternate implementation option could be to
return segment;
in the functions likemonitorApiGateway
instead ofreturn this;
which would allow people to easily access theMetricWithAlarmSupport
properties for the resource they are monitoring, but that obviously removes chainability of that function which is also a breaking change.The text was updated successfully, but these errors were encountered: