[Textbased languages] Render Lens suggestions in Discover #151581

stratoula · 2023-02-20T07:18:59Z

Summary

This PR adds the unified histogram on the text based mode in Discover and depicts Lens suggestions to the users. The users can navigate from it to Lens for further exploration and adding the viz into a dashboard.

Some notes:

Lens now exposes a new api to fetch the suggestions from a text based query. Later it can also be used to return suggestions for the dataview mode (the other part of the aforementioned issue)
Lens visualizations have been updated to return the correct previewIcon (the majority of them were using the empty icon and not the icon that represents them). This icon appears on the visualization selection dropdown
When Lens suggests a table, then the chart is not displayed (Discover already offers a table view)
The legacy metric is excluded from the suggestions as it is not compatible with the text based languages.
The text based languages are treated a bit differently on the unified histogram as they do not work with search sessions.
This feature is on technical preview, so we can iterate more on that on later esql milestones. This is the first iteration required for the milestone 1.
The ESQL search strategy has a default size of 1000 https://github.com/elastic/kibana/blob/main/src/plugins/data/common/search/expressions/essql.ts#L113 which means that this is the max rows that we retrieve. I am keeping the default as is irrelevant from the PR. In ESQL this limitation doesn't exist so I think we are fine.

Flaky test runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1972

Checklist

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

stratoula · 2023-03-01T08:02:52Z

src/plugins/unified_histogram/public/container/hooks/use_state_props.ts

+    return (
+      query &&
+      isOfAggregateQueryType(query) &&
+      (getAggregateQueryMode(query) === 'sql' || getAggregateQueryMode(query) === 'esql')


ℹ️ I am adding this additional check as this will be needed for esql

nit: at least it will make it easier to add a new one :D

Suggested change

(getAggregateQueryMode(query) === 'sql' || getAggregateQueryMode(query) === 'esql')

(['sql', 'esql'].some( mode === getAggregateQueryMode(query)))

stratoula · 2023-03-01T08:05:22Z

x-pack/plugins/lens/public/visualizations/partition/suggestions.ts

@@ -120,7 +120,7 @@ export function suggestions({
    const newShape = getNewShape(groups, subVisualizationId as PieVisualizationState['shape']);
    const baseSuggestion: VisualizationSuggestion<PieVisualizationState> = {
      title: i18n.translate('xpack.lens.pie.suggestionLabel', {
-        defaultMessage: 'As {chartName}',


ℹ️ Some charts were using the As ... format and other just the visualization name. I decided to keep the latter for consistency (the title is depicted on the suggestions dropdown)

dej611

Had a code review on the Lens side: proposed some changes to align the new API with the existing Lens API.

x-pack/plugins/lens/public/lens_suggestions_api.ts

…ified Histogram components, as well as reducing async issues by making lensSuggestionsApi a layout prop and delaying rendering until it's available

…based column updates are handled

Text-based Discover Lens cleanup

davismcphee

Thanks for letting me work with you on getting this over the finish line, and as promised my stamp of approval ✅! LGTM, and I'm excited to get this into main -- a great step forward to a DiscoLens future 🪩

dej611

I think I've found a bug on the unified search introduced with this PR while testing it with Safari:

The feature is super cool @stratoula ! 🎉

On a side note it is not obvious discovering this awesome DiscoLens 🪩 as there's nothing that hints the user the presence of a visualization in SQL mode. Perhaps a basic histogram can be visualized for the simple SELECT * FROM <dataView> query?

On the code review side I've left mostly minor improvements and nitpicks. The only change I would recommend is to rename the stateHelperApi.suggestionsApi into stateHelperApi.suggestions to be in style sync with the other 2 apis.

src/plugins/unified_histogram/public/chart/hooks/use_chart_panels.ts

src/plugins/unified_histogram/public/container/container.tsx

dej611 · 2023-03-27T08:36:29Z

src/plugins/unified_histogram/public/container/hooks/use_state_props.ts

+    return (
+      query &&
+      isOfAggregateQueryType(query) &&
+      (getAggregateQueryMode(query) === 'sql' || getAggregateQueryMode(query) === 'esql')


nit: at least it will make it easier to add a new one :D

Suggested change

(getAggregateQueryMode(query) === 'sql' || getAggregateQueryMode(query) === 'esql')

(['sql', 'esql'].some( mode === getAggregateQueryMode(query)))

src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts

x-pack/plugins/lens/public/plugin.ts

stratoula · 2023-03-27T09:51:29Z

On a side note it is not obvious discovering this awesome DiscoLens 🪩 as there's nothing that hints the user the presence of a visualization in SQL mode. Perhaps a basic histogram can be visualized for the simple SELECT * FROM query?

@dej611 thanx for the nits. We discussed it with the ESQL WG and we decided against it

kibana-ci · 2023-03-27T10:51:56Z

💔 Build Failed

Failed CI Steps

Jest Tests #6

Test Failures

[job] [logs] Jest Tests #6 / Discover component selected data view with time field displays chart toggle
[job] [logs] Jest Tests #6 / Discover histogram layout component render should not render null if there is a search session
[job] [logs] Jest Tests #6 / Discover histogram layout component render should not render null if there is no search session, but isPlainRecord is true
[job] [logs] Jest Tests #6 / Discover histogram layout component reset search button renders the button when there is a saved search
[job] [logs] Jest Tests #6 / Discover histogram layout component reset search button should call resetSavedSearch when clicked

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	446	447	+1
`lens`	891	892	+1
`unifiedHistogram`	57	59	+2
total			+4

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`lens`	592	608	+16
`unifiedHistogram`	23	24	+1
total			+17

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	397.4KB	398.5KB	+1.1KB
`lens`	1.3MB	1.3MB	+1.1KB
`unifiedHistogram`	38.3KB	43.7KB	+5.4KB
total			+7.5KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`lens`	49	48	-1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`lens`	34.0KB	34.3KB	+215.0B
`unifiedHistogram`	5.7KB	5.9KB	+141.0B
total			+356.0B

Unknown metric groups

API count

id	before	after	diff
`lens`	686	702	+16
`unifiedHistogram`	61	64	+3
total			+19

ESLint disabled line counts

id	before	after	diff
`securitySolution`	433	436	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	513	516	+3

History

💚 Build #115535 succeeded 4fef6cb
💚 Build #115298 succeeded 59fde5b
💚 Build #115224 succeeded d0d2175
💚 Build #114916 succeeded 886475b
💛 Build #114270 was flaky 2e3c7b0

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

kibana-ci · 2023-03-27T15:15:51Z

💚 Build Succeeded

Buildkite Build
Commit: 32e212b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	446	447	+1
`lens`	891	892	+1
`unifiedHistogram`	57	59	+2
total			+4

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`lens`	592	608	+16
`unifiedHistogram`	23	24	+1
total			+17

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	397.4KB	398.5KB	+1.1KB
`lens`	1.3MB	1.3MB	+1.1KB
`unifiedHistogram`	38.3KB	43.7KB	+5.4KB
total			+7.5KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`lens`	49	48	-1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`lens`	34.0KB	34.3KB	+215.0B
`unifiedHistogram`	5.7KB	5.9KB	+141.0B
total			+356.0B

Unknown metric groups

API count

id	before	after	diff
`lens`	686	702	+16
`unifiedHistogram`	61	64	+3
total			+19

ESLint disabled line counts

id	before	after	diff
`securitySolution`	433	436	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	513	516	+3

History

💔 Build #115616 failed 8bf34d4
💚 Build #115601 succeeded 344530a
💔 Build #115583 failed 7603649
💚 Build #115535 succeeded 4fef6cb
💚 Build #115298 succeeded 59fde5b

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

stratoula added 12 commits February 20, 2023 09:18

Export lens suggestions

de1ff64

Display Lens suggestion in Discover

00cb8b8

fix checks

bffa8ac

Fix translations

234f0df

Fix jest tests

30298e7

Update jest tests

be41a2a

Fix functional tests

8fb6c45

Fix metric dimensions

40f338d

Add suggestions selector

65a1e4d

Merge branch 'main' into textbased-discover-lens

315fa50

Fix functional test

f184264

Fixes suggestions problem

6f4b9b2

stratoula changed the title ~~Export lens suggestions~~ [Textbased languages] Render Lens suggestions in Discover Feb 21, 2023

stratoula added 15 commits February 22, 2023 12:57

backup of results

42089ae

Add number of results

ec81464

Use the enum for the textBased status

33779f5

Change the implementation for hits

2c5ec62

Fix FTs

08030da

Fix FT test

32869b8

Display the chart icon on the dropdown

6f68bb1

Fix Lens FT

d41e767

Merge branch 'main' into textbased-discover-lens

ab9020c

Add unit tests

5c25035

Add Lens unit tests

de2a97d

Merge branch 'main' into textbased-discover-lens

7c7cf8e

Add jest test for chart

03bd2a7

Adds more unit tests

d3916f0

Write some FTs

201f5af

stratoula commented Mar 1, 2023

View reviewed changes

dej611 reviewed Mar 10, 2023

View reviewed changes

x-pack/plugins/lens/public/lens_suggestions_api.ts Outdated Show resolved Hide resolved

x-pack/plugins/lens/public/lens_suggestions_api.ts Outdated Show resolved Hide resolved

stratoula and others added 17 commits March 10, 2023 14:22

Apply PR comments

83c4c74

Merge with main and resolve conflicts

2e3c7b0

Merge branch 'main' into textbased-discover-lens

886475b

Merge branch 'main' into textbased-discover-lens

d0d2175

Merge branch 'main' into textbased-discover-lens

951eb39

Make text based total hits state internal to Unified Histogram

f510e8c

Revert changes to use_lens_props to respect refetch$

15b8f8b

Add total hits support for text based queries to Unified Histogram

df9b6a7

Add auto refetch support for currentSuggestion

d72d016

Move Lens suggestions out of the state container and into the main Un…

2778344

…ified Histogram components, as well as reducing async issues by making lensSuggestionsApi a layout prop and delaying rendering until it's available

Refetch when current suggestion is changed

ef2d9c5

Add columns comment

a18cd30

Show horizontal rule above table in text based mode

3588bd5

Fix issues with text based total hits behaviour, and update how text …

3bdb2eb

…based column updates are handled

Manage extra Unified Histogram refetches when using text based languages

89ea7b9

Fix Jest tests

c7ec599

Merge pull request #25 from davismcphee/textbased-discover-lens-cleanup

59fde5b

Text-based Discover Lens cleanup

davismcphee approved these changes Mar 24, 2023

View reviewed changes

Merge branch 'main' into textbased-discover-lens

4fef6cb

dej611 reviewed Mar 27, 2023

View reviewed changes

Apply PR comments

7603649

Fix jest tests

344530a

dej611 approved these changes Mar 27, 2023

View reviewed changes

stratoula added 2 commits March 27, 2023 16:09

Nit

8bf34d4

Revert

32e212b

stratoula merged commit dec52ef into elastic:main Mar 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Textbased languages] Render Lens suggestions in Discover #151581

[Textbased languages] Render Lens suggestions in Discover #151581

stratoula commented Feb 20, 2023 •

edited

Loading

stratoula Mar 1, 2023

dej611 Mar 27, 2023

stratoula Mar 1, 2023 •

edited

Loading

dej611 left a comment

davismcphee left a comment

dej611 left a comment

dej611 Mar 27, 2023

stratoula commented Mar 27, 2023

kibana-ci commented Mar 27, 2023 •

edited

Loading

API count

ESLint disabled line counts

Total ESLint disabled count

kibana-ci commented Mar 27, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

	(getAggregateQueryMode(query) === 'sql' \|\| getAggregateQueryMode(query) === 'esql')
	(['sql', 'esql'].some( mode === getAggregateQueryMode(query)))

[Textbased languages] Render Lens suggestions in Discover #151581

[Textbased languages] Render Lens suggestions in Discover #151581

Conversation

stratoula commented Feb 20, 2023 • edited Loading

Summary

Checklist

stratoula Mar 1, 2023

Choose a reason for hiding this comment

dej611 Mar 27, 2023

Choose a reason for hiding this comment

stratoula Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

dej611 Mar 27, 2023

Choose a reason for hiding this comment

stratoula commented Mar 27, 2023

kibana-ci commented Mar 27, 2023 • edited Loading

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

kibana-ci commented Mar 27, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

stratoula commented Feb 20, 2023 •

edited

Loading

stratoula Mar 1, 2023 •

edited

Loading

kibana-ci commented Mar 27, 2023 •

edited

Loading