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

[Lens] Use top metrics instead of top hits for last value function #98733

Closed
flash1293 opened this issue Apr 29, 2021 · 9 comments
Closed

[Lens] Use top metrics instead of top hits for last value function #98733

flash1293 opened this issue Apr 29, 2021 · 9 comments
Labels
enhancement New value added to drive a business result Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@flash1293
Copy link
Contributor

Right now Lens is using the top hits aggregation to power the last value function. This works fine, but comes with a (IMHO pretty large) limitation: It's not possible to sort terms by this metric.

We should add top metrics to the agg config layer and use it for last value in Lens instead of top hit. The limitations of top metrics vs top hits are not relevant for the way we offer this feature in Lens (access to non-aggregatable fields, getting multiple hits)

@flash1293 flash1293 added enhancement New value added to drive a business result Team:Visualizations Visualization editors, elastic-charts and infrastructure Team:AppServices Feature:Lens labels Apr 29, 2021
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@flash1293
Copy link
Contributor Author

cc @wylieconlon I can vaguely remember we discussed this at some point - do you know whether there was anything blocking us from implementing this?

@wylieconlon
Copy link
Contributor

@flash1293 I thought we wanted to add non-aggregated fields in the future, since this is useful for tables. We could generate different aggs for that case if we want to, so it's not a blocker.

I think the main limitation is that it looks like you can't use scripted fields in top_metrics, but you can use runtime fields. The documentation also doesn't list boolean as a type that you can use, but I tested and it seemed to work. From this list.

So overall I think the scripted fields support is the biggest issue, but it is probably solveable.

@timroes
Copy link
Contributor

timroes commented Apr 30, 2021

++ Wylie, I think we should consider those use-cases just as separate operations if needed later for table. Given that we plan to deprecate scripted fields in favor of runtime fields for 8.0 I guess we are fine if we wouldn't support them for Last value specifically. The only problem I see with this is that we could break existing charts using a scripted field in Last value, but I am not sure how realistic that use-case really is and whether it's worth blocking moving (or waiting to move till 8.0) on that backwards compatibility.

@flash1293
Copy link
Contributor Author

There's another limitation which might be more serious - array values are not supported:

DELETE abc

POST abc/_doc
{
  "arr": ["x", "a", "y", "b", "z", "c"],
  "timestamp": "2020-02-02T00:00:00.000Z"
}

GET abc/_search
{
  "size": 0,
  "aggs": {
    "a": {
      "top_metrics": {
                "metrics": {"field": "arr.keyword"},
        "sort": {"timestamp": "desc"}
        
      }
    }
  }
}
  "aggregations" : {
    "a" : {
      "top" : [
        {
          "sort" : [
            "2020-02-02T00:00:00.000Z"
          ],
          "metrics" : {
            "arr.keyword" : "a"
          }
        }
      ]
    }
  }

@wylieconlon
Copy link
Contributor

@flash1293 I tested that finding and you're right, we can't use top_metrics with array. This is a major issue and I don't think we should continue down the path of testing top_metrics in Lens because we can't detect arrays.

@flash1293
Copy link
Contributor Author

Closing this for now - I'm going to open an issue on ES side whether we can lift these limitations in some way - I think there are a bunch of options.

@wylieconlon
Copy link
Contributor

@flash1293 I believe the parent issue is actually this one: elastic/elasticsearch#53194 and I opened a docs issue already: elastic/elasticsearch#72889

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests

4 participants