[Lens] Combined histogram/range aggregation for numbers #76121

dej611 · 2020-08-27T17:14:57Z

Summary

This PR introduces the combined histogram/range aggregation in Lens as a single operation.

When opening the panel the Granularity range input set to the middle of the range

It is possible to manually change the Granularity level (now there are 7 "ticks" available)

Or go fully manual and explicitly set each interval/range

New range intervals will pick the previous value as new from value:

In case the last range is "open" (the to value is +Infinity) then it's still ok:

It is possible to drag and drop range intervals
An invalid range interval happens when from > to

This PR is still a WIP, the full list of tasks to complete is the following:

Fixes
- Make the range results render
  - Fix the x interval label formatting + tooltips
  - Fix the filtering selection issue
- Make the Max bars work correctly => Now unified range input
- Workout the correct step values for the Granularity UI => compute it based on STEPS and MAX_HISTOGRAM_BARS configured
- Add range/intervals validation logic
- Add Tooltips to the range forms panel
- Revisit the Granularity UI
- Revisit the Auto interval UI
- Fix the Drag and Drop re-order issue in Advanced Editor
Missing features
- Add basic jest tests
  - Fix broken tests
- Add more jest tests
- Combine with Add auto interval to histogram AggConfig #76001 for full auto interval goodness
- Revisit i18n

Related issue #59424

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

…ograms

dej611 · 2020-09-07T17:51:20Z

@cchaos @AlonaNadler The PR is in good shape to be reviewed now.

The panel does not show the Max Intervals (bars) anymore now, as after a discussion with @flash1293, I decided to stop at the development for a good MVP first, instead of an all-in-one solution.

I think that probably the Max Intervals (bars) feature may require some additional discussion about what the user may expect from it, and how to retrieve all the data in order to make that happen, which should be held in a future enhancement step. I preferred to give priority to the base functionality as for now in this PR: let me know what you think.

...k/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx

wylieconlon

Most of this review is based on the idea that there should be consistency between how we expose these options in Visualize and Lens, not because we can't do something different, but because I don't think these differences are intentional.

...k/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx

x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

...k/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx

dej611 · 2020-09-18T15:50:21Z

@wylieconlon @AlonaNadler @cchaos the PR is up-to-date with the latest design anche changes requested.
Please let me know your feedback about this state.

dej611 · 2020-09-21T08:29:27Z

@elasticmachine merge upstream

flash1293

I remember we talked about fixing the "Over time" case to always put the date on the x axis if it's present, but this doesn't seem to be in the latest version:

Could you check what happened there?

Otherwise this looks good to me

src/plugins/data/common/search/aggs/buckets/range.ts

dej611 · 2020-09-21T11:01:23Z

@flash1293 I think this reverted it to handle in a different way: 41a937c (#76121)

It broke the original behaviour but perhaps there's a way to have the same result at higher level?

flash1293 · 2020-09-21T11:19:49Z

@dej611 We can sort that out separately, thanks for linking to the change

wylieconlon

Very nice work! I think we can merge this and make some behavioral tweaks later.

wylieconlon · 2020-09-21T23:29:31Z

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/constants.ts

+export const DEFAULT_INTERVAL = 1000;
+export const AUTO_BARS = 'auto';
+export const MIN_HISTOGRAM_BARS = 1;
+export const SLICES = 6;


My opinion on the granularity settings has changed a bit since we last talked about it. The main reason is that with these settings, there is a big jump from 1 bar to 10-16 bars, which prevents me from getting anything in between. I'm not sure what the right tweak is, but if I could hardcode my "optimal" steps it might be something like [1, 5, 10, 20, 40, 80, 100, 200].

I have some concerns about an hardcoded set of values:

we might have to think about a strategy in case the user specify a maxHistogramBars value which is not covered in the list

because the range input starts with the middle mark, the list might have an odd number of values

Because of 1) I feel we may fall again under a slices approach - maybe it's a rare scenario ok -, while for 2) it's more a stylistic reason.

...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx

AlonaNadler · 2020-09-22T14:02:40Z

Great work @dej611 👍 👏
The PR looks great, big improvement.

the auto ranges works great. The manual custom intervals work fantastic.
Few minor things:
Sometimes when I click on the + nothing happens. But once I clicked on the + multiple times the granularity changes. Ideally every click on + or - changes the preview. example:

We need a text review doc.

dej611 · 2020-09-22T14:18:37Z

Sometimes when I click on the + nothing happens. But once I clicked on the + multiple times the granularity changes. Ideally every click on + or - changes the preview. example:

@AlonaNadler I see.
That is relative to the number of slices set right now compared to the dataset whole range. Even if the numbers of slices goes from 7 to 5 it will still happen based on the dataset size for smaller ones.

flash1293 · 2020-09-22T14:19:16Z

Sometimes when I click on the + nothing happens. But once I clicked on the + multiple times the granularity changes. Ideally every click on + or - changes the preview. example:

This is a limitation we can't work around right now because of the reasons discussed in the last weekly (we don't know the actual data distribution in the config UI). I think the current UI does a good job at working with this limitation but it's not perfect. Once we expose the currently rendered data we can think about how to improve this experience.

cchaos

@dej611 I pushed a design cleanup commit essentially adding some tooltips to the buttons and fixing accessibility and layouts. LGTM from my side. It would be nice if we could eventually add a tooltip generally to the "Granularity" label explaining the concept, but I won't hold up this PR anymore.

kibanamachine · 2020-09-23T10:11:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a0e9aec

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
lens	461	+14	447

page load bundle size

id	value	diff	baseline
lens	1.1MB	+30.2KB	1.1MB

History

💚 Build #76298 succeeded a0e9aec
💚 Build #76201 succeeded cdff81b
💚 Build #76161 succeeded 4cfa4e6
💚 Build #76117 succeeded 40deac1
💚 Build #75781 succeeded ad9f4f4

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* master: (31 commits) skip tests for old pacakge (elastic#78194) [Ingest Pipelines] Add url generator for ingest pipelines app (elastic#77872) [Lens] Rename "telemetry" to "stats" (elastic#78125) [CSM] Url search (elastic#77516) [Drilldowns] Config to disable URL Drilldown (elastic#77887) [Lens] Combined histogram/range aggregation for numbers (elastic#76121) Remove legacy plugins support (elastic#77599) 'Auto' interval must be correctly calculated for natural numbers (elastic#77995) [CSM] fix ingest data retry order messed up (elastic#78163) Add response status helpers (elastic#78006) Bump react-beautiful-dnd (elastic#78028) [Security Solution][Detection Engine] Bubbles up more error messages from ES queries to the UI (elastic#78004) Index pattern - refactor constructor (elastic#77791) Add `xpack.security.sameSiteCookies` to docker allow list (elastic#78192) Remove [key: string]: any; from IIndexPattern (elastic#77968) Remove requirement for manage_index_templates privilege for Index Management (elastic#77377) [Ingest Manager] Agent bulk actions UI (elastic#77690) [Metrics UI] Add inventory view timeline (elastic#77804) Reporting/Docs: Updates for setting to enable CSV Download (elastic#78101) Update to latest rum-react (elastic#78193) ...

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Wylie Conlon <wylieconlon@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Marta Bondyra <marta.bondyra@elastic.co> Co-authored-by: cchaos <caroline.horn@elastic.co>

#78287) Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Wylie Conlon <wylieconlon@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Marta Bondyra <marta.bondyra@elastic.co> Co-authored-by: cchaos <caroline.horn@elastic.co> Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Wylie Conlon <wylieconlon@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Marta Bondyra <marta.bondyra@elastic.co> Co-authored-by: cchaos <caroline.horn@elastic.co>

✨ First range/histogram implementation draft

62ce297

dej611 added release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 Feature:Lens v7.10.0 labels Aug 27, 2020

dej611 added 8 commits September 1, 2020 17:52

✨ Implemented DnD reordering of ranges

dff176f

🐛 Fixed transition from histogram to ranges

4e1c418

Merge remote-tracking branch 'upstream/master' into feature/lens/hist…

8650416

…ograms

✅ Add first batch of tests

02fecd3

Merge remote-tracking branch 'upstream/master' into feature/lens/hist…

cf4f62d

…ograms

✨ Reworked and addressed some more features

e3ca488

🐛 Fix the i18n check issue duplicating symbols

8fda9d8

✏️ Fix typo

a6e6064

dej611 mentioned this pull request Sep 3, 2020

[Lens] Create combined histogram/range aggregation for numbers #59424

Closed

dej611 added 3 commits September 3, 2020 18:34

💄 Align UI to new design

b250e6e

🐛 Fix i18n message duplicate name

3f3d1f7

💄 Match UI mock

67ec91f

cchaos self-requested a review September 4, 2020 19:11

dej611 added 2 commits September 7, 2020 19:33

🐛 Fix x label, tooltip and filters with ranges

83d8067

💄 Revisit UI removing max Bars for now

0bc6887

mbondyra reviewed Sep 8, 2020

View reviewed changes

...k/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx Outdated Show resolved Hide resolved

mbondyra reviewed Sep 8, 2020

View reviewed changes

...k/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx Outdated Show resolved Hide resolved

flash1293 mentioned this pull request Sep 8, 2020

Allow non-primitive data values elastic/elastic-charts#808

Closed

3 tasks

dej611 added 2 commits September 8, 2020 18:59

💄 Reworked the popover behaviour to be reactive on change

f4e0875

♻️ Restored back maxBars

bcb59d6

wylieconlon suggested changes Sep 8, 2020

View reviewed changes

dej611 added 2 commits September 9, 2020 14:58

👌 First batch of review fixes

6fa81b3

💄 Add explanation tooltips

777f630

🏷️ Fix type check issue

a0d2b23

dej611 mentioned this pull request Sep 18, 2020

[Lens] Improve range formatting #77881

Closed

Merge branch 'master' into feature/lens/histograms

ad9f4f4

flash1293 reviewed Sep 21, 2020

View reviewed changes

src/plugins/data/common/search/aggs/buckets/range.ts Outdated Show resolved Hide resolved

wylieconlon approved these changes Sep 21, 2020

View reviewed changes

dej611 added 2 commits September 22, 2020 11:50

👌 Use data test ids

40deac1

👌 Use data plugin range definition

4cfa4e6

♻️ Revert esaggs type refactor

cdff81b

dej611 requested a review from cchaos September 22, 2020 15:48

Adding tooltips and fixing layouts/spacing

a0e9aec

cchaos approved these changes Sep 22, 2020

View reviewed changes

dej611 self-assigned this Sep 23, 2020

dej611 merged commit 0f8043c into elastic:master Sep 23, 2020

dej611 deleted the feature/lens/histograms branch September 23, 2020 08:23

dej611 linked an issue Sep 23, 2020 that may be closed by this pull request

[Lens] Create combined histogram/range aggregation for numbers #59424

Closed

dej611 mentioned this pull request Sep 23, 2020

[7.x] [Lens] Combined histogram/range aggregation for numbers (#76121) #78287

Merged

dej611 mentioned this pull request Sep 24, 2020

[Lens] Remove Over time suggestions for numeric intervals #78442

Merged

1 task

dej611 mentioned this pull request Oct 22, 2020

[Lens] Improved range formatter #80132

Merged

1 task

timroes mentioned this pull request Jan 13, 2021

Easier way to re-order range selectors in Range aggregation #8936

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Combined histogram/range aggregation for numbers #76121

[Lens] Combined histogram/range aggregation for numbers #76121

dej611 commented Aug 27, 2020 •

edited

Loading

dej611 commented Sep 7, 2020

wylieconlon left a comment

dej611 commented Sep 18, 2020

dej611 commented Sep 21, 2020

flash1293 left a comment

dej611 commented Sep 21, 2020

flash1293 commented Sep 21, 2020

wylieconlon left a comment

wylieconlon Sep 21, 2020

dej611 Sep 22, 2020

AlonaNadler commented Sep 22, 2020

dej611 commented Sep 22, 2020

flash1293 commented Sep 22, 2020

cchaos left a comment

kibanamachine commented Sep 23, 2020

[Lens] Combined histogram/range aggregation for numbers #76121

[Lens] Combined histogram/range aggregation for numbers #76121

Conversation

dej611 commented Aug 27, 2020 • edited Loading

Summary

Checklist

dej611 commented Sep 7, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

dej611 commented Sep 18, 2020

dej611 commented Sep 21, 2020

flash1293 left a comment

Choose a reason for hiding this comment

dej611 commented Sep 21, 2020

flash1293 commented Sep 21, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon Sep 21, 2020

Choose a reason for hiding this comment

dej611 Sep 22, 2020

Choose a reason for hiding this comment

AlonaNadler commented Sep 22, 2020

dej611 commented Sep 22, 2020

flash1293 commented Sep 22, 2020

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 23, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

page load bundle size

History

dej611 commented Aug 27, 2020 •

edited

Loading