[ML] Move Index Data Visualizer into separate plugin (Part 1) #100922

qn895 · 2021-05-28T16:41:46Z

Summary

This PR is a follow up of #96408 (comment) and is part of the migration to move the index based Data Visualizer into its own separate plugin. Changes include:

To follow-up in future PR

Add back links to ML job creation cards
Add back View in Data Visualizer card after file import is successful
Move data_loader outside of ML data_visualizer folder

Reviewer tips

Kibana Design Team: no styling changes were made, only moving the files inside plugins/ml toplugins/file_data_visualizer
QA: some functional tests are commented, to be re-enabled in a follow up PR

Testing the Index Data Visualizer

Should work correctly with index pattern and saved search
Should work correctly with all controls (e.g. shard sample, field types, show empty fields)
Refreshing the page should restore previously set controls and table sorting
If user has permission to use Discover, the Discover card should show up in right action panel
In this PR, we are not displaying the Anomaly Detection job creation cards or Data Frame Analytics creation card in the right action panel (to be added in a follow up)
Clicking View in Lens should navigate to Lens page with previously set query, time range, and prefilled chart for the field
Expanding a geo_point or geo_shape field row should show a map with shapes as a layer

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
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This was checked for cross-browser compatibility

jgowdyelastic · 2021-06-07T10:26:04Z

x-pack/plugins/data_visualizer/public/application/common/components/expanded_row/types.ts

@@ -0,0 +1,6 @@
+/*


is this file needed?

Thanks for catching that. Deleted here 76a7b5b

jgowdyelastic · 2021-06-07T10:33:53Z

x-pack/plugins/data_visualizer/public/application/file_data_visualizer/file_data_visualizer.tsx

+    share,
+    security,
+    fileUpload,
+    indexPatternFieldEditor,


it might be worth adding a comment here to say that indexPatternFieldEditor isn't used by the file data visualizer, but we need to add it here as we're sharing a context with the index data visualizer.
At least, that's my assumption as to why it's being added here.
Same for fileUpload in the index data visualizer

It might be better to create a shared component which provides the KibanaContextProvider wrapper. as this code is duplicated across both data visualizers.

it looks like lens should also be added to these lists of services

Thanks for catching that. The indexPatternFieldEditor should not have been in the file data visualizer and have been removed here 76a7b5b. For now the index data visualizer and file data visualizer have different contexts.

Also added lens to the list of services for the index data here visualizer 267d3b7

jgowdyelastic · 2021-06-07T10:48:20Z

x-pack/plugins/data_visualizer/server/capabilities.ts

+) => {
+  core.capabilities.registerProvider(() => {
+    return {
+      dataVisualizer: {


i don't think we need this capability at all. as the switcher does not contain any logic, this will aways be true.

Good point 👍 I removed capabilities registration altogether here b043d6e

jgowdyelastic · 2021-06-07T10:50:34Z

x-pack/plugins/data_visualizer/server/models/data_visualizer/data_visualizer.ts

@@ -0,0 +1,1385 @@
+/*


This file is a bit too big IMO.
This might be a good opportunity to separate out util functions and types to make this more manageable.
Or for a follow up...

Split up the model into other files here 403a8e3

jgowdyelastic · 2021-06-07T11:02:25Z

x-pack/plugins/data_visualizer/server/routes/routes.ts

+          body: results,
+        });
+      } catch (e) {
+        logger.warn(e);


This should probably be logger.error
but i'm not sure we should be just throwing the error to the log like this without any context of where it has come from.
As these errors are passed to the client for display, do we even need to add them to the kibana log?

Removed logging and Logger here 403a8e3

jgowdyelastic · 2021-06-07T11:09:16Z

x-pack/plugins/data_visualizer/server/types/deps.ts

-export class FileDataVisualizerPlugin implements Plugin {
-  setup() {}
-  start() {}
+export interface StartDeps {


StartDeps should live next to SetupDeps
This file probably isn't needed and StartDeps can be moved to the plugin.ts

I think originally StartDeps was in a different types in order avoid circular dependencies because it was also used in server/routes. For now, I moved SetupDeps into the same file for better consistency.

jgowdyelastic · 2021-06-07T11:12:43Z

x-pack/plugins/file_upload/server/utils/runtime_field_utils.ts

+
+type RuntimeType = typeof RUNTIME_FIELD_TYPES[number];
+
+export const isPopulatedObject = <U extends string = string>(


this has already been added to common
x-pack/plugins/data_visualizer/common/utils/object_utils.ts

There would be no harm in moving the two functions below to common too.

This addition is unfortunately in the file_upload plugin, which the data_visualizer plugin depends on. We had to add runtime mapping support for the file_upload/time_field_range end point in this PR and these two functions are used to validate the runtimeMappingsSchema. I think this is something we can consolidate later once we have a way to share utility functions across all of our plugins.

ah ok, sorry i missed the file path and didn't realise this was in a different plugin.

jgowdyelastic · 2021-06-07T11:16:24Z

x-pack/plugins/ml/public/application/components/data_grid/column_chart.scss

@@ -1,9 +1,9 @@
-.mlDataGridChart__histogram {


why do these classes need renaming when they are in ML?
The data visualizer should not rely on any styles in ML

Thanks for catching that. Reverted the ML changes here aaf142c

jgowdyelastic · 2021-06-07T11:22:53Z

x-pack/plugins/ml/public/application/datavisualizer/index_based/index_data_visualizer.tsx

+      getIndexDataVisualizerComponent().then(setIndexDataVisualizer);
+    }
+  }, []);
+  return IndexDataVisualizer ? (


not for this PR, but it would be good to switch this and the file data viz to use React.lazy rather than these custom lazy loaders.
file data viz has FileDataVisualizerWrapper to do this when embedding in home, but I didn't change how ML was doing it as I intended to change it after this PR was in.

I'll add this item to the meta issue #101435 👍

jbudz

limits.yml LGTM

peteharverson · 2021-06-08T11:40:14Z

...n/index_data_visualizer/components/index_data_visualizer_view/index_data_visualizer_view.tsx


  const [lastRefresh, setLastRefresh] = useState(0);

  useEffect(() => {
-    timeBasedIndexCheck(currentIndexPattern, true);
-  }, []);
+    if (!currentIndexPattern.isTimeBased()) {


Note that this check and warning probably only apply when using the data visualizer inside of ML.

Added this item to the meta issue #101435 👍

docs/developer/plugin-list.asciidoc

peteharverson

Gave this a test and functionally LGTM. Just left a couple of earlier comments.

jgowdyelastic

LGTM!

kibanamachine · 2021-06-08T19:48:58Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dataVisualizer`	-	236	+236
`fileDataVisualizer`	181	-	-181
`ml`	1756	1688	-68
total			-13

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
`dataVisualizer`	-	59	+59
`fileDataVisualizer`	19	-	-19
total			+40

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`dataVisualizer`	-	1	+1

Async chunks

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

id	before	after	diff
`dataVisualizer`	-	1.1MB	⚠️ +1.1MB
`fileDataVisualizer`	1.0MB	-	-1.0MB
`ml`	5.9MB	5.8MB	-118.2KB
total			-21.1KB

Page load bundle

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

id	before	after	diff
`dataVisualizer`	-	13.6KB	+13.6KB
`fileDataVisualizer`	12.7KB	-	-12.7KB
`ml`	67.1KB	66.8KB	-255.0B
total			+651.0B

Unknown metric groups

API count

id	before	after	diff
`dataVisualizer`	-	59	+59
`fileDataVisualizer`	19	-	-19
total			+40

async chunk count

id	before	after	diff
`dataVisualizer`	-	2	+2
`fileDataVisualizer`	2	-	-2
total			-0

References to deprecated APIs

id	before	after	diff
`ml`	121	99	-22

History

💔 Build #129847 failed a8e6fbd
💚 Build #129629 succeeded ee5f8ab
💔 Build #129606 failed 5149bf5
💚 Build #129365 succeeded 200f874
💔 Build #129323 failed 4eb627e

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

cc @qn895

kibanamachine · 2021-06-08T19:53:02Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 100922

* master: (54 commits) Implement "select all" rules feature (elastic#100554) [ML] Remove script fields from the Anomaly detection alerting rule executor (elastic#101607) [Security solutions][Endpoint] Update event filtering texts (elastic#101563) [Enterprise Search] Mocks/tests tech debt - avoid hungry mocking (elastic#101107) [FTR] Updates esArchive paths [FTR] Updates esArchive paths [Security Solution][Detection Engine] Adds runtime field tests (elastic#101664) Added APM PHP agent to the list of agent names (elastic#101062) [CI] Restore old version_info behavior when .git directory is present (elastic#101642) [Fleet] Add fleet server telemetry (elastic#101400) [APM] Syncs agent config settings to APM Fleet policies (elastic#100744) [esArchiver] drop support for --dir, use repo-relative paths instead (elastic#101345) Revert "[xpack/test] restore incremental: false in ts project" [Security Solution] Remove Host Isolation feature flag (elastic#101655) [xpack/test] restore incremental: false in ts project [DOCS] Adds link to video landing page (elastic#101413) [ML] Move Index Data Visualizer into separate plugin (Part 1) (elastic#100922) Improve security plugin return types (elastic#101492) [ts] migrate `x-pack/test` to composite ts project (elastic#101441) [App Search] Updated Search UI to new URL (elastic#101320) ...

…100922) (#101665) * Resolve merge conflicts * Make sure CODEOWNERS file is empty

qn895 added 26 commits May 25, 2021 13:36

[ML] Add index visualizer

c0ad968

[ML] Readd support for global state

cb3408d

[ML] Add time buckets & fix dependencies

ed3e3dc

[ML] Working ver

593d862

[ML] Add back and boolean support

751512b

[ML] Remove old files inside ml

101f26b

[ML] Rename files

aa35249

[ML] Move field type icon

f201863

[ML] Create new folder structure

6630d2e

[ML] Organize index_data_visualizer

d0b713c

[ML] Move types into index_data_visualizer folder

8b7d221

[ML] Move more files into file_data_visualizer

c81c986

[ML] Move more files into index_data_visualizer

64a663e

[ML] Add new data visualizer model

8b7cb47

[ML] Remove getVisualizerFieldStats which is not used by dv

c3d1ad9

[ML] Delete redundant folder

0d12c91

[ML] Copy old data visualizer routes to new plugin

f343a9e

[ML] Remove old routes

a8aaf36

[ML] Disable for ml job cards tests for now

9e07266

[ML] Remove todos

89c916e

[ML] Move the toast error to the UI component

bd30435

[ML] Fix map styling

df79e21

[ML] Add runtime_mappings for internal/file_upload/time_field_range

adbcaac

[ML] Move routes into folder

ef65d20

[ML] Update permissions

25b0f57

[ML] Update texts

21a460f

qn895 added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:File and Index Data Viz ML file and index data visualizer labels May 28, 2021

Merge branch 'master' into ml-move-index-data-visualizer

200f874

jgowdyelastic reviewed Jun 7, 2021

View reviewed changes

jbudz approved these changes Jun 7, 2021

View reviewed changes

qn895 mentioned this pull request Jun 7, 2021

[Meta] Move Data Visualizer to its own plugin #101435

Closed

25 tasks

qn895 added 9 commits June 7, 2021 11:13

[ML] Revert ml data test subj/class name changes

aaf142c

[ML] Split up data visualizer model, remove Logger

403a8e3

[ML] Remove empty file and indexPatternFieldEditor

76a7b5b

[ML] Move imports of file_upload

836b5eb

[ML] Update plugin dependencies

267d3b7

Merge upstream/main into branch

e976122

Re-add missing data_visualizer.json

5149bf5

Remove capabilities in data_visualizer

b043d6e

Fix test subjs

ee5f8ab

peteharverson reviewed Jun 8, 2021

View reviewed changes

docs/developer/plugin-list.asciidoc Show resolved Hide resolved

peteharverson reviewed Jun 8, 2021

View reviewed changes

qn895 added 2 commits June 8, 2021 10:23

Update ownership for data_visualizer and file_upload code to be ml

d978ee2

Merge upstream/main into branch

a8e6fbd

jgowdyelastic approved these changes Jun 8, 2021

View reviewed changes

Update estypes after 98266

71e4db8

qn895 added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 8, 2021

qn895 merged commit 65b8dda into elastic:master Jun 8, 2021

qn895 mentioned this pull request Jun 8, 2021

[7.x] [ML] Move Index Data Visualizer into separate plugin (Part 1) (#100922) #101665

Merged

qn895 added a commit that referenced this pull request Jun 9, 2021

[7.x] [ML] Move Index Data Visualizer into separate plugin (Part 1) (#…

0d4edb2

…100922) (#101665) * Resolve merge conflicts * Make sure CODEOWNERS file is empty

peteharverson mentioned this pull request Jun 30, 2021

[ML] Move the index data visualizer into a separate plugin #98939

Closed

10 tasks

qn895 mentioned this pull request Jul 16, 2021

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

Closed

35 tasks

qn895 deleted the ml-move-index-data-visualizer branch December 21, 2021 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Move Index Data Visualizer into separate plugin (Part 1) #100922

[ML] Move Index Data Visualizer into separate plugin (Part 1) #100922

qn895 commented May 28, 2021 •

edited

Loading

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021

jgowdyelastic Jun 7, 2021

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021 •

edited

Loading

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021 •

edited

Loading

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021

jgowdyelastic Jun 7, 2021

qn895 Jun 7, 2021

jgowdyelastic Jun 7, 2021 •

edited

Loading

qn895 Jun 7, 2021 •

edited

Loading

jbudz left a comment

peteharverson Jun 8, 2021

qn895 Jun 8, 2021

peteharverson left a comment

jgowdyelastic left a comment

kibanamachine commented Jun 8, 2021

API count

async chunk count

References to deprecated APIs

kibanamachine commented Jun 8, 2021


		type RuntimeType = typeof RUNTIME_FIELD_TYPES[number];

		export const isPopulatedObject = <U extends string = string>(

[ML] Move Index Data Visualizer into separate plugin (Part 1) #100922

[ML] Move Index Data Visualizer into separate plugin (Part 1) #100922

Conversation

qn895 commented May 28, 2021 • edited Loading

Summary

To follow-up in future PR

Reviewer tips

Testing the Index Data Visualizer

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qn895 Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

qn895 Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

jbudz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 8, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Any counts in public APIs

Async chunks

Page load bundle

API count

async chunk count

References to deprecated APIs

History

kibanamachine commented Jun 8, 2021

💔 Backport failed

qn895 commented May 28, 2021 •

edited

Loading

qn895 Jun 7, 2021 •

edited

Loading

jgowdyelastic Jun 7, 2021 •

edited

Loading

jgowdyelastic Jun 7, 2021 •

edited

Loading

qn895 Jun 7, 2021 •

edited

Loading