Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kbn/optimizer] implement "requiredBundles" property of KP plugins #70911

Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jul 7, 2020

Plugin teams who were pinged for review

This change triggers a ton of review because it changes a lot of kibana.json files. These changes are validated at build time by the @kbn/optimizer and I've talked to @kobelb and we've agreed that we're going to merge this even if we don't get review from all the plugins. I encourage you to see and understand the change described here as it will impact your plugin during development, but please don't feel like you need to schedule review unless you're on the @elastic/kibana-platform or @elastic/kibana-operations team.


Closes #69458

This PR requires that plugin define the bundles which they import from, so that when the bundle for that plugin is loaded we can be certain that the imported-from bundles are loaded even when those plugins are disabled.

When a plugin tries to import from another bundle but the bundle is not mentioned in the kibana.json file the following error will be logged:

import [../../../../../src/plugins/kibana_react/public] references a public export of the [kibanaReact] bundle, but that bundle is not in the "requiredPlugins" or "requiredBundles" list in the plugin manifest [{path to}/kibana.json]

When users fix this problem while running @kbn/optimizer in --watch mode (or just using yarn start), webpack will notice the change to the kibana.json and rebuild, adapting to changes in kibana.json at runtime without needing to restart the optimizer.

Similarly, when requiredBundles has an extra plugin listed another error will be logged and again the error can be cleared by fixing the kibana.json file.

2020-07-06 19 01 07

Finally, in order to make sure that the kibana.json file is tracked by webpack when it watches files for changes learned about compilation.fileDependencies. This is an array of every file path that webpack interacted with while creating the build, including files that don't make it into the final bundle or impact the output of the bundle. This list is useful for measuring how many files each bundle forces webpack to consult, which was originally the intention behind the moduleCount metric. The size of this list, plus a static value of 100 for each .scss module created, are used to create a new workUnits value that is tracked by the optimizer. Using this metric to allocate bundles to workers rather than moduleCount makes @kbn/optimizer marginally more efficient on systems with lots of CPU cores.

We still track the moduleCount for reporting purposes as it's still a very useful metric for identifying surprisingly heavy imports and other unexpected side-effects of changing imports in public bundles.

@spalger spalger added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 7, 2020
@spalger spalger force-pushed the implement/plugins-declare-required-bundles branch from 5ed29a7 to f956f08 Compare July 8, 2020 18:12
@paul-tavares
Copy link
Contributor

FYI - this PR for Security (Endpoint) is dependent on this change - #71198

@spalger spalger marked this pull request as ready for review July 9, 2020 15:48
@spalger spalger requested review from a team as code owners July 9, 2020 15:48
@spalger spalger requested a review from a team July 9, 2020 15:48
@spalger spalger requested review from a team as code owners July 9, 2020 15:48
@spalger spalger requested a review from a team July 9, 2020 15:48
@spalger spalger requested review from a team as code owners July 9, 2020 15:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@spalger spalger requested review from a team as code owners July 9, 2020 15:48
Co-authored-by: Josh Dover <me@joshdover.com>
@spalger
Copy link
Contributor Author

spalger commented Jul 9, 2020

@elasticmachine merge upstream

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APM change looks good.

@spalger
Copy link
Contributor Author

spalger commented Jul 9, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@spalger spalger merged commit fa93a81 into elastic:master Jul 10, 2020
@spalger spalger deleted the implement/plugins-declare-required-bundles branch July 10, 2020 01:43
spalger pushed a commit to spalger/kibana that referenced this pull request Jul 10, 2020
…lastic#70911)

Co-authored-by: Josh Dover <me@joshdover.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
#	src/plugins/embeddable/kibana.json
#	src/plugins/telemetry/kibana.json
#	src/plugins/ui_actions/kibana.json
#	x-pack/plugins/apm/kibana.json
#	x-pack/plugins/upgrade_assistant/kibana.json
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 10, 2020
…11y-overlay

