Added option to display 'Others' buckets in the pie chart

[FlorianLiers]

The change implements the "Others" bucket for the pie chart as discussed in #1961 or #3817. Moreover, it provides a work-around for #7451 by showing the "Others" buckets.

The implementation may be a base for "Others" buckets for other charts as well. It is backwards compatible, since it is turned off by default. Original implemented for Kibana 4.5 (with ES 2.3.1) but works for 5 as well.

The implementation modifies the results from ES and adds "Others" buckets, if possible. In particular, it can not add such buckets in the very first aggregation for non-count metrics. For sub-aggregations, the parent buckets is used as reference for 100%.

The new feature looks like this:

[elasticsearch-release]

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

[Bargs]

This is a good effort, but unfortunately it doesn't address the two major blockers mentioned in #1961:

• "Others" isn't a real bucket, so it can't support sub-aggregations
• Only works with count metric

We really need elastic/elasticsearch#12411 to fix this issue properly.

[FlorianLiers]

You are right, the pull request does not resolve #1961. So maybe my first sentence was a bit too optimistic. I added a general command to #1961. The following comment is more specific for this pull request.

The proposed change just uses the information already returned by ES and enhances the visualisation with that. The three options are:

• No: No changes in the visualization for users not interested in "others"
• Yes: Adds "others" bucket
  • for all aggregations if the metric is a count metric
  • for sub-aggregations if the metric is a non-count metric
• Yes, for all: Maybe the name is missleading. It just fills up the "outer rings" visually. It does not perform a real sub-aggregation for the "others" bucket (because of the blocking issues you mentioned). I added the option because pie charts sometimes look "nicer" if the outer rings are filled up and are a closed circle.

What do you think? Maybe I have to change the names of the options?

• Yes -> "Yes, if possible"
• Yes, All -> "Yes, if possible and fill-up the outer rings"

[Bargs]

Is there a compelling reason not to fill up the outer ring? It seems like we should either fill them all up, or leave them all empty so that the "other" bucket is always visually consistent across parent and sub-aggregations.

I'll try to play around with this some more and get you more detailed feedback.

I'd also like someone a bit more familiar with the vislib to take a look, @panda01 maybe?

[FlorianLiers]

Since there is no real sub-aggregation for the "others" bucket, the outer ring just repeats the information already present in the parent bucket. If it is not filled up, it is a visual sign that the information is not present. I think that there are good arguments for both and it depends on the use case.

[Bargs]

@FlorianLiers sorry for the lag time on this, we've been busy the last couple weeks trying to get the 5.0 alpha4 release out the door.

I spent some more time with the PR today and I have a few concerns.

• The option only works for the Terms agg, but it's presented as a global pie chart option. The way it's presented now, as a regular user I might expect it to create an others bucket for all aggs where that might be applicable, like the Filters or Range aggs.
• The difference in behavior between Count and non-Count metrics is confusing and potentially misleading. As a user who has no understanding of the implementation of this feature, I might enable the "Others" bucket with a Sum metric, see that there's no "Others" bucket in my chart, and assume the slices I see represent 100% of the docs.
• As it exists now, the code in this PR is modifying results that represent the state of the Elasticsearch response. Once the data runs through this post processing it has diverged from the elasticsearch response. I think this could potentially cause issues for other code that consumes these results. I'm not sure if there's any precedence for this sort of post processing in the Vizlib, so I'd like to get someone else's input.
• If we decided to add this, I would rather see it implemented as an option on the Terms agg, and see it work across all visualization types. But that would only solve the first bullet, not the rest.

I'm torn on this because I totally get how useful this feature is for a particular use case. But an implementation without any changes in Elasticsearch comes with a lot of caveats. I'd love to get some thoughts from another dev who's more familiar with the Viz lib. @rashidkpc or @panda01 do you guys have any thoughts on this?

[rashidkpc]

I agree with @Bargs, the implementation here solves a single problem in the pie chart, but creates other problems.

If we were going to patch the bug in Elasticsearch on the UI side, it would be done on the level of the terms agg. That said, I don't think we should solve this in Kibana at all. "Other" should be a real bucket in Elasticsearch, faking it on the UI side will only lead to confusion.

[jccq]

@rashidkpc following the trail of issues this boils down to an issue that is marked as "explore" and "high hanging fruit" elastic/elasticsearch#12316 (if i am not wrong)

the result is that Kibana 4 has this major gaping hole at the moment How about indeed considering the compromise with the fix at term agg level?

please consider that elasticsearch SEVERAL TIMES returns incomplete/incorrect results and its just after a lot of investigation that one finds out that that is the case (E.g. distinct values). Nobody has warned the users there. I would say this would not be worse thanks for considering

[elasticmachine]

Can one of the admins verify this patch?

[rashidkpc]

I'd take a patch adding notes to the aggregations to explain the approximate nature of them, that absolutely seems reasonable.

The proper place to so solve this is in Elasticsearch. I encourage you to make a note on elastic/elasticsearch#12411

[FlorianLiers]

Thank you for the analysis, @Bargs. If the option should be located the aggregation option, that would be fine for me. If that is the only blocking issue, I would be glad to do this change.

The difference between the count and non-count results seems to be a natural one. How to show a 100% sum, if there is some input missing. I would formulate it the other way round: If a user knows that the results are not showing 100% of the counts, Kibana 4 does not provide any means to show that in the pie chart. And the comments in #1961 indicate that there are a lot of power user that require this feature.

Which “other problems” do you see, @rashidkpc? Are you referring to the same issues @Bargs mentioned or do you see others? I do not see any attempt of ES to fix this issue. The solution I propose is like a browser polyfill for ES: Use a future feature of ES now. :-) As I mentioned in #1961, even if ES is fixing this, it might not the result that the users require. Just as a note: elastic/elasticsearch#12411 is not about a other bucket (it states that “term aggregation now provides other bucket”). It is about the sub-aggregation done within the other bucket. So, as long as you do not require a sub-aggregation in the other bucket, ES will not provide you any other solution as already provided today.

[Bargs]

How to show a 100% sum, if there is some input missing.

That's exactly the problem though, a normal user won't realize that some of the input is missing. If we expose an option on the terms agg called "Show Others" most users will expect it to work for all metrics. If it only works for Count, users will either be disappointed (if they understand the issues) or misled (if they don't).

elastic/elasticsearch#12411 should provide what we need because metrics are simply sub-aggregations. Clint does mention a potential workaround on that ticket though. A "Show Others" option could issue two requests, one for the "Top N terms" and a second for "everything except the Top N terms". Such an implementation should work across all viz types and all metrics. I don't necessarily know what all the ramifications of such an implementation would be off the top of my head, but if it could be accomplished without adding a huge amount of complexity to the existing code, I would be receptive to it.

[ppisljar]

Hmm ... this already has a discuss label on it but just as a note ...

@Bargs

• your first point of option being there for all aggs and not actually doing anything for them ... i agree on that. on the other hand there already are such options in visualize (bound to data extends for example in many cases just doesn't do anything ... it depends on your data really ..). but in this case we could definetely or 1) move it to a different place or 2) add logic to show it only when appropriate.
• when you are talking what a normal user would expect, i think our current implementation is actually off of what a user would expect. at least I was not getting that pie slices actually don't represent a whole ... that there is others ... which is not shown at all ...
• third point about modifying es result ... if i understand correctly that whole agg_reponse module is there just for that. modify/parse the es response and make it ready to be consumed by vislib, so i think that's the right place to add such a behavior.
• 4th point ... yes probably that would be nice. But i don't see this being that important in line charts or area charts, or maps ...

What I feel really strongly about is that we should not just wait for es to solve this for us (specially when it seems this might take a while). Of course for a perfect solution we depend on them, but that doesn't mean we can't offer a big improvement and much better experience to our users before.

[elasticmachine]

Can one of the admins verify this patch?

[Bargs]

i think our current implementation is actually off of what a user would expect

I totally agree, but at least it's consistent. This PR introduces inconsistency and as a result, even more confusion. All of the metrics need to work the same way.

if i understand correctly that whole agg_reponse module is there just for that.

We extract the data from the response but we rarely (if ever?) crunch the data in any way. That's not an argument against additional data processing, just a word of caution that we'll be creating new precedents if we do.

What I feel really strongly about is that we should not just wait for es to solve this for us

If there's a pressing need for this, we should talk to the ES team directly and explain why it's important to us before writing any hacks of our own.

[tbragin]

Per discussion above, marking this as stalled on Elasticsearch for now. We should discuss with them the likelyhood of elastic/elasticsearch#12411 happening, because that is also marked as "stalled".

[JeffBolle]

This has been stuck for months, but there are many users out there who need accurate accounting of the full results set within visualizations, even if there are limitations on the sub aggregations within that "other" block. Living with that limitation is better than the current results that end up being at best incomplete and at worst misleading.

[thomasneirynck]

We seem to be blocked here. I'm going to close this PR for now, since it's not really clear if/when elastic/elasticsearch#12411 is solved, this change would be still the starting point.

Thanks for the contribution @FlorianLiers!

[thomasneirynck]

after talking to @tbragin, reopening this. We will confirm with ES team first before making a call here.

[elasticmachine]

Can one of the admins verify this patch?

[ppisljar]

we decided we will tackle this in another way, but might take a while to happen as we are blocked on few other changes. What we need to do is issue two (or more) requests to elastic search to get all the data that we need and this is currently not possible in visualize.

[JeffBolle]

Is there a new issue or pull request those of us who desperately need this functionality can follow?

[ppisljar]

the old issues are still relevant.

[celesteking]

The "Group other values in separate bucket" still not working. Neither for line charts, nor for bar charts. There's a box to tick, but it does nothing, no "other" bucket is created and no "other" bucket adds up to final result. You should remove the checkbox if it doesn't work.