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

[TSVB] Change term sorting to match Kibana Core #14679

Merged
merged 5 commits into from
Nov 9, 2017

Conversation

simianhacker
Copy link
Member

This PR changes the way TSVB sorts terms aggs to use the entire time range instead of the last bucket. For example, let say you're looking at the last 15 minutes, if you create a series where the metric is "average of system.cpu.user.pct" and set the interval to auto, TSVB currently will calculate the bucket size to be 10 seconds and then create a filter aggregation with a range that encapsulates the last 10 seconds of the time period along with the metric above and sort on the result. With this PR that filter aggregation (with the range) is removed and instead will sort on the average of system.cpu.user.pct for the entire time range (basically averaging all the buckets together).

This will make the results for basic aggregations in TSVB and Kibana Core match. This has been a source of confusion for a few users.

Thoughts? @alexfrancoeur @tbragin @rashidkpc

@simianhacker simianhacker added Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) discuss v6.1.0 v7.0.0 labels Oct 30, 2017
@simianhacker simianhacker changed the title [TSVB] Change term sorting to make Kibana Core [TSVB] Change term sorting to match Kibana Core Oct 30, 2017
@simianhacker
Copy link
Member Author

@monicasarbu This concerns you too :D

@alexfrancoeur
Copy link

@simianhacker woah, big change! While I agree that it is a huge source of confusion for users and am happy we're addressing it, I also believe that having the ability to show the most recent value is one of the stronger features of TSVB. Especially for operational monitoring use case. Will we still allow the option to report on the last full or partial bucket for a series? Or are we completely removing it? Also, how does this affect things like overall aggregations? Are they still useful? Maybe we can sync offline here, I'd just like to understand the impact a bit more.

@simianhacker
Copy link
Member Author

@alexfrancoeur This only affects the order of the terms returned when doing a group by, everything else remains unchanged.

@shaharmor
Copy link
Contributor

This not only fixes the order, it also fixes the issue of not showing the data for all the time range for some terms.

Without the fix:
screen shot 2017-11-05 at 17 37 00

With the fix:
screen shot 2017-11-05 at 17 36 30

This is a blessed fix!

Copy link
Contributor

@mattapperson mattapperson left a comment

Choose a reason for hiding this comment

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

LGTM

@simianhacker
Copy link
Member Author

@alexfrancoeur Any objections? I'm ready to merge this.

@alexfrancoeur
Copy link

@simianhacker LGTM as well, no objections. I think we'll need to make sure #14798 is completed soon to reiterate what it means to be experimental. This will likely affect visualizations in production. @monicasarbu any objections on the beats side for modules?

@shaharmor
Copy link
Contributor

Hey, any update on this?

@simianhacker simianhacker merged commit 680cf1a into elastic:master Nov 9, 2017
simianhacker added a commit that referenced this pull request Nov 9, 2017
* [TSVB] Change term sorting to make Kibana Core

* removing last bucket changes

* Update tests to reflect new sorting metric
@simianhacker
Copy link
Member Author

Back ported to 6.x with 0b3de29

@simianhacker simianhacker deleted the fix-term-sorting branch April 17, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix review v6.1.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants