Fix grafana dashboard lint script #5221
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Nested dashboard panels are now recursively checked - Panel datasource must point to the datasource variable
|
@dapplion do you know why it is required to hard code a value here? it seems like it does not matter what value is used here as long as it is not lodestar/scripts/lint-grafana-dashboards.mjs Line 171 in 466c754 |
|
diff of lint script is hard to reason about because logic to assert panels was extracted into function to be able to call it recursively here is a better diff to understand changes done to diff --git a/scripts/lint-grafana-dashboards.mjs b/scripts/lint-grafana-dashboards.mjs
index 4082300d53..754045bc05 100755
--- a/scripts/lint-grafana-dashboards.mjs
+++ b/scripts/lint-grafana-dashboards.mjs
@@ -214,17 +214,12 @@ function assertTemplatingListItemContent(json, varName, item) {
*/
function assertPanels(panels) {
for (const panel of panels) {
+ // Panel datasource must point to the datasource variable
+ if (panel.datasource) {
+ panel.datasource.type = "prometheus";
+ panel.datasource.uid = `\${${variableNameDatasource}}`;
+ }
if (panel.targets) {
for (const target of panel.targets) {
// All panels must point to the datasource variable
if (target.datasource) {
target.datasource.type = "prometheus";
+ target.datasource.uid = `\${${variableNameDatasource}}`;
- target.datasource.uid = "${DS_PROMETHEUS}";
}
// Disable exemplar
@@ -238,9 +233,5 @@ function assertPanels(panels) {
}
}
}
+ // Recursively check nested panels
+ if (panel.panels) {
+ assertPanels(panel.panels);
+ }
}
} |
|
other interesting approach as mentioned in grafana/grafana#10786 (comment), recommends to replace all uids with null-string (
Other repositories using lodestar dashboards + auto-provisioning are replacing Based on my observations, we could support auto-provisioning without workarounds if we remove the prometheus variable altogether or remove uid references as mentioned above. However, both of those approaches are not ideal as it would not longer be possible to specific which prometheus datasource to use in case there are multiple. |
Performance Report✔️ no performance regression detected Full benchmark results
|
wemeetagain
approved these changes
Mar 2, 2023
|
🎉 This PR is included in v1.6.0 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Dashboard linter script was not catching some references to internal prometheus uids, see #5210 (comment).
Description
scripts/validate-grafana-dashboards.shFurther observations
There are two observations unrelated to the changes in this PR which I am not sure are an issue or not
DS_PROMETHEUSvariable is not referenced by any variable or dashboard, this happens for all dashboards where we haveDS_PROMETHEUSin__inputs, I think this happens though because on import it will replaces the variable with the acutal uid, in my case this seems to bePBFA97CFB590B2093and if I exported without sharing externally"uid": "${DS_PROMETHEUS}"is replaced by"uid": "PBFA97CFB590B2093"Export for sharing externallythere will always be a diff even if same grafana version is used, I am not sure why this replacements happens...it should not set some internal uid here. I guess just have to run./scripts/validate-grafana-dashboards.shlocally to fix this before commiting