dashboard-linter
dashboard-linter copied to clipboard
add rule for legend
It's a good practice to prefix the legend with the instance ID, so that dashboards can show all instances, or only a selected one
Thank you for this @zeitlinger.
However, I don't think that this is a hard and fast rule, but more of a guideline. There are absolutely cases where we would want to aggregate all returned series with an average, or sum. Particularly on a stat panel or pie.
There are other cases where we would prefer to aggregate on a different label, such as cluster
or a SUO specific label.
If we were to include this as a linting rule, I would advise that we have it emit a warning rather than an error to prevent breaking changes.
As an example, running this on the entire grafana/cloud-onboarding repo, this increases the number of integrations/mixins with failures from 13 to 76.
The 13 which were failing before this change are because of similar changes which broke backward compatibility and/or introduced a rule as an error when it was really a guideline, or simply wasn't thought out thoroughly.
A rule like target-counter-agg, which checks to see that an aggregation function is used, or the legend includes any label would be more applicable, without requiring a specific aggregation, or a specific label.
Yes, I agree with @rgeyer , as there are cases when metrics are aggregated by job or cluster, or something else, effectively dropping instance label. Therefore it would be real challenge to enforce such rule. As a general guideline, it would be nice to have this reminder for legend.
Great feedback. Let's see if we can come up with a better rule:
If global aggregation: skip rule
else if aggregation x (e.g. job, cluster) is used: legend should start with {{x}} -
else: legend should start with {{instance}} -
.
Hello @zeitlinger.
I'm doing some much-overdue maintenance and housekeeping here. Do you still have cycles to look at this?
Or perhaps it's best we close it?
let's close it - the pr is probably out of date