Fix autocomplete panic when condition has wrong type #3277
Conversation
| @@ -358,6 +358,8 @@ func createDistinctAttributeIterator( | |||
| } | |||
| keyIter = makeIter(keyPath, parquetquery.NewStringInPredicate([]string{cond.Attribute.Name}), selectAs("key", cond.Attribute)) | |||
| valIter = makeIter(boolPath, pred, selectAs("bool", cond.Attribute)) | |||
| default: | |||
| continue | |||
There was a problem hiding this comment.
Another option is returning an error, but instead opted for ignoring the condition.
The reasoning is that autocomplete should be a smooth experience to the user. If the filtering query has an error, they'll get that feedback via normal search—ie. when they execute the query.
There was a problem hiding this comment.
are we missing case statements for things like kind = server and status = ok?
it seems like we should support auto complete on things like:
{ name = "foo" && kind = server && status = error && span.foo = ...
There was a problem hiding this comment.
That function builds the iterators for the generic attribute columns which don't support those special types.
There was a problem hiding this comment.
thx for the clarification.
agree with your initial thought we should just ignore the bad condition over returning an error.
mapno
requested review from
joe-elliott,
annanay25,
mdisibio,
kvrhdn,
zalegrala,
electron0zero,
ie-pham and
stoewer
as code owners
| @@ -358,6 +358,8 @@ func createDistinctAttributeIterator( | |||
| } | |||
| keyIter = makeIter(keyPath, parquetquery.NewStringInPredicate([]string{cond.Attribute.Name}), selectAs("key", cond.Attribute)) | |||
| valIter = makeIter(boolPath, pred, selectAs("bool", cond.Attribute)) | |||
| default: | |||
| continue | |||
There was a problem hiding this comment.
are we missing case statements for things like kind = server and status = ok?
it seems like we should support auto complete on things like:
{ name = "foo" && kind = server && status = error && span.foo = ...
Co-authored-by: Joe Elliott <joe.elliott@grafana.com>
|
Hello @mapno!
Please, if the current pull request addresses a bug fix, label it with the |
mapno
added
type/bug
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-3277-to-release-v2.3 origin/release-v2.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b61b12acd8844f37f7fee1d6ad15d1acf410755eWhen the conflicts are resolved, stage and commit the changes: If you have the GitHub CLI installed: # Push the branch to GitHub:
git push --set-upstream origin backport-3277-to-release-v2.3
# Create the PR body template
PR_BODY=$(gh pr view 3277 --json body --template 'Backport b61b12acd8844f37f7fee1d6ad15d1acf410755e from #3277{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title "[release-v2.3] Fix autocomplete panic when condition has wrong type" --body-file - --label "type/bug" --label "area/query" --label "backport" --base release-v2.3 --milestone release-v2.3 --webOr, if you don't have the GitHub CLI installed (we recommend you install it!): # Push the branch to GitHub:
git push --set-upstream origin backport-3277-to-release-v2.3
# Create a pull request where the `base` branch is `release-v2.3` and the `compare`/`head` branch is `backport-3277-to-release-v2.3`.
# Remove the local backport branch
git switch main
git branch -D backport-3277-to-release-v2.3 |
|
The autocomplete changes weren't released in v2.3. Nothing to backport! 😅 |
* Fix autocomplete panic when condition has wrong type * Add comment * Update CHANGELOG.md Co-authored-by: Joe Elliott <joe.elliott@grafana.com> --------- Co-authored-by: Joe Elliott <joe.elliott@grafana.com> (cherry picked from commit b61b12a)
* Fix autocomplete panic when condition has wrong type * Add comment * Update CHANGELOG.md Co-authored-by: Joe Elliott <joe.elliott@grafana.com> --------- Co-authored-by: Joe Elliott <joe.elliott@grafana.com> (cherry picked from commit b61b12a)
What this PR does:
It prevents a panic when a filtering query is passed to
/search/tag/<tag>/valueswith a attribute condition (.a=b) wherebis a special type (server/client,ok/error/unset) but.ais of a different type (string,int). For example,span.http.status_code=server. This would create iterators with nil predicates and panics when it callediter.Next().Now, such conditions are ignored and iterators are not built for them.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]