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

Chart and legend should be ordered by filter agg order #10543

Closed
Bargs opened this issue Feb 23, 2017 · 16 comments
Closed

Chart and legend should be ordered by filter agg order #10543

Bargs opened this issue Feb 23, 2017 · 16 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) PR sent regression v6.0.0

Comments

@Bargs
Copy link
Contributor

Bargs commented Feb 23, 2017

Kibana version: 5.0+
Elasticsearch version: 5.0+
Server OS version: Any
Browser version: Any
Original install method (e.g. download page, yum, from source, etc.): Any

Note: This is a regression in 5.0

Description of the problem including expected versus actual behavior:

In 4.x charts with filter aggs were ordered according to the order of the filters:

screen shot 2017-02-23 at 2 27 05 pm

In 5.0 the ordering appears to be alphabetical based on the label:

screen shot 2017-02-23 at 2 28 10 pm

I wasn't able to find a PR that intentionally changed this, so it appears to be a regression in 5.0+.

@Bargs Bargs added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) bug Fixes for quality problems that affect the customer experience labels Feb 23, 2017
@thomasneirynck
Copy link
Contributor

thomasneirynck commented Feb 24, 2017

This seems to be a change from Elasticsearch 2.x to 5.x. (EDIT: right here: elastic/elasticsearch@b01e3f0)

The query response for 5.x reorders the filters alphabetically, while 2.x preserves the ordering:


e.g.: given following query agains ES 5.x:

{
  "size": 0,
  "query": {
    "bool": {
      "must": [
        {
          "query_string": {
            "query": "*",
            "analyze_wildcard": true
          }
        },
        {
          "range": {
            "@timestamp": {
              "gte": 1486545962207,
              "lte": 1486674963278,
              "format": "epoch_millis"
            }
          }
        }
      ],
      "must_not": []
    }
  },
  "_source": {
    "excludes": []
  },
  "aggs": {
    "2": {
      "date_histogram": {
        "field": "@timestamp",
        "interval": "30m",
        "time_zone": "America/New_York",
        "min_doc_count": 1
      },
      "aggs": {
        "3": {
          "filters": {
            "filters": {
              "geo.dest: IN": {
                "query_string": {
                  "query": "geo.dest: IN",
                  "analyze_wildcard": true
                }
              },
              "geo.dest: CN": {
                "query_string": {
                  "query": "geo.dest: CN",
                  "analyze_wildcard": true
                }
              }
            }
          }
        }
      }
    }
  }
}

The response has the filters reordered:

