elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.63k stars 8.22k forks source link

[TSVB] Legend doesn't indicate which series name is being shown, just the split value #75990

Closed wylieconlon closed 4 years ago

wylieconlon commented 4 years ago

TSVB should have the same legend behavior as Visualize when showing duplicate values across different series. Instead, it shows a legend which doesn't indicate the source of any value:

Two series, but which one is shown

Example workflow:

  1. Group by the Top 10 Terms of a field
  2. Create a second layer with the same Group By setting
  3. Configure different metrics, such as "average cpu" vs "max cpu"
  4. They look the same

Expected result, showing unique names for each series:

Screen Shot 2020-08-26 at 10 57 17 AM
elasticmachine commented 4 years ago

Pinging @elastic/kibana-app (Team:KibanaApp)

timroes commented 4 years ago

@markov00 is this something you consider us fixing inside TSVB, or should this rather be addressed on elastic-charts level?

markov00 commented 4 years ago

@timroes I think this should be done directly in TSVB as a default way label the series, is not an issue of charts, but the desired series name should be sent to elastic-charts from TSVB, we just render the configuration prodived.

markov00 commented 4 years ago

There are multiple ways to change the series name in elastic-charts: https://elastic.github.io/elastic-charts/?path=/story/stylings--custom-series-name https://elastic.github.io/elastic-charts/?path=/story/stylings--custom-series-name-config https://elastic.github.io/elastic-charts/?path=/story/stylings--custom-series-name-formatting

timroes commented 4 years ago

I wonder if we would always want to show the metric name or only once we have at least 2 different metrics in there, so that it could actually lead to an ambigious legend. Marco or Wylie, any thoughts on that?

nickofthyme commented 4 years ago

I'm doing this for vislib. I think we could just throw that logic in the charts plugin and use it in tsvb. I can work on this after the vislib pr is merged.

wylieconlon commented 4 years ago

@nickofthyme That sounds best, the existing logic in vislib is working well. @timroes to answer your question, the existing behavior of vislib works fine for me, which is to only add the extra text when it's needed.

nickofthyme commented 4 years ago

@wylieconlon and @timroes I looked into this issue some more and I found that TSVB allows multiple "compounding" aggressions/metrics for a single layer (e.g. Count of Max AvgTicketPrice). Whereas in vislib you can only have one AFAIK. This makes it hard to put this aggression into a human readable string like Max AvgTicketPrice to be used in a label such as ES Air: Max AvgTicketPrice, as done in vislib.

image

But we currently allow the key to be used in the layer title with a user defined value. Thus labeling the series with Series 1 - {{key}} and Series 2 - {{key}} produces something like this...

image

This way the series are discernible to the user without the need to determine the combined human readable aggregation.

I think if anything we could just combine the aggregations and use that when there is more than one layer but to me using the {{key}} title seems like a much better option that is already in place.

timroes commented 4 years ago

@nickofthyme ++ on the {{key}} approach. Will also prevent any breaking changes on charts in this case.

nickofthyme commented 4 years ago

Ok cool. In that case, I'm gonna close this issue. Anyone feel free to re-open if you think it needs a better solution.

nickofthyme commented 4 years ago

To be clear this code is here

https://github.com/elastic/kibana/blob/3c99839cf7a624e7a8201f8636ae8c9b10cd8a9c/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/format_key.js#L20-L25

Looks to be added here (https://github.com/elastic/kibana/pull/11266) in v5.5.0.