-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Infrastructure UI] Improve alerts query and fix import #152549
[Infrastructure UI] Improve alerts query and fix import #152549
Conversation
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The duplicate request problem has been solved
|
||
return buildEsQuery(undefined, queries, filters); | ||
return buildEsQuery(undefined, [], filters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't we passing a data view here? Is it because we have more control over the filters that are passed to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As done in the observability
plugin, the data view was not passed because we need more control over those filters yeah! This way we are just taking care of passing the list of hosts, the data range and the alert status, without the need to retrieve the alerts data view.
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
## 📓 Summary Closes elastic#152544 The issue was caused by a wrong import, where we were directly importing the hook implementation instead of the context hook exposed by `constate`. This PR also improves a bit the alerts status filtering switching from `match_phrase` to a `term` query. ## 🧪 Testing - Navigate to the Hosts View - Open dev tools - Refresh the page or trigger a new search - Verify the `/snapshot` API is called once to retrieve the table metrics --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
📓 Summary
Closes #152544
The issue was caused by a wrong import, where we were directly importing the hook implementation instead of the context hook exposed by
constate
.This PR also improves a bit the alerts status filtering switching from
match_phrase
to aterm
query.🧪 Testing
/snapshot
API is called once to retrieve the table metrics