Migrate most plugins to synchronous lifecycle by pgayvallet · Pull Request #89562 · elastic/kibana

pgayvallet · 2021-01-28T14:13:09Z

Summary

Part of #53268, was unblocked by #88981
Fix #71925
Fix #74395

This PR migrates most of the existing asynchronous plugins to synchronous lifecycle.

Change the client and server side Plugin signature to no longer accept returning promises from setup and start
Introduce the (deprecated) AsyncPlugin interface (same interface as the old Plugin)
Migrate most of the async plugins to synchronous lifecycles
Explicitly use AsyncPlugin instead of Plugin for plugins I couldn't migrate
Logs warnings in development mode for asynchronous plugins
Update the legacy plugin migration guide examples
Add missing explicit Plugin interface on some plugins + some overall cleanup

Remaining async plugins

There are only 3 2 plugins this PR does not migrate:

server-side
- fleet
client-side
- ~~monitoring~~ addressed by [Monitoring] Fixed alert imports #89935
- vis_type_table

I will create a distinct issue to discuss about those with the owners once this PR is merged.

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

…-async-config-plugins

pgayvallet

Implementation review

pgayvallet · 2021-01-29T11:54:47Z

src/core/public/plugins/plugin.ts

+    if (isPromise(startContract)) {
+      return startContract.then((resolvedContract) => {
+        this.startDependencies$.next([startContext, plugins, resolvedContract]);
+        return resolvedContract;
+      });
+    } else {
+      this.startDependencies$.next([startContext, plugins, startContract]);
+      return startContract;
+    }


I need to dissociate promises from other return values in the plugin system, so the plugin wrapper can no longer be async. This is why I need to chain the promise to resolve the startDependencies

pgayvallet · 2021-01-29T11:57:34Z

src/core/public/plugins/plugins_service.ts

+        if (this.coreContext.env.mode.dev) {
+          // eslint-disable-next-line no-console
+          console.log(
+            `Plugin ${pluginName} is using asynchronous setup lifecycle. Asynchronous plugins support will be removed in a later version.`
+          );
+        }


Until we implement a client-side logger, we don't have any other options than just console.log the warning for client-side plugins

pgayvallet · 2021-01-29T11:59:53Z

src/core/server/plugins/plugin.ts

+  private instance?:
+    | Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>
+    | AsyncPlugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;


I started by adding a EitherPlugin = Plugin | AsyncPlugin type, but as this is only a temporary measure, I think that having the explicit composite type in the few places we are using plugins is better.

patrykkopycinski

Osquery LGTM

sebelga

ES UI changed LGTM. Thanks!

weltenwort

Seems to work for the infra plugin, thank you 👍

Out of curiosity, do you remember if the config semantics changes since the new platform was introduced? We used the async access to config because the config wasn't loaded early enough. A simple test shows it is now. 🤷

pgayvallet · 2021-02-02T15:53:02Z

@weltenwort

Out of curiosity, do you remember if the config semantics changes since the new platform was introduced? We used the async access to config because the config wasn't loaded early enough

The synchronous config access API was added last week by #88981, but apart from that I don't think we changed the behavior of the context.config.create() API over the last year.

pgayvallet · 2021-02-02T15:59:03Z

@elastic/endpoint-app-team @elastic/kibana-presentation @elastic/stack-monitoring-ui still waiting for your reviews

…-async-config-plugins

chrisronline

LGTM for Stack Monitoring!

…-async-config-plugins

kibanamachine · 2021-02-08T09:14:23Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`data`	236.1KB	236.4KB	+265.0B
`fleet`	759.7KB	760.0KB	+265.0B
`graph`	1.2MB	1.2MB	+265.0B
total			+795.0B

Page load bundle

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

id	before	after	diff
`cloud`	5.2KB	5.2KB	-6.0B
`core`	466.6KB	467.7KB	+1.1KB
`fleet`	365.9KB	365.9KB	-6.0B
`inspector`	44.1KB	44.1KB	-6.0B
`licensing`	14.8KB	14.8KB	-6.0B
`monitoring`	49.5KB	49.5KB	-6.0B
`presentationUtil`	25.9KB	25.9KB	-6.0B
`regionMap`	19.4KB	19.4KB	-6.0B
`tileMap`	18.0KB	18.0KB	-6.0B
`uptime`	19.3KB	19.3KB	-6.0B
`urlDrilldown`	42.1KB	42.4KB	+265.0B
`visTypeTimeseries`	126.7KB	126.7KB	-6.0B
`visTypeVega`	57.4KB	57.4KB	-6.0B
`visTypeVislib`	36.0KB	36.0KB	-6.0B
`visTypeXy`	46.5KB	46.5KB	-6.0B
`visualize`	28.8KB	28.8KB	-6.0B
total			+1.3KB