{
  "took": 40,
  "timed_out": false,
  "_shards": {
    "total": 1,
    "successful": 1,
    "failed": 0
  },
  "hits": {
    "total": 8634,
    "max_score": 0,
    "hits": []
  },
  "aggregations": {
    "2": {
      "buckets": [
        {
          "3": {
            "buckets": {
              "geo.dest: CN": {
                "doc_count": 2
              },
              "geo.dest: IN": {
                "doc_count": 1
              }
            }
          },
          "key_as_string": "2017-02-08T04:00:00.000-05:00",
          "key": 1486544400000,
          "doc_count": 4
        },
        {
          "3": {
            "buckets": {
              "geo.dest: CN": {
                "doc_count": 2
              },
              "geo.dest: IN": {
                "doc_count": 3
              }
            }
          },
...

Compare this to ES 2.x, which preserves the ordering of the filters:

request:

{
  "size": 0,
  "query": {
    "filtered": {
      "query": {
        "query_string": {
          "query": "*",
          "analyze_wildcard": true
        }
      },
      "filter": {
        "bool": {
          "must": [
            {
              "range": {
                "@timestamp": {
                  "gte": 1473942923418,
                  "lte": 1475246506396,
                  "format": "epoch_millis"
                }
              }
            }
          ],
          "must_not": []
        }
      }
    }
  },
  "aggs": {
    "2": {
      "date_histogram": {
        "field": "@timestamp",
        "interval": "12h",
        "time_zone": "America/New_York",
        "min_doc_count": 1,
        "extended_bounds": {
          "min": 1473942923418,
          "max": 1475246506396
        }
      },
      "aggs": {
        "3": {
          "filters": {
            "filters": {
              "geo.dest: IN": {
                "query": {
                  "query_string": {
                    "query": "geo.dest: IN",
                    "analyze_wildcard": true
                  }
                }
              },
              "geo.dest: CN": {
                "query": {
                  "query_string": {
                    "query": "geo.dest: CN",
                    "analyze_wildcard": true
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

response, which preserves the ordering:

{
  "took": 160,
  "timed_out": false,
  "_shards": {
    "total": 3,
    "successful": 3,
    "failed": 0
  },
  "hits": {
    "total": 14005,
    "max_score": 0,
    "hits": []
  },
  "aggregations": {
    "2": {
      "buckets": [
        {
          "3": {
            "buckets": {
              "geo.dest: IN": {
                "doc_count": 10
              },
              "geo.dest: CN": {
                "doc_count": 11
              }
            }
          },
          "key_as_string": "2016-09-19T12:00:00.000-04:00",
          "key": 1474300800000,
          "doc_count": 64
        },
        {
          "3": {
            "buckets": {
              "geo.dest: IN": {
                "doc_count": 651
              },
              "geo.dest: CN": {
                "doc_count": 730
              }
            }
          },
          "key_as_string": "2016-09-20T00:00:00.000-04:00",
          "key": 1474344000000,
          "doc_count": 3918
        },
        {
          "3": {
            "buckets": {
              "geo.dest: IN": {
                "doc_count": 110
              },
              "geo.dest: CN": {
                "doc_count": 114
              }
            }
          },
          "key_as_string": "2016-09-20T12:00:00.000-04:00",
          "key": 1474387200000,
          "doc_count": 716
        },
...

@thomasneirynck
Copy link
Contributor

Strictly speaking, given that JSON maps do not determine order, Kibana shouldn't have relied on the ordering of the ES-response to determine the order of the legend to begin with. So this was already present in 4.x, but due to the implementation in ES 2.x, it didn't show up for users. Due to the change in implementation in ES 5.x, it now shows up in Kibana.

cc @clintongormley

@Bargs
Copy link
Contributor Author

Bargs commented Feb 24, 2017

We're definitely doing some sorting beyond what ES returns. This screenshot is from master:

screen shot 2017-02-24 at 11 29 54 am

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Feb 24, 2017

@Bargs you're right about that. That screenshot is from the request-body. Kibana uses the ordering of the ES-responses to determine the ordering of the legend, which wasn't problematic, until it was.

@Bargs Bargs changed the title Chart and legend are no longer ordered by filter agg order Chart and legend should be ordered by filter agg order Feb 24, 2017
@Bargs
Copy link
Contributor Author

Bargs commented Feb 24, 2017

You're right, the lack of context in that view bites me all the time. #10569

@thomasneirynck
Copy link
Contributor

Adding the discuss label: changing this to match the 4.x behavior, will change the behavior for existing 5.x users. We'll need to determine which order is the "correct" one. Alphabetical order (5.x) , or order in the filter-UI (4.x).

@tbragin
Copy link
Contributor

tbragin commented Feb 26, 2017

Personally, I prefer the 4.x behavior, because there the user at least has some way to control the order. I actually ran into this myself in 5.0 as a user (not realizing what the 4.x behavior was). I ended up having to thoughtfully craft my labels just to get them to show up in a particular order... it was quite frustrating.

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Feb 27, 2017

@GlenRSmith @ppisljar @tbragin discussed and in agreement, 4.x was the correct behavior. Kibana-legends should respect the order in the UI. This means the alphabetical sorting currently in 5.0, 5.1, and 5.2 is considered "wrong".

@varunsharma27
Copy link
Contributor

@thomasneirynck I'm currently working on fixing this Issue (Preserving Filter Order), could you please help me out a little? Regarding the code path (Request <-> Response) for visualizations.

@thomasneirynck
Copy link
Contributor

hi @varundbest, do you have something up already (this can be rough)? If so, you can create a PR, and we can discuss implementation details on the PR iso this ticket.

@varunsharma27
Copy link
Contributor

@thomasneirynck Nope, i'm looking for some hints though. It's kind of a blocker for us and I need to finish it up ASAP.

@thomasneirynck
Copy link
Contributor

hi @varundbest, this is something we're looking to address for the next major release (v6). As it looks now that would go out sometime early fall. We'll unlikely backport this to 5.x, giving we'd be swapping the current behavior yet again, and like to keep breaking changes for majors. Would that be OK timeframe for you?

If not, let me know, and we can set something up to help you get going.

@varunsharma27
Copy link
Contributor

@thomasneirynck That would actually be a little too late for us. We want our vertical bar charts to be sorted based on day of week, kibana does alphabetical ordering.
I checked the source code and request <-> response code path seems a little all over the place and I think there are multiple paths based on the type of visualisation, Editor and Dashboard panel (I'm not sure) .
A basic implementation would be having an option "Preserve Order" in filter aggregation and if that's set to true then we re-order response based on our request.
And I'm unable to comprehend the workflow.

@thomasneirynck
Copy link
Contributor

hi @varundbest, I agree code-path is not easy to follow. That's an ongoing work in progress. there's some improvement in that area in the upcoming 6.0 (#11786).

As for your outline on making this optional. That sounds good, but it's unfortunate that this will clutter the UI even more. That was one of the reasons we're hesitant to backport this to v5. Changes in default behavior are acceptable in Majors, but that's more difficult for patche-releases.

After some internal discussion, we're now reconsidering backporting this to v5 branches once this is addressed. I wish I could give you a clearer date, but all we can commit too right now it's tentatively scheduled for v6 ga. We'd backport it at the same time. so depending on when this gets done, it could be available sooner than fall in a v5 patch. But if possible, we'd like to avoid a backport altogether.

Lastly, your use-case is fairly common, and I know of others who have done a similar type of ordering of days of week in v5. Thes users used custom labels, and preceded them with 1-2-3-4-5-6-7 so he ordering would match sun-sat, or mon-sun, or whatever it needed to be. Would that work as a temporary work-around for you?

cc @alexfrancoeur

@Sinmson
Copy link

Sinmson commented Aug 11, 2017

hey @thomasneirynck , all the version 6.0 alphas got releas. Kibana 6. I could not read out of this post that the legend ordering changed. Do you know something about this?

@ppisljar
Copy link
Member

we hope to fix this in time for 6 and also backport it to 5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) PR sent regression v6.0.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants