[data views] Move data views api from data plugin and into its own #113497

mattkime · 2021-09-30T03:57:00Z

Summary

Moving the data views api from the data plugin into its own plugin to shrink the data plugin bundle size and for better separation of concerns.

In order to minimize the impact of this change, the data plugin provides the data views api and re-exports data view exports. This should be removed once all api consumers are using the data view plugin directly.

mattkime · 2021-10-03T23:02:10Z

@elasticmachine merge upstream

mattkime · 2021-10-04T01:55:08Z

@elasticmachine merge upstream

mbondyra

VisEditors changes LGTM 🆗

jgowdyelastic

ML changes LGTM

Dosant

LGTM 🚀

Noticed couple issues that could be addressed:

Dosant · 2021-10-04T09:10:35Z

src/plugins/data_views/jest.config.js

+  preset: '@kbn/test',
+  rootDir: '../../..',
+  roots: ['<rootDir>/src/plugins/data_views'],
+  testRunner: 'jasmine2',


could try to remove 'jasmine2' override here. maybe it would all pass with default runner

Dosant · 2021-10-04T09:13:26Z

src/plugins/data_views/tsconfig.json

+    "server/**/*.json"
+  ],
+  "references": [
+    { "path": "../../core/tsconfig.json" },


Likely not all of these references needed for data_views plugin

x-pack/plugins/translations/translations/ja-JP.json

mistic

Changes under operations team code owners LGTM

… into data_views_to_own_plugin

spalger

I think the intention here is good, but I don't think splitting views out of the data plugins is actually helping. Splitting plugins is not a good way to separate concerns and the overall impact on initial page load bundles is +11kb. I think we should take another approach here.

mistic

I've misread the bundles info here. If the goal here was to reduce the overall bundles size, it actually did the opposite. Thanks @spalger for double checking this

mattkime · 2021-10-04T14:03:34Z

Splitting plugins is not a good way to separate concerns

I'm confused by this in part because this PR is pretty clean meaning there was a relatively good separation of concerns to begin with. Having two plugins instead of one should encourage maintaining this separation. Is there an alternative approach you recommend?

and the overall impact on initial page load bundles is +11kb.

We have a goal to keep bundle sizes below 100kb but I'm unaware of any guidelines around limiting total payload growth, particularly around something as small as 11kb.

I haven't investigated it yet, but I suspect the additional 11kb is because the data plugin reexports most of the data views plugin exports. This is to limit the overall impact on the code base due to splitting plugins. That said, I suspect you have more experience thinking about these things - what overhead can we expect introducing a new plugin? How many kb?

mattkime · 2021-10-04T14:05:38Z

I've misread the bundles info here. If the goal here was to reduce the overall bundles size, it actually did the opposite. Thanks @spalger for double checking this

No, the goal was to reduce the size of the data plugin, which it did. The goal was not to reduce overall bundle size - I would expect some amount of combined bundle size increase by separating plugins. This is 2% growth compared to the starting point. Its not desired but I don't understand why it would dissuade from moving forward with this.

Here's where we split out field formatters from the data plugin - #107173 - Page load bundle sizes grew by 14kb and more specifically a 11.7kb growth from data plugin including field formatters vs field formatters in its own plugin.

tylersmalley · 2021-10-04T14:49:41Z

@mattkime, the point of reducing plugin bundles is to reduce the overall page load. Details can be found in this issue: #95853

kibanamachine · 2021-10-04T14:52:35Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	571	536	-35
`dataViews`	-	43	+43
total			+8

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
`data`	2800	2796	-4
`dataViews`	-	531	+531
total			+527

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
`data`	44	43	-1
`dataViews`	-	6	+6
total			+5

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
`data`	49	48	-1
`dataViews`	-	5	+5
total			+4

Page load bundle

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

id	before	after	diff
`data`	491.0KB	464.4KB	-26.6KB
`dataViews`	-	38.1KB	+38.1KB
`visualizations`	32.8KB	32.8KB	+20.0B
total			+11.5KB

Unknown metric groups

API count

id	before	after	diff
`data`	3186	3181	-5
`dataViews`	-	671	+671
total			+666

References to deprecated APIs

id	before	after	diff
`apm`	57	73	+16
`canvas`	64	67	+3
`dashboard`	217	240	+23
`data`	2	463	+461
`dataViews`	-	231	+231
`dataVisualizer`	146	242	+96
`discover`	1135	1790	+655
`fleet`	33	39	+6
`graph`	102	168	+66
`indexPatternEditor`	53	85	+32
`indexPatternFieldEditor`	100	156	+56
`indexPatternManagement`	306	496	+190
`infra`	358	507	+149
`inputControlVis`	209	325	+116
`lens`	248	348	+100
`maps`	1176	1689	+513
`ml`	466	704	+238
`monitoring`	39	53	+14
`observability`	112	166	+54
`osquery`	22	34	+12
`reporting`	25	29	+4
`savedObjects`	26	44	+18
`savedObjectsManagement`	17	29	+12
`security`	61	71	+10
`securitySolution`	878	1008	+130
`stackAlerts`	130	183	+53
`timelines`	75	84	+9
`transform`	96	148	+52
`uptime`	16	26	+10
`visDefaultEditor`	141	223	+82
`visTypeTimelion`	36	45	+9
`visTypeTimeseries`	226	347	+121
`visTypeVega`	29	35	+6
`visualizations`	89	104	+15
`visualize`	93	99	+6
total			+3568

History

💛 Build #157363 was flaky a844127
💔 Build #157355 failed 9980dbb
💔 Build #157351 failed 6faa465
💚 Build #157344 succeeded 6d7d848
💚 Build #157291 succeeded 803e818

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

spalger · 2021-10-04T17:04:15Z

@mattkime and I have discussed this at length and agree that this doesn't meet the goal of improving page load time or reducing bundle sizes, but think that it ultimately is worth merging for the purpose of encapsulating the "data views" and eventually being able to move that into a package. I've agreed to work with the app services team and help consult on further efforts to reduce the size of the data plugin, which we really should be able to do.

We will prevent many data* plugins from being created and I think we can accept this 11.5kb debt for now.

kibanamachine · 2021-10-04T17:08:43Z

💔 Backport failed

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

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

…lastic#113497) * initial pass at moving data views into own plugin * require expressions plugin, fix META_FIELDS reference * bundle limits and localization * fix integration test * update plugin list and jest config * type fixes * search fixes * fix localization * fix mocks * fix mocks * fix stub * type fixes * fix import on test file * path fixes * remove shorted dotted from data plugin * more todo removal * eslint fixes * eslint fix * simplify data views server plugin * simplify data views server plugin * simplify data views server plugin * fix imports on api routes * fix imports on api routes * update plugin list * ts fixes * ts fixes * add deprecation notice * fix circular dependency and api integration test * fix circular dependency and api integration test * rename types for better clarity * path fixes * jest.config and tsconfig cleanup Co-authored-by: Kibana Machine <[email protected]>

…own (#113497) (#113833) * [data views] Move data views api from data plugin and into its own (#113497) * initial pass at moving data views into own plugin * require expressions plugin, fix META_FIELDS reference * bundle limits and localization * fix integration test * update plugin list and jest config * type fixes * search fixes * fix localization * fix mocks * fix mocks * fix stub * type fixes * fix import on test file * path fixes * remove shorted dotted from data plugin * more todo removal * eslint fixes * eslint fix * simplify data views server plugin * simplify data views server plugin * simplify data views server plugin * fix imports on api routes * fix imports on api routes * update plugin list * ts fixes * ts fixes * add deprecation notice * fix circular dependency and api integration test * fix circular dependency and api integration test * rename types for better clarity * path fixes * jest.config and tsconfig cleanup Co-authored-by: Kibana Machine <[email protected]> * localization fix Co-authored-by: Kibana Machine <[email protected]>

mattkime added 3 commits September 29, 2021 22:56

initial pass at moving data views into own plugin

c0f91f7

require expressions plugin, fix META_FIELDS reference

a5d2e0d

bundle limits and localization

10aab14

mattkime changed the title ~~initial pass at moving data views into own plugin~~ [data views] Move data views api from data plugin and into its own Sep 30, 2021

mattkime added 15 commits September 29, 2021 23:32

fix integration test

81eae52

update plugin list and jest config

78c639d

type fixes

93ec684

search fixes

3487531

fix localization

457389b

fix mocks

dd14e3a

fix mocks

7e6b177

fix stub

6d27810

type fixes

3a23774

fix import on test file

0711f96

path fixes

8bfff12

remove shorted dotted from data plugin

04d0e59

more todo removal

606383a

eslint fixes

d675c1c

eslint fix

0d933ce

mattkime added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes Team:AppServicesSv v7.16.0 v8.0.0 labels Oct 1, 2021

mattkime added 5 commits October 1, 2021 14:47

simplify data views server plugin

429e23b

simplify data views server plugin

115f25c

simplify data views server plugin

b405d5c

fix imports on api routes

cef2fd6

fix imports on api routes

544b5ce

mattkime requested review from a team as code owners October 3, 2021 23:01

kibanamachine and others added 2 commits October 3, 2021 19:02

Merge branch 'master' into data_views_to_own_plugin

6faa465

path fixes

9980dbb

mattkime requested a review from a team as a code owner October 3, 2021 23:42

Merge branch 'master' into data_views_to_own_plugin

a844127

mbondyra approved these changes Oct 4, 2021

View reviewed changes

pgayvallet approved these changes Oct 4, 2021

View reviewed changes

jgowdyelastic approved these changes Oct 4, 2021

View reviewed changes

Dosant approved these changes Oct 4, 2021

View reviewed changes

mistic approved these changes Oct 4, 2021

View reviewed changes

mattkime added 2 commits October 4, 2021 07:47

jest.config and tsconfig cleanup

2345897

Merge branch 'data_views_to_own_plugin' of github.com:mattkime/kibana…

293d7dc

… into data_views_to_own_plugin

spalger reviewed Oct 4, 2021

View reviewed changes

mistic requested changes Oct 4, 2021

View reviewed changes

mattkime merged commit bbb2e96 into elastic:master Oct 4, 2021

mattkime mentioned this pull request Oct 4, 2021

[7.x] [data views] Move data views api from data plugin and into its own (#113497) #113833

Merged

This was referenced Oct 5, 2021

[data views] Review data views plugin / strategy for reducing bundle size #113922

Closed

[index patterns] break index patterns out of data plugin into its own plugin #104367

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[data views] Move data views api from data plugin and into its own #113497

[data views] Move data views api from data plugin and into its own #113497

mattkime commented Sep 30, 2021 •

edited

Loading

mattkime commented Oct 3, 2021

mattkime commented Oct 4, 2021

mbondyra left a comment

jgowdyelastic left a comment

Dosant left a comment

Dosant Oct 4, 2021

Dosant Oct 4, 2021

mistic left a comment

spalger left a comment

mistic left a comment

mattkime commented Oct 4, 2021

mattkime commented Oct 4, 2021 •

edited

Loading

tylersmalley commented Oct 4, 2021 •

edited

Loading

kibanamachine commented Oct 4, 2021

API count

References to deprecated APIs

spalger commented Oct 4, 2021

kibanamachine commented Oct 4, 2021

[data views] Move data views api from data plugin and into its own #113497

[data views] Move data views api from data plugin and into its own #113497

Conversation

mattkime commented Sep 30, 2021 • edited Loading

Summary

mattkime commented Oct 3, 2021

mattkime commented Oct 4, 2021

mbondyra left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

Dosant left a comment

Choose a reason for hiding this comment

Dosant Oct 4, 2021

Choose a reason for hiding this comment

Dosant Oct 4, 2021

Choose a reason for hiding this comment

mistic left a comment

Choose a reason for hiding this comment

spalger left a comment

Choose a reason for hiding this comment

mistic left a comment

Choose a reason for hiding this comment

mattkime commented Oct 4, 2021

mattkime commented Oct 4, 2021 • edited Loading

tylersmalley commented Oct 4, 2021 • edited Loading

kibanamachine commented Oct 4, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Any counts in public APIs

Public APIs missing exports

Page load bundle

API count

References to deprecated APIs

History

spalger commented Oct 4, 2021

kibanamachine commented Oct 4, 2021

💔 Backport failed

mattkime commented Sep 30, 2021 •

edited

Loading

mattkime commented Oct 4, 2021 •

edited

Loading

tylersmalley commented Oct 4, 2021 •

edited

Loading