-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Create a filter with field:value when last value metric is used on a datatable #160509
Conversation
Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations) |
2c58e23
to
644969e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested all the cases you suggested and everything worked well.
I did find a confusing scenario, but I'm not sure if there's anything we can do.
When you're dealing with an array value but you have "Show array values" turned off (using top_metrics
agg instead of top_hits
), the filter that is created excludes all the documents in the time range, making the metric go blank (ignore the terrible formatting you see in the video, I created an issue for that: #160815).
Screen.Recording.2023-06-28.at.2.41.38.PM.mov
This is definitely because of elastic/elasticsearch#82281 (original issue in Lens #102956).
I did some experiments on top_metrics
and, given an array of numbers, top_metrics
returns a single number that seems to be somehow computed from the numbers in the array, but through no predictable calculation I could figure out.
DELETE abc
POST abc/_doc
{
"arr": [10, 50],
"timestamp": "2020-02-02T00:00:00.000Z"
}
GET abc/_search
{
"size": 0,
"aggs": {
"a": {
"top_metrics": {
"metrics": {"field": "arr"},
"sort": {"timestamp": "desc"}
}
}
}
}
Result:
"aggregations": {
"a": {
"top": [
{
"sort": [
"2020-02-02T00:00:00.000Z"
],
"metrics": {
"arr": 30
}
}
]
}
So in this scenario the filter would get created as arr : 30
which doesn't match any docs. (Also, why 30
? Docs say "A top_metric aggregation on array values may return inconsistent results.")
Anyway, not caused in any way by your PR, but perhaps something we could connect with ES about as a follow-up.
src/plugins/data/common/search/aggs/metrics/filtered_metric.test.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/common/search/aggs/metrics/filtered_metric.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @drewdaemon for the thorough review! So the mentioned problem happens because of precision? Eg day_of_week_i doesn't have this effect and creates a correct filter. I am wondering if we should just block key:value filter for last values when value is of type number and is not integer (so has a fraction). Not sure though. @stratoula WDYT?
@mbondyra I am trying to decide which option is going to cause less confusion. I tend to believe that we should keep it as it is on your PR. I feel that if numeric fields work differently it will cause more confusion. |
Scratch that, it's because of the last value actually being an array, not because of the precision so it wouldn't help anyway. Yeah, it's probably on elasticsearch, let's talk during the sync. |
💚 Build Succeeded
Metrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Fixes #152883
The task says this is the behavior that we want for datatable, but with the way data plugin interacts with Lens, it's actually impossible to modify only for datatable. In my opinion that's an improvement though, we probably want the same behavior for all supported visualization (so also metric).
Scenarios to test:
1,2,3 returns the
price:${value}
filter4. returns
price:exists
filter5. returns
category.keyword:${value}
coming from filter by6.
OR
filter for array values