[ML] Add Lens and Discover integration to index based Data Visualizer by qn895 · Pull Request #89471 · elastic/kibana

qn895 · 2021-01-27T19:13:06Z

Summary

Part of #86387. This PR brings:

A new card that will link the index to Discover
A way for users to directly visualize the fields in the Lens app (currently supports date, number, IP, and keyword fields)

Small fix for Maps to auto zoom correctly

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
Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-01-27T19:13:08Z

Pinging @elastic/ml-ui (:ml)

pheyos

Good to have functional tests as part of this PR! 🎉 Left a few comments.
Additionally, we might want to add the new features to the permission tests and would also like to validate some discover and lens page content. I'll take a closer look and report back with more details.

…er-integration

…chart, add boolean chart

…er-integration

pheyos

I've noticed that when I open Discover in the same browser tab, it correctly applies the selected time range. But opening the Discover link in a new browser tab falls back to Last 15 minutes. Not sure if this can be fixed on our side.

peteharverson

Tested latest edits - looks good overall. Just one more edit needed for date field types I think.

pheyos

Great to see the added permission tests and the trial / basic license test split!

pheyos

For the extended Discover checks:
We already validate that Discover displays the query as expected. IMO it would be good to also add a second assertion for the hit count of that discover page. That way we can be sure that data is loaded as expected. I've created an example, how this could look like.

For the Lens link validation:
I'd suggest to have a separate test file, because it would interrupt the existing index_data_visualizer tests badly. I think this would be good to have in a follow-up PR because it will require some additional work.
What do you think @peteharverson ?

peteharverson · 2021-01-29T14:59:56Z

Adding the hit count assertion for the Discover drilldown looks like it is worth including in this PR @pheyos.

Adding new tests for the Lens link in a follow-up PR is sensible. Would be great to have even some basic validation that expected content is displayed in the Lens page.

darnautov · 2021-01-29T14:50:22Z

+import { CombinedQuery } from '../../../common';
+
+export function getActions(
+  indexPattern: IndexPattern,


I guess it should be IIndexPattern interface instead

note: we should replace all IndexPattern imports with IIndexPattern later because otherwise, this class end up in our bundle

…er-integration

…r link, types

qn895 · 2021-01-29T17:25:18Z

I've noticed that when I open Discover in the same browser tab, it correctly applies the selected time range. But opening the Discover link in a new browser tab falls back to Last 15 minutes. Not sure if this can be fixed on our side.

Thanks! Good catch. I added timeRange as well as the refreshInterval to the link now so the time range should be consistent, even in a new tab.

peteharverson

Tested latest edits for date fields and the Discover 'open in new tab' behavior, and LGTM

pheyos

Open Discover in new tab works well now for me and the extended Discover checks are looking good! Just one comment around the trial/basic test split:

…er-integration

pheyos

Tested and functional tests LGTM

darnautov

LGTM

…er-integration

kibanamachine · 2021-02-05T17:56:58Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 16ff572

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1739	1741	+2

Async chunks

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

id	before	after	diff
`ml`	6.6MB	6.6MB	+9.0KB

Page load bundle

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

id	before	after	diff
`ml`	57.1KB	57.1KB	+23.0B

History

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

…alizer (#89471) (#90478)

qn895 added 5 commits January 27, 2021 12:41

[ML] Add lens linking

c392036

[ML] Add query to discover

71cc046

[ML] Update tests

bdb54f3

[ML] Update map zoom level

9ab57a7

[ML] Clean up

2e8a18d

qn895 added release_note:enhancement :ml v8.0.0 Feature:File and Index Data Viz ML file and index data visualizer v7.12.0 labels Jan 27, 2021

qn895 requested review from darnautov, jgowdyelastic, peteharverson and pheyos January 27, 2021 19:13

qn895 self-assigned this Jan 27, 2021

qn895 requested a review from a team as a code owner January 27, 2021 19:13

qn895 mentioned this pull request Jan 27, 2021

[Meta][ML] Data Visualizer redesign & plugins integration #86387

Closed

35 tasks

pheyos reviewed Jan 28, 2021

View reviewed changes

peteharverson reviewed Jan 28, 2021

View reviewed changes

qn895 added 5 commits January 28, 2021 11:44

Merge remote-tracking branch 'upstream/master' into ml-dv-lens-discov…

8b1ebda

…er-integration

[ML] Update text, remove assertViewInLensActionNotExists, fix number …

96ff5ac

…chart, add boolean chart

[ML] Update text, remove assertViewInLensActionNotExists, fix number …

60d483d

…chart, add boolean chart

Merge remote-tracking branch 'upstream/master' into ml-dv-lens-discov…

c364f06

…er-integration

[ML] Fix texts

6da9486

pheyos reviewed Jan 29, 2021

View reviewed changes

peteharverson reviewed Jan 29, 2021

View reviewed changes

Comment thread ...c/application/datavisualizer/index_based/components/field_data_row/action_menu/lens_utils.ts Outdated

Comment thread ...c/application/datavisualizer/index_based/components/field_data_row/action_menu/lens_utils.ts Outdated

pheyos reviewed Jan 29, 2021

View reviewed changes

Comment thread x-pack/test/functional/apps/ml/permissions/read_ml_access.ts

Comment thread x-pack/test/functional_basic/apps/ml/data_visualizer/index.ts Outdated

pheyos reviewed Jan 29, 2021

View reviewed changes

darnautov reviewed Jan 29, 2021

View reviewed changes

qn895 added 6 commits January 29, 2021 09:36

Merge remote-tracking branch 'upstream/master' into ml-dv-lens-discov…

55ac0fe

…er-integration

[ML] Add translations, update to IIndexPattern, fix timerange discove…

a84a885

…r link, types

[ML] Update tests

6119e07

[ML] Fix DataRecognizer to use IIndexPattern

2ce76e2

[ML] Fix hidden

98004fa

[ML] Fix assertRecognizerCardNotExists

22563d2

peteharverson approved these changes Feb 1, 2021

View reviewed changes

pheyos reviewed Feb 1, 2021

View reviewed changes

Comment thread x-pack/test/functional_basic/apps/ml/data_visualizer/index_data_visualizer_actions_panel.ts Outdated

qn895 added 2 commits February 1, 2021 09:11

Merge remote-tracking branch 'upstream/master' into ml-dv-lens-discov…

9b83e7e

…er-integration

[ML] Fix duplicate INITIAL_LOCATION and FtrProviderContext

6d6ccb4

pheyos approved these changes Feb 1, 2021

View reviewed changes

darnautov approved these changes Feb 2, 2021

View reviewed changes

qn895 added 4 commits February 3, 2021 10:05

Merge remote-tracking branch 'upstream/master' into ml-dv-lens-discov…

f145d8b

…er-integration

[ML] Update imports to top level

5eb8784

Merge remote-tracking branch 'upstream/master' into ml-dv-lens-discov…

35ea3e2

…er-integration

[ML] Fix license header

16ff572

qn895 merged commit 70d6143 into elastic:master Feb 5, 2021

qn895 deleted the ml-dv-lens-discover-integration branch February 5, 2021 17:59

qn895 mentioned this pull request Feb 5, 2021

[7.x] [ML] Add Lens and Discover integration to index based Data Visualizer (#89471) #90478

Merged

qn895 added a commit that referenced this pull request Feb 5, 2021

[7.x] [ML] Add Lens and Discover integration to index based Data Visu…

bcdfea7

…alizer (#89471) (#90478)

legrego mentioned this pull request Feb 11, 2021

ML Data Visualizer shouldn't link to Discover if it is not available #91198

Closed

Conversation

qn895 commented Jan 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Jan 27, 2021

Uh oh!

pheyos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pheyos left a comment

Choose a reason for hiding this comment

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pheyos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pheyos left a comment

Choose a reason for hiding this comment

Uh oh!

peteharverson commented Jan 29, 2021

Uh oh!

Uh oh!

Uh oh!

darnautov Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

darnautov Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qn895 commented Jan 29, 2021

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

pheyos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pheyos left a comment

Choose a reason for hiding this comment

Uh oh!

darnautov left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 5, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

qn895 commented Jan 27, 2021 •

edited

Loading