* 'master' of github.com:elastic/kibana: (33 commits)
  address index templates feedback (elastic#71353)
  Upgrade EUI to v26.3.1 (elastic#70243)
  [build] Creates Linux aarch64 archive (elastic#69165)
  [SIEM][Detection Engine] Fixes skipped tests (elastic#71347)
  [SIEM][Detection Engine][Lists] Adds read_privileges route for lists and list items
  [kbn/optimizer] implement "requiredBundles" property of KP plugins (elastic#70911)
  [Security Solution][Exceptions] - Exceptions modal pt 2 (elastic#70886)
  [ML] DF Analytics:  stop status polling when job stopped (elastic#71159)
  [SIEM][CASE] IBM Resilient Connector (elastic#66385)
  jenkins_xpack_saved_objects_field_metrics.sh expects to be run from the KIBANA_DIR in CI
  Deduplication of entries and items before sending to endpoint (elastic#71297)
  [services/remote/webdriver] fix eslint error (elastic#71346)
  send slack notifications on visual baseline failures
  fix visual regression job (elastic#70999)
  [Ingest Manager] Add schema to usageCollector. (elastic#71219)
  [ftr] use typed chromeOptions object, adding TEST_BROWSER_BINARY_PATH (elastic#71279)
  [Ingest Manager] Fix limited packages incorrect response (elastic#71292)
  Support multiple features declaring same properties (elastic#71106)
  [Security_Solution][Resolver]Add beta badge to Resolver panel (elastic#71183)
  [DOCS] Clarify trial subscription levels (elastic#70636)
  ...
spalger pushed a commit that referenced this pull request Jul 10, 2020
…ins (#70911) (#71355)

Co-authored-by: Josh Dover <me@joshdover.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
#	src/plugins/embeddable/kibana.json
#	src/plugins/telemetry/kibana.json
#	src/plugins/ui_actions/kibana.json
#	x-pack/plugins/apm/kibana.json
#	x-pack/plugins/upgrade_assistant/kibana.json
brianseeders added a commit that referenced this pull request Jul 11, 2020
brianseeders added a commit that referenced this pull request Jul 11, 2020
spalger added a commit that referenced this pull request Jul 11, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 13, 2020
* master: (78 commits)
  Bump lodash package version (elastic#71392)
  refactor: 💡 use allow-list in AppArch codebase (elastic#71400)
  improve bugfix 7198 test stability (elastic#71250)
  [Security Solution][Ingest Manager][Endpoint] Optional ingest manager (elastic#71198)
  [Metrics UI] Round metric threshold time buckets to nearest unit (elastic#71172)
  [Security Solution][Endpoint] Policy creation callback fixes + Improved error handling in user manifest loop (elastic#71269)
  [Security Solution] Allow to configure Event Renderers settings (elastic#69693)
  Fix a11y keyboard overlay (elastic#71214)
  [APM] UI text updates (elastic#71333)
  [Logs UI] Limit `extendDatemath` to valid ranges (elastic#71113)
  [SIEM] fix tooltip of notes (elastic#71342)
  address index templates feedback (elastic#71353)
  Upgrade EUI to v26.3.1 (elastic#70243)
  [build] Creates Linux aarch64 archive (elastic#69165)
  [SIEM][Detection Engine] Fixes skipped tests (elastic#71347)
  [SIEM][Detection Engine][Lists] Adds read_privileges route for lists and list items
  [kbn/optimizer] implement "requiredBundles" property of KP plugins (elastic#70911)
  [Security Solution][Exceptions] - Exceptions modal pt 2 (elastic#70886)
  [ML] DF Analytics:  stop status polling when job stopped (elastic#71159)
  [SIEM][CASE] IBM Resilient Connector (elastic#66385)
  ...
@sebelga
Copy link
Contributor

sebelga commented Jul 16, 2020

@spalger do you mind explaining the difference between requiredPlugins and requiredBundles? I thought that "esUiShared" was also a plugin like any other. Thanks!

@spalger
Copy link
Contributor Author

spalger commented Jul 16, 2020

@sebelga it is, the difference is that requiredPlugins defines the dependency tree for plugin contracts. If you're only using static imports for a plugin you don't need it's plugin contract, and therefore can execute even if the dependent plugin is disabled as long as their static code is still loaded and available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embedding Embedding content via iFrame Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team Team:Operations Team label for Operations Team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[kbn/optimizer] Only allow refs to bundles described in kibana.json