Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(stepfunctions): specify line colors for Step Functions Executions graph to ensure failed executions don't show as green #562

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

kamalgill
Copy link
Contributor

The Step Functions Executions graph doesn't specify line colors for the various metrics, which may lead to failed executions displaying as a green line, and succeeded executions displaying as an orange line, as in the screenshot below. This PR specifies line colors to avoid that anomaly.

Screenshot 2024-08-20 at 11 00 05 AM


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

@kamalgill kamalgill changed the title Specify line colors for Step Functions Executions graph to ensure failed executions don't show as green fix(stepfunctions): specify line colors for Step Functions Executions graph to ensure failed executions don't show as green Aug 20, 2024
Copy link

@adamstein-eb adamstein-eb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@echeung-amzn echeung-amzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally omitted these and left it to CloudWatch defaults so we didn't have to handle a11y concerns too.

The motivation for this is understandable though, but can you at least use (/update) the colors in https://github.com/cdklabs/cdk-monitoring-constructs/blob/main/lib/common/widget/color.ts so they can be consistent across other usages, if any?

@kamalgill
Copy link
Contributor Author

@echeung-amzn I have pushed a follow-up commit based on your feedback

Copy link
Member

@echeung-amzn echeung-amzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the build failed. You'll need to run a full build locally (i.e. yarn build) and commit any updated snapshot files.

@kamalgill
Copy link
Contributor Author

@echeung-amzn I have updated the snaphots via yarn build and have squashed my commits in this PR

@echeung-amzn echeung-amzn merged commit b94319b into cdklabs:main Aug 23, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants