feat(querier): support dynamic variable ALL for ClickHouse queries#9892
Closed
ixchio wants to merge 4 commits intoSigNoz:mainfrom
Closed
feat(querier): support dynamic variable ALL for ClickHouse queries#9892ixchio wants to merge 4 commits intoSigNoz:mainfrom
ixchio wants to merge 4 commits intoSigNoz:mainfrom
Conversation
This change integrates the existing QueryTransformer with the renderVars function to handle the __all__ case for dynamic variables. When a user selects 'ALL' for a dashboard variable, instead of substituting the literal __all__ value (which would match nothing), we now parse the SQL and remove that filter condition entirely. Changes: - Added convertToVariableValues() to bridge API types to transformer - Added hasAllVars() for quick detection of __all__ variables - Integrated transform step in renderVars() before substitution - Logs transformation diffs for observability in production - Falls back to original query if parsing fails (safe rollout) Tests added for all new functionality. Fixes SigNoz#9889
|
Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗 |
Member
|
See #9889 (comment) |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What
This integrates the existing
QueryTransformerwithrenderVars()to properly handle the__all__case for dynamic variables in ClickHouse queries.Why
When users select "ALL" for a dashboard variable, the filter should be removed entirely - not substituted with the literal
__all__string (which matches nothing).Before:
WHERE service_name = '__all__'→ returns nothingAfter:
WHEREclause for that variable is removed → returns allHow
__all__valueQueryTransformerto parse SQL and remove those filter conditionsTesting
renderVarsflow with__all__Fixes #9889
Note
Implements proper handling of dashboard variables set to
__all__in ClickHouse queries.QueryTransformerintorenderVars()to detect__all__dynamic variables and remove their filter clauses; logs transform results and falls back to original query on errorsconvertToVariableValuesandhasAllVars, plus unit and integration tests inclickhouse_query_test.godeploy/install.shtweaks: more robust Docker start flow on macOS and small messaging/comment cleanupsWritten by Cursor Bugbot for commit 32aabb0. This will update automatically on new commits. Configure here.