-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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 cluster variable in mixins #16778
Conversation
Fix mixin cluster parameter otherwise generated mixins looks like this: ```json {"type":"prometheus","uid":"${datasource}"},"label":"cluster","name":"job","query":"label_values(etcd_server_has_leader{job=~\".*etcd.*\"}, job)","refresh":2,"type":"query"}]},"time":{"from":"now-15m","to":"now"},"timezone": "`}}{{ .Values.grafana.defaultDashboardsTimezone }}{{`","title":"etcd","uid":"c2f4e12cdf69feb95caa41a5a1b423d9"}`}} ``` where name is job when the variable name used in dashboard queries is cluster. Signed-off-by: QuentinBisson <quentin@giantswarm.io>
86ce12c
to
46b42a7
Compare
cc @ahrtr as I see you refactored to grafonnet recently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for raising @QuentinBisson.
For context here is the original pr that introduced configurable cluster label: https://github.com/etcd-io/etcd/pull/13255/files
Potential regression when we refactored the mixin recently.
Can take a look at this tonight and provide some feedback.
cc @v-zhuravlev
Thank you @jmhbnz. As you can see in this line https://github.com/etcd-io/etcd/pull/13255/files#diff-007651fd1693f51e6c72b237590b65d2a1a72fa3bba2d24993c2bf417bb25792R1294 the variable name used to be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Thanks for fixing this @QuentinBisson.
cc @v-zhuravlev to double confirm. |
@ahrtr , @QuentinBisson , |
thx @v-zhuravlev for the quick confirmation. |
Fix mixin cluster parameter otherwise generated mixins looks like this:
where name is job when the variable name used in dashboard queries is cluster.
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.
Issue came up here: prometheus-community/helm-charts#3880 (comment)