Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Fallback to terms agg search if terms enum doesn’t return results #143619

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Oct 19, 2022

Problem
The suggestion component will currently use the terms enum API if no service names are given, and a normal search (using terms agg) otherwise. The terms enum approach is faster, and works well to provide initial results immediately.
However, when the user starts typing the terms enum API will only return terms that starts with the given search query. This means that the query "rum" won't return the term "opbeans-rum".

Solution
This PR solves this by falling back to ordinary search if terms enum doesn't provide any results. This strikes a balance between speed and accuracy.

Review without whitespace: https://github.com/elastic/kibana/pull/143619/files?diff=unified&w=1

@sorenlouv sorenlouv requested a review from a team as a code owner October 19, 2022 08:06
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Oct 19, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@gbamparop
Copy link
Contributor

If I understand this correctly, it might be confusing to the user as the search strategy changes depending on whether there are data returned form the terms enum API so they will have a different experience in each case.

@sorenlouv
Copy link
Member Author

sorenlouv commented Oct 19, 2022

If I understand this correctly, it might be confusing to the user as the search strategy changes depending on whether there are data returned form the terms enum API so they will have a different experience in each case.

You are right that it will change the search strategy on the fly now. But I think that actually creates the behaviour that users expect. Depending on the context, a component will currently either use terms enum or ordinary search. This means that sometimes the search query must match the start of the search term, and in other cases it can be a partial match in the middle. This is the bigger problem, that this attempts to solve.

What actual problems do you see this causing? It will only do the fall back if terms enum didn't have any results, so it's not like the user is any worse of.

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sorenlouv
Copy link
Member Author

@gbamparop and I discussed this offline and he is ok with the changes.

@sorenlouv sorenlouv merged commit 1def8b5 into elastic:main Oct 19, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 19, 2022
@sorenlouv sorenlouv deleted the fallback-to-terms-agg-search branch October 19, 2022 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:fix Team:APM All issues that need APM UI Team support v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants