This repository has been archived by the owner on Aug 30, 2021. It is now read-only.
Fix for multi-value template vars in tag_values queries #35
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.
This PR addresses an issue we've run into while attempting to create templated dashboards.
Here's our basic setup:
We have 3 template values that are essentially a drill-down—where each template var's allowed values depend on the previous template var(s). e.g.,
where the set of relevant tag values for "Server Roles" depends on the selected Datacenter(s), and the set of relevant tag values for "Hostnames" is dependent on both "Datacenter" and "Server Role".
We have the template vars set up as follows:
The problem we run into is that all of the above vars have "Multi-value" enabled... but the parsing of the
tag_values
params doesn't handle that correctly.Once expanded we end up with queries like this:
but the current parsing logic in
datasource.js
just splits on commas, which breaks the multi-valued "filters". So this PR changes the parsing logic to properly understand multi-values, while preserving existing functionality (and adding lenience for leading/trailing whitespace).By way of an "FYI"—I originally went down the path of defining a grammar for the query syntax, and using Jison to generate a parser. It actually works incredibly well—and is the more correct and robust solution. However, I also recognize that it would constitute a more significant code change, introduce the need to invoke jison again in the future to regenerate the parser any time the grammar were updated, etc. So in the name of being non-invasive, I opted to just adjust the existing regexes in this PR.
But if you're interested in seeing the grammar and how it behaves, see this gist which can be pasted into jison "try it" editor to see it in action. The grammar could also easily be further extended to handle the other query types, such as
tag_names()
andmetrics()
—and provides a far more robust and maintainable solution for that, should it be decided that the process/code overhead is an acceptable tradeoff.I'd potentially be willing to whip up an alternate PR using that solution instead, if preferred -- let me know