-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Description
aws/cloudwatch doesn't yet have a support for expressions, either on metrics or alarms.
I'd like to contribute by adding this feature in a backward-compatible way.
Use Case
This feature is needed so as to exploit all cloudwatch capabilites when it comes to expressions:
- Creating expression-based Widgets/Graphs/Dashboards
- Creating expression-based Alarms
Proposed Solution
Limitations
Current implementation forces the usage of IMetric Interface everywhere which:
-
Assumes MetricGraphConfig always have a MetricName & Namespace which is not the case for expression-based metrics according to docs, and consequently:
- Mapping a metric to a widget JSON here forces the Namespace & MetricName, and for expressions they must not exist.
-
Assumes MetricAlarmConfig always have the Namespace and MetricName which is not the case for expression-based alarms according to the docs, and consequently:
- Mapping a metric to Alarm here forces the Namespace & MetricName, and for expression they must not exist.
Another limitation some missing RenderingProperties.
To support expressions we need id
, expression
, and visible
, but adding all of them to RenderingProperties
wouldn't make sense because of expression
.
One last limitation is a public property in the Alarm class, this assumes that a metric will always be an IMetric
and forces that any future changes must inherit the IMetric
interface which is not doable, because Expressions are different from Regular Metrics in terms of Graph/Alarm configurations.
Solution
NOTE: For UML diagrams, font color means:
- Black for exisitng
- Green for new
- Red for moved
To be able to change this behaviour in a backward-compatible way, I propose the following:
-
Having a super interface
IBaseMetric
that currentIMetric
extends and add a newIExpressionMetric
that extends it as shown here -
Having a super interface
CommonRenderingProperties
that currentMetricRenderingProperties
uses and add a newExpressionMetricRenderingProperties
that extends it as shown here- NOTE:
accountId
andregion
aren't related to this change, but I think it's definitely still nice to have to support cross-account-cross-region metrics and dashboards.
- NOTE:
-
Deprecate the public property previously mentioned and use some
alarmMetric
that usesIBaseMetric
instead, because someone may have done
const alarm = new Alarm(...);
alarm.metric.<something>; // Thus cannot be removed but can be depreciated.
Other
- 👋 I may be able to implement this feature request
-
⚠️ This feature might incur a breaking change
This is a 🚀 Feature Request