History

💚 Build #103698 succeeded 49127b3
💚 Build #103317 succeeded 6a4044f
💛 Build #102483 was flaky b6074c7
💚 Build #102350 succeeded 320dd5f
💔 Build #102232 failed 59740e4

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

* first pass * migrate more plugins * migrate yet more plugins * more oss plugins * fix test file * change Plugin signature on the client-side too * fix test types * migrate OSS client-side plugins * migrate OSS client-side test plugins * migrate xpack client-side plugins * revert fix attempt on fleet plugin * fix presentation start signature * fix yet another signature * add warnings for server-side async plugins in dev mode * remove unused import * fix isPromise * Add client-side deprecations * update migration examples * update generated doc * fix xpack unit tests * nit * (will be reverted) explicitly await for license to be ready in the auth hook * Revert "(will be reverted) explicitly await for license to be ready in the auth hook" This reverts commit fdf73fe * restore await on on promise contracts * Revert "(will be reverted) explicitly await for license to be ready in the auth hook" This reverts commit fdf73fe * Revert "restore await on on promise contracts" This reverts commit c5f2fe5 * add delay before starting tests in FTR * update deprecation ts doc * add explicit contract for monitoring setup * migrate monitoring plugin to sync * change plugin timeout to 10sec * use delay instead of silence # Conflicts: # x-pack/plugins/xpack_legacy/server/plugin.ts

* Migrate most plugins to synchronous lifecycle (#89562) * first pass * migrate more plugins * migrate yet more plugins * more oss plugins * fix test file * change Plugin signature on the client-side too * fix test types * migrate OSS client-side plugins * migrate OSS client-side test plugins * migrate xpack client-side plugins * revert fix attempt on fleet plugin * fix presentation start signature * fix yet another signature * add warnings for server-side async plugins in dev mode * remove unused import * fix isPromise * Add client-side deprecations * update migration examples * update generated doc * fix xpack unit tests * nit * (will be reverted) explicitly await for license to be ready in the auth hook * Revert "(will be reverted) explicitly await for license to be ready in the auth hook" This reverts commit fdf73fe * restore await on on promise contracts * Revert "(will be reverted) explicitly await for license to be ready in the auth hook" This reverts commit fdf73fe * Revert "restore await on on promise contracts" This reverts commit c5f2fe5 * add delay before starting tests in FTR * update deprecation ts doc * add explicit contract for monitoring setup * migrate monitoring plugin to sync * change plugin timeout to 10sec * use delay instead of silence # Conflicts: # x-pack/plugins/xpack_legacy/server/plugin.ts * fix mock

* master: (55 commits) [APM-UI][E2E] use githubNotify step (elastic#90514) [APM] Export ProcessorEvent type (elastic#90540) [Lens] Retain column config (elastic#90048) [Data Table] Add unit tests (elastic#90173) Migrate most plugins to synchronous lifecycle (elastic#89562) skip flaky suite (elastic#90555) skip flaky suite (elastic#64473) [actions] improve email action doc (elastic#90020) [Fleet] Support Fleet server system indices (elastic#89372) skip flaky suite (elastic#90552) Bump immer dependencies (elastic#90267) Unrevert "Migrations v2: don't auto-create indices + FTR/esArchiver support (elastic#85778)" (elastic#89992) [Search Sessions] Use sync config (elastic#90138) chore(NA): add safe guard to remove bazelisk from yarn global at bootstrap (elastic#90538) [test] Await retry.waitFor (elastic#90456) chore(NA): integrate build buddy with our bazel setup and remote cache for ci (elastic#90116) Skip failing suite (elastic#90526) [Fleet] Fix incorrect conversion of string to numeric values in agent YAML (elastic#90371) [Docs] Update reporting troubleshooting for arm rhel/centos (elastic#90385) chore(NA): build bazel projects all at once in the distributable build process (elastic#90328) ...

pgayvallet added 6 commits January 28, 2021 15:11

first pass

37d965a

migrate more plugins

8bd7c21

migrate yet more plugins

235a656

more oss plugins

19f8ee0

fix test file

d2325e2

change Plugin signature on the client-side too

1f4beba

pgayvallet added chore Feature:New Platform labels Jan 28, 2021

pgayvallet added 6 commits January 29, 2021 08:09

fix test types

7f0816a

migrate OSS client-side plugins

36e4e4a

migrate OSS client-side test plugins

f7efac0

migrate xpack client-side plugins

9162085

revert fix attempt on fleet plugin

08659e5

Merge remote-tracking branch 'upstream/master' into kbn-53268-migrate…

86e78b0

…-async-config-plugins

pgayvallet mentioned this pull request Jan 29, 2021

[Core] Deprecate async lifecycles #53268

Closed

6 tasks

pgayvallet added 8 commits January 29, 2021 09:41

fix presentation start signature

d9d830b

fix yet another signature

147fe61

add warnings for server-side async plugins in dev mode

55d0a68

remove unused import

9d775f2

fix isPromise

63651af

Add client-side deprecations

d64d04f

update migration examples

daf9db0

update generated doc

1cbb228

pgayvallet added release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0 labels Jan 29, 2021

pgayvallet changed the title ~~Migrate plugins to synchronous lifecycle~~ Migrate most plugins to synchronous lifecycle Jan 29, 2021

pgayvallet commented Jan 29, 2021

View reviewed changes

pgayvallet added 2 commits January 29, 2021 13:29

fix xpack unit tests

fb4cb34

nit

d3a24d0

patrykkopycinski approved these changes Feb 1, 2021

View reviewed changes

justinkambic approved these changes Feb 1, 2021

View reviewed changes

sebelga approved these changes Feb 1, 2021

View reviewed changes

smith mentioned this pull request Feb 1, 2021

[APM] remove config$ observable #89931

Closed

smith approved these changes Feb 1, 2021

View reviewed changes

spalger approved these changes Feb 1, 2021

View reviewed changes

weltenwort approved these changes Feb 2, 2021

View reviewed changes

pgayvallet added 4 commits February 3, 2021 07:58

Merge remote-tracking branch 'upstream/master' into kbn-53268-migrate…

d686682

…-async-config-plugins

migrate monitoring plugin to sync

3267081

Merge remote-tracking branch 'upstream/master' into kbn-53268-migrate…

09850d8

…-async-config-plugins

change plugin timeout to 10sec

6a4044f

poffdeluxe approved these changes Feb 3, 2021

View reviewed changes

chrisronline approved these changes Feb 3, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into kbn-53268-migrate…

49127b3

…-async-config-plugins

YulNaumenko mentioned this pull request Feb 4, 2021

Alerting team plugins to use synchronous setup and start functions #74395

Closed

pgayvallet added 2 commits February 8, 2021 07:57

Merge remote-tracking branch 'upstream/master' into kbn-53268-migrate…

da51a5a

…-async-config-plugins

use delay instead of silence

7a31a69

pgayvallet merged commit 3b3327d into elastic:master Feb 8, 2021

alexwizp mentioned this pull request Jul 2, 2021

[Canvas] Expression reveal image. Async libs and images loading. #103399

Merged

7 tasks

poffdeluxe mentioned this pull request Jul 8, 2021

[Canvas] Remove async from server plugin setup #74374

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate most plugins to synchronous lifecycle#89562

Migrate most plugins to synchronous lifecycle#89562
pgayvallet merged 38 commits intoelastic:masterfrom
pgayvallet:kbn-53268-migrate-async-config-plugins

pgayvallet commented Jan 28, 2021 •

edited by mikecote

Loading

Uh oh!

pgayvallet left a comment

Uh oh!

pgayvallet Jan 29, 2021

Uh oh!

pgayvallet Jan 29, 2021

Uh oh!

pgayvallet Jan 29, 2021

Uh oh!

patrykkopycinski left a comment

Uh oh!

sebelga left a comment

Uh oh!

weltenwort left a comment

Uh oh!

pgayvallet commented Feb 2, 2021

Uh oh!

pgayvallet commented Feb 2, 2021

Uh oh!

chrisronline left a comment

Uh oh!

kibanamachine commented Feb 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Conversation

pgayvallet commented Jan 28, 2021 • edited by mikecote Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Remaining async plugins

Checklist

Uh oh!

pgayvallet left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

patrykkopycinski left a comment

Choose a reason for hiding this comment

Uh oh!

sebelga left a comment

Choose a reason for hiding this comment

Uh oh!

weltenwort left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet commented Feb 2, 2021

Uh oh!

pgayvallet commented Feb 2, 2021

Uh oh!

chrisronline left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 8, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

pgayvallet commented Jan 28, 2021 •

edited by mikecote

Loading