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

Add support for statistic field in CloudwatchMetricsTarget #543

Merged
merged 2 commits into from
May 18, 2023

Conversation

Butterneck
Copy link
Contributor

What does this do?

Add support for statistic field in CloudwatchMetricsTarget

Why is it a good idea?

The existing statistics field is being ignored by grafana 9.x, it must be named statistic (not statistics) and must be a string (not a list)

Copy link
Collaborator

@JamesGibo JamesGibo left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, I do not use cloudwatch so will have to trust that this is required, but as this is not a breaking change I am happy to add it.
Please could you add a little description of what the difference is between the 2 fields to the doc strings?

grafanalib/cloudwatch.py Show resolved Hide resolved
@Butterneck
Copy link
Contributor Author

Honestly I'm not able to answer your (legitimate) questions, since I could not find any documentation about it.

I was trying to create a dashboard that uses CloudWatch for some panels and through some reverse engineering I found that setting the field "statistics" did not produce any effect (the query is considered wrong), calling it "statistic" and changing it's type from list to string, instead, works fine.

I've just added a new field instead of replacing the existing one, since "statistics" seems to have a meaning at some points (https://grafana.com/docs/grafana/latest/datasources/aws-cloudwatch/template-queries-cloudwatch/#query-variable); and I've added it just to v9 because I've tested it with this version.

grafanalib/cloudwatch.py Outdated Show resolved Hide resolved
grafanalib/cloudwatch.py Show resolved Hide resolved
@JamesGibo JamesGibo merged commit c46a600 into weaveworks:main May 18, 2023
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