feat(aws-cloudwatch): can specify an id in math expression metrics #17126
Conversation
|
|
github-actions
bot
added
the
@aws-cdk/aws-cloudwatch
There was a problem hiding this comment.
This is great!
To help out users when they don't specify any ids, I think we should also implement behavior similar to the Cloudwatch console where as you add MathExpressions to a graph widget, they are automatically assigned ids starting with e1, e2, and incrementing from there. This would ensure that customers graphs render correctly even if they do not manually specify ids themselves.
We can either merge as is, and open another issue for the default id behavior. Or you can add to this PR. Let me know what you'd like to do!
corymhall
force-pushed
the
cloudwatch-math-ids
branch
from
45896ef
to
67d6a66
Compare
| @@ -173,6 +176,31 @@ export class MetricSet<A> { | |||
| if (!entry.id && id) { | |||
| entry.id = id; | |||
| this.metricById.set(id, entry); | |||
| // if we don't have an id and this is a math expression | |||
There was a problem hiding this comment.
@madeline-k I've implemented the auto assign id logic here. Rather than try and figure out if an id has already been manually assigned, I decided to instead just reserve an id format for auto assigned ids.
Also, since this logic is at the metric level, it also applies to Alarms in addition to Graphs. Alarms already had their own similar auto assign id logic which this will overwrite.
There was a problem hiding this comment.
hmm I may have to rethink that since we have integration tests in other modules reference the Alarm id.
There was a problem hiding this comment.
@madeline-k I updated the logic so that it only applies to Graphs, but did end up having to update the kinesis integration tests.
add the ability to assign a unique id to a math expression metric. Currently if you are creating multiple math expression metrics they will all get the id of m1 which makes it difficult to differentiate them in the console or reference them in other math expression metrics fixes #13942
Co-authored-by: Madeline Kusters <80541297+madeline-k@users.noreply.github.com>
corymhall
force-pushed
the
cloudwatch-math-ids
branch
from
6ca7d85
to
74fee7a
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
rix0rrr
added
bug
It is intended that all metric identifiers referenced in a MathExpression are included in the `usingMetrics` map. This allows passing around complex metrics as a single object, because the math expression object carries around its dependencies with it. This is slightly different than what people might be used to from raw CloudWatch, where there is no hierarchy and all metrics are supposed to be listed in the graph widget or alarm with a unique ID, and then referenced by ID. We can't make this contract obvious anymore by adding a hard validation, but we can add warnings to hint people at the right way to reference metrics in math expressions. Fixes #13942, closes #17126.
It is intended that all metric identifiers referenced in a MathExpression are included in the `usingMetrics` map. This allows passing around complex metrics as a single object, because the math expression object carries around its dependencies with it. This is slightly different than what people might be used to from raw CloudWatch, where there is no hierarchy and all metrics are supposed to be listed in the graph widget or alarm with a unique ID, and then referenced by ID. We can't make this contract obvious anymore by adding a hard validation, but we can add warnings to hint people at the right way to reference metrics in math expressions. Fixes #13942, closes #17126. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
It is intended that all metric identifiers referenced in a MathExpression are included in the `usingMetrics` map. This allows passing around complex metrics as a single object, because the math expression object carries around its dependencies with it. This is slightly different than what people might be used to from raw CloudWatch, where there is no hierarchy and all metrics are supposed to be listed in the graph widget or alarm with a unique ID, and then referenced by ID. We can't make this contract obvious anymore by adding a hard validation, but we can add warnings to hint people at the right way to reference metrics in math expressions. Fixes aws#13942, closes aws#17126. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
add the ability to assign a unique id to a math expression metric.
Currently if you are creating multiple math expression metrics they will
all get the id of m1 which makes it difficult to differentiate them in
the console or reference them in other math expression metrics
fixes #13942
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license