-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 label replace in loki-operational.libsonnet #12789
Conversation
@cstyan I would really appreciate the review :) |
@QuentinBisson sorry, I'm failing to visualize in my head what the new value of the label replace would end up inserting as the label, can you help me out here with a more concrete example of what happens currently vs what we should be doing? |
Sure, so the issue is that if we set the per_cluster_label now to |
Okay this makes sense, thanks 👍
can this not cause issues in other places though where we have just |
The previous one should be enough but i'll run it an test. From what I understood, there are 4 use cases:
|
This PR fixes the loki-operational.libsonnet mixin in case the value of per_cluster_label starts with cluster_. For historical reasons, we named the cluster label cluster_id so generation actually replaces all cluster_id with cluster_id_id in this dashboard because the replace is incorrect
@cstyan I checked and there is no other use cases for cluster apart from those 3 replace. The one used in the templating section is managed via the addCluster libsonnet function. We could keep the replace of |
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 digging in and providing an explanation for your changes 👍
This PR fixes the loki-operational.libsonnet mixin in case the value of per_cluster_label starts with cluster_.
For historical reasons, we named the cluster label cluster_id so generation actually replaces all cluster_id with cluster_id_id in this dashboard because the replace is incorrect
What this PR does / why we need it:
This PR fixes the loki operational dashboard when the cluster label starts with cluster_
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)docs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR