fix(cloudwatch): automatic metric math label cannot be suppressed #17639

NetaNir · 2021-11-23T01:34:11Z

According to CloudWatch docs:

For the Label column of the expression, enter a name that describes what the expression is calculating.
If the result of an expression is an array of time series, each of those time series is displayed on the graph with a separate line, with different colors. Immediately under the graph is a legend for each line in the graph. For a single expression that produces multiple time series, the legend captions for those time series are in the format Expression-Label Metric-Label. For example, if the graph includes a metric with a label of Errors and an expression FILL(METRICS(), 0) that has a label of Filled With 0:, one line in the legend would be Filled With 0: Errors. To have the legend show only the original metric labels, set Expression-Label to be empty.

In the current implementation, if the label is left empty, the expression string is used, which makes the graph cumbersome. In multi widget dashboards where real estate is scarce, it becomes a real issue.

See my cats widget before the fix:

My cats widget after the fix:

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

gitpod-io · 2021-11-23T01:34:13Z

kaizencc · 2021-11-23T14:20:09Z

packages/@aws-cdk/aws-cloudwatch/lib/metric.ts

@@ -129,7 +129,7 @@ export interface MathExpressionOptions {
  /**
   * Label for this metric when added to a Graph in a Dashboard
   *
-   * @default - Expression value is used as label
+   * @default - The legend will only show the original metric labels


If we change the default here, won't customers be surprised that their stacks have changed upon update? That seems problematic for a stable module.

To have the legend show only the original metric labels, set Expression-Label to be empty.

Instead of omitting the label entirely, can you set label = ''? To me, that seems to follow the cloudformation docs of explicitly setting the label to be empty.

If we change the default here, won't customers be surprised that their stacks have changed upon update? That seems problematic for a stable module.

I would argue that the current default behavior is unexpected, but I see your point about the stability of the API and limiting the change to a specific input

I like Neta's behavior better, actually, so I would recommend doing that. My apologies for implementing the original thing (shittily).

If we are concerned about introducing unexpected changes, I would recommend going with a feature flag instead.

…neta/allow-empty-label

rix0rrr

Why do this with a magic value (label: ''), instead of a property (expressionAsLabel: false) ?

Also, can we autodetect when this might be applicable? Like, when the expression is a search expression?

rix0rrr

After experimenting, changed my mind. It makes sense, sorry. Added some docs to clarify use cases.

mergify · 2022-04-03T10:05:56Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-04-03T10:51:33Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: d378bcc
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-04-03T10:51:54Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/1.152.0/CHANGELOG.md) For convenience, extracted the relevant CHANGELOG entry: ## [1.152.0](v1.151.0...v1.152.0) (2022-04-06) ### Features * **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9)) * **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64)) * **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667) * **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209)) * **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e)) * **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5)) * **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e)) * add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010)) * **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df)) * **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605) * **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076) * **synthetics:** new puppeteer 3.5 runtime ([#19673](#19673)) ([ce2b91b](ce2b91b)), closes [#19634](#19634) ### Bug Fixes * **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969) * **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160) * **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421) * **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2)) * **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042)) * **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399) * **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad)) * **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550) * **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3)) * **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648) * **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259) * **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751) * **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)

See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/2.20.0/CHANGELOG.md) For convenience, extracted the relevant CHANGELOG entry: ## [2.20.0](v2.19.0...v2.20.0) (2022-04-07) ### Features * **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9)) * **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64)) * **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667) * **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209)) * **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e)) * **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5)) * **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e)) * add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010)) * **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df)) * **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605) * **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076) ### Bug Fixes * **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969) * **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160) * **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421) * **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2)) * **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042)) * **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399) * **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad)) * **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550) * **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3)) * **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648) * **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259) * **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751) * **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)

…s#17639) According to CloudWatch [docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/using-metric-math.html): > For the Label column of the expression, enter a name that describes what the expression is calculating. If the result of an expression is an array of time series, each of those time series is displayed on the graph with a separate line, with different colors. Immediately under the graph is a legend for each line in the graph. For a single expression that produces multiple time series, the legend captions for those time series are in the format Expression-Label Metric-Label. For example, if the graph includes a metric with a label of Errors and an expression FILL(METRICS(), 0) that has a label of Filled With 0:, one line in the legend would be Filled With 0: Errors. **To have the legend show only the original metric labels, set Expression-Label to be empty.** In the current implementation, if the label is left empty, the expression string is used, which makes the graph cumbersome. In multi widget dashboards where real estate is scarce, it becomes a real issue. See my cats widget before the fix: <img width="1432" alt="Screen Shot 2021-11-22 at 5 17 35 PM" src="https://user-images.githubusercontent.com/8578043/142959081-f3c28ea9-dd36-431f-b123-262eed0b2625.png"> My cats widget after the fix: <img width="1250" alt="Screen Shot 2021-11-22 at 5 20 34 PM" src="https://user-images.githubusercontent.com/8578043/142959125-2ea5f7b3-9170-43bc-ba8b-233dc1af9700.png"> ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

fix(cloudwatch): allow empty label im a math expression

5e14fbf

github-actions bot added the @aws-cdk/aws-cloudwatch Related to Amazon CloudWatch label Nov 23, 2021

github-actions bot assigned madeline-k Nov 23, 2021

mergify bot added the contribution/core This is a PR that came from AWS. label Nov 23, 2021

NetaNir requested review from madeline-k and kaizencc November 23, 2021 01:34

NetaNir added 3 commits November 22, 2021 17:35

english

885eecb

update tests

461487e

i never liked this linter anyway

79d0982

kaizencc reviewed Nov 23, 2021

View reviewed changes

NetaNir added 3 commits November 27, 2021 15:30

emoty string

c2bf6c7

Merge branch 'neta/allow-empty-label' of github.com:aws/aws-cdk into …

cc97964

…neta/allow-empty-label

revert test

147c399

NetaNir changed the title ~~fix(cloudwatch): allow empty label in a math expression~~ feat(cloudwatch): empty string passed as label in a math expression will use CloudWatch default - the original metric label Nov 27, 2021

scope test

b6f0856

NetaNir requested review from kaizencc and removed request for madeline-k November 27, 2021 23:53

Merge branch 'master' into neta/allow-empty-label

d1d6760

rix0rrr requested changes Mar 30, 2022

View reviewed changes

rix0rrr assigned rix0rrr and unassigned madeline-k Mar 30, 2022

rix0rrr added 2 commits April 1, 2022 15:41

Merge remote-tracking branch 'origin/master' into neta/allow-empty-label

21455ac

Update docs

be3c016

rix0rrr changed the title ~~feat(cloudwatch): empty string passed as label in a math expression will use CloudWatch default - the original metric label~~ fix(cloudwatch): automatic expression label cannot be suppressed Apr 1, 2022

rix0rrr added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Apr 1, 2022

github-actions bot added the p2 label Apr 1, 2022

Change implementation a bit

5d7c26c

rix0rrr changed the title ~~fix(cloudwatch): automatic expression label cannot be suppressed~~ fix(cloudwatch): automatic metric math label cannot be suppressed Apr 1, 2022

rix0rrr approved these changes Apr 1, 2022

View reviewed changes

rix0rrr added 2 commits April 1, 2022 17:18

Also explain dynamic labels in the README

65269f8

Fix compilation of example

598b6a8

rix0rrr approved these changes Apr 2, 2022

View reviewed changes

Fix another example compilation failure

5d104f7

Merge branch 'master' into neta/allow-empty-label

d378bcc

mergify bot merged commit 7fa3bf2 into master Apr 3, 2022

mergify bot deleted the neta/allow-empty-label branch April 3, 2022 10:51

This was referenced Apr 7, 2022

chore(release): 1.152.0 #19781

Merged

chore(release): 2.20.0 #19801

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cloudwatch): automatic metric math label cannot be suppressed #17639

fix(cloudwatch): automatic metric math label cannot be suppressed #17639

NetaNir commented Nov 23, 2021

gitpod-io bot commented Nov 23, 2021

kaizencc Nov 23, 2021

NetaNir Nov 24, 2021

rix0rrr Dec 1, 2021

rix0rrr left a comment

rix0rrr left a comment

mergify bot commented Apr 3, 2022

aws-cdk-automation commented Apr 3, 2022

mergify bot commented Apr 3, 2022

fix(cloudwatch): automatic metric math label cannot be suppressed #17639

fix(cloudwatch): automatic metric math label cannot be suppressed #17639

Conversation

NetaNir commented Nov 23, 2021

gitpod-io bot commented Nov 23, 2021

kaizencc Nov 23, 2021

Choose a reason for hiding this comment

NetaNir Nov 24, 2021

Choose a reason for hiding this comment

rix0rrr Dec 1, 2021

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

mergify bot commented Apr 3, 2022

aws-cdk-automation commented Apr 3, 2022

AWS CodeBuild CI Report

mergify bot commented Apr 3, 2022