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] Math operation with time shift can lead to error when changing the aggregation order #146200

Closed
dej611 opened this issue Nov 23, 2022 · 4 comments · Fixed by #150406
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@dej611
Copy link
Contributor

dej611 commented Nov 23, 2022

Describe the bug:

Create a date histogram with a count() - count(shift='7d') formula metric. Now add a Top values breakdown and disable the Aggregate by this dimension first option.

This error will appear:
Screenshot 2022-11-23 at 18 58 11

If the formula does not contain the math operation, then all works fine (count(shift='7d') only).

@dej611 dej611 added bug Fixes for quality problems that affect the customer experience Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Nov 23, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

@dej611
Copy link
Contributor Author

dej611 commented Nov 23, 2022

Found when testing #144564

@drewdaemon
Copy link
Contributor

May be related to/caused by optimizations introduced in #140859

@stratoula
Copy link
Contributor

stratoula commented Feb 6, 2023

No this also happens to 8.5. I tested how it worked on 8.4.3 and I see a different behavior there.

image

On the vertical axis I have count() - count(shift='7d')

The same chart on 8.5++ fails with the bug mentioned above. The interesting thing is that on the lens documentation we have https://github.com/elastic/kibana/blob/main/docs/user/dashboard/lens.asciidoc
image

So taking under consideration this, the behavior of 8.4 seems to be the correct one.

Otherwise, if something changed (that I don't remember) then the bug above happens here https://github.com/elastic/kibana/blob/main/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts#L212

There are no buckets returned for this agg so if I change this line from agg.buckets to agg?.buckets it won't fail

image

So the fix is easy, the question is which is the correct behavior? @dej611 or @drewdaemon do you remember?

@stratoula stratoula self-assigned this Feb 7, 2023
stratoula added a commit that referenced this issue Feb 7, 2023
## Summary

Closes #146200

This bug can easily be replicated with the following scenario:
1. Add a date histogram on the x axis
2. Add a formula with shift (use different timeshifts) i.e. count() -
count(shift=1d)
3. Add a terms breakdown and switch off the Aggregate by this function
first

The fi is easy, it fails on the each look as it requires the agg to have
buckets but it doesnt at this scenario. I also added a test.

How it looks now
<img width="1049" alt="image"
src="https://user-images.githubusercontent.com/17003240/217195447-3b6edb8d-0814-4148-888d-9bd64e924c76.png">


### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
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:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants