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

[Infra Monitoring UI] Change node metrics tables to use a terms agg #128645

Closed
Tracked by #115235
miltonhultgren opened this issue Mar 28, 2022 · 5 comments
Closed
Tracked by #115235
Labels
Feature:Metrics UI Metrics UI feature info-needed Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@miltonhultgren
Copy link
Contributor

Currently the tables use a composite aggregation that asks for a single page of 10 000 items, the result is then sorted on the client side (via the Metrics Explorer API).
This means we risk missing a node that is higher relevance due to the shard ordering and that the browser has to do work that Elasticsearch could do for us.

Rather we should aim to have the tables run their own custom aggregation query that uses a terms agg and lets Elasticsearch do the searching. We can do this by using the data plugins default search strategy.
The key part of this query is the bucketing size in the date_range sub aggregation, the Metrics Explorer API has code for this that would be good to re-use. The date_range buckets can then be calculated moving backwards from the end of the timerange (which improves the predictability compared to using a date_histogram).

As a result of doing this we can also simplify the data hooks for the tables, a lot of that code is only there to transform the response from the Metrics Explorer API to something more suited for the tables.

AC

TBD

@miltonhultgren miltonhultgren added Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Mar 28, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@miltonhultgren miltonhultgren changed the title [Infra Monitoring UI] Swap node metrics tables to use a terms agg [Infra Monitoring UI] Change node metrics tables to use a terms agg Mar 28, 2022
@miltonhultgren
Copy link
Contributor Author

A side benefit of doing this is that we'll also make it possible to use non-keyword fields in our tables. The Metrics Explorer API we use today only supports metric aggregations, so showing things like Kubernetes pod phase isn't possible to query for since it's a keyword field.

@roshan-elastic
Copy link

@smith Do you think any Metrics Explorer work is worth doing given you were mentioning there was talk about TSVB dependencies etc?

Trying to figure out whether the priority of this would be impacted...

@smith
Copy link
Contributor

smith commented Mar 8, 2023

@smith Do you think any Metrics Explorer work is worth doing given you were mentioning there was talk about TSVB dependencies etc?

I'm not sure this issue is talking about "Metrics Explorer work" per se, but usage of the APIs currently used by the Metrics Explorer. I don't have any guidance on prioritization of anything related to the Metrics Explorer at this time.

@smith
Copy link
Contributor

smith commented Jun 15, 2023

I don't think it makes sense to do this giving other upcoming changes for asset views.

@smith smith closed this as not planned Won't fix, can't repro, duplicate, stale Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature info-needed Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

No branches or pull requests

4 participants