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

Log deprecation warnings for plugins which will no longer be disable-able in 8.0 #111890

Closed

Conversation

lukeelmers
Copy link
Member

@lukeelmers lukeelmers commented Sep 10, 2021

NOTE: This PR was replaced by #112602

Closes #110614

Notes if you are pinged for a CODEOWNERS review

This PR moves forward with the plan outlined in #89584 to remove the ability for plugins to be disable-able by default in 8.0.

If you are tagged for a review here, it is for one of the following reasons:

  • Your plugin currently has an enabled property in your config schema, so a deprecation has been added for it.
  • Your plugin currently implicitly has an enabled property but does not have a config schema, however you had indicated you need to preserve the ability to disable it (e.g. vis_type_* plugins). In this case an explicit config has been added for you.

The good news is that most likely you'll only need to review one or two lines of code 🙂

To test the deprecations:

  1. Start Kibana with a default/empty logging config, and <my-plugin>.enabled: true
  2. You should see the deprecation logged to the console
  3. Deprecation should also be logged if <my-plugin>.enabled: false

I also tried to find any documentation that referred to a plugin with an .enabled property. If the docs for your app have been changed, please give them a look. (Preview: https://kibana_111890.docs-preview.app.elstc.co/diff)

This change will be backported to 7.16. In a follow-up PR, we will implement the actual 8.0 breaking change by removing the deprecated config.

Notes for Core Team

The approach here does the following:

  1. Adds a deprecation from the config service for any plugin with an .enabled config that's implicitly added (i.e., plugins which currently have no config schema)
  2. Adds a config schema to a few vis_type_* plugins which currently don't have one but will need to keep an .enabled config in 8.0
  3. Manually add a deprecation warning to plugins which already have an .enabled config that's explicitly added in their config schema
    • To make this easier I've added a new deprecate and deprecateFromRoot to the ConfigDeprecationFactory
  4. Updates docs for any references to deprecated .enabled properties.

I went with this approach because:

  • It's simple and explicit
  • We will already need to manually change every existing config schema with an .enabled property in 8.0, so this is just a precursor to that PR
    • Sure, this means 2 PRs where we ping a bunch of codeowners instead of just one, but it also gives these teams a heads up
  • It doesn't require any special treatment for 3rd party plugins:
    • 3rd party plugins which already have a config schema & an .enabled property are unaffected
    • 1st party and 3rd party plugins which currently have no config schema are treated in the same way (deprecation is logged)
    • If any 1st or 3rd party plugin wants to support an .enabled property moving forward, they just add it to their config schema

However, I'm still open to feedback on alternatives.

Release note

Deprecates the ability for most plugins to be disabled using the {plugin_name}.enabled config option. In 8.0, most Kibana plugins can no longer be disabled using this option. Plugin developers can still opt-in to this feature by explicitly adding an .enabled property to their config schema.

If you are currently using one of these options in your Kibana config, please remove it before upgrading to 8.0.

@lukeelmers lukeelmers added release_note:deprecation Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 v7.16.0 labels Sep 10, 2021
@lukeelmers lukeelmers self-assigned this Sep 10, 2021
@lukeelmers
Copy link
Member Author

@elasticmachine merge upstream

@lukeelmers lukeelmers marked this pull request as ready for review September 16, 2021 03:30
@lukeelmers lukeelmers requested a review from a team as a code owner September 16, 2021 03:30
@lukeelmers lukeelmers requested a review from a team September 16, 2021 03:30
@lukeelmers lukeelmers requested review from a team as code owners September 16, 2021 03:30
@lukeelmers
Copy link
Member Author

@elasticmachine merge upstream

@lukeelmers lukeelmers requested a review from a team September 20, 2021 18:15
@lukeelmers lukeelmers requested review from a team as code owners September 20, 2021 18:15
@lukeelmers lukeelmers requested a review from a team September 20, 2021 18:15
@lukeelmers lukeelmers requested review from a team as code owners September 20, 2021 18:15
@lukeelmers lukeelmers requested review from a team, academo and paul-tavares September 20, 2021 18:15
@lukeelmers
Copy link
Member Author

Sorry folks, Github seems to be confused now that master has been merged into this PR and is attempting to include all of the changes which have been made in the past few days. Apologies for the noise 🙁

I'll go ahead and close this PR and open a new one.

@lukeelmers lukeelmers closed this Sep 20, 2021
@lukeelmers lukeelmers deleted the feat/deprecate-disableable-plugins branch September 20, 2021 18:20
@lukeelmers lukeelmers restored the feat/deprecate-disableable-plugins branch September 20, 2021 18:21
@lukeelmers
Copy link
Member Author

Replacement PR: #112602

@lukeelmers lukeelmers deleted the feat/deprecate-disableable-plugins branch September 20, 2021 18:57
@kibanamachine
Copy link
Contributor

kibanamachine commented Sep 20, 2021

💔 Build Failed

Failed CI Steps


Test Failures

Kibana Pipeline / general / Jest Integration Tests.packages/kbn-plugin-helpers/src/integration_tests.builds a generated plugin into a viable archive

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 13 times on tracked branches: https://github.com/elastic/kibana/issues/89079


Stack Trace

Error: expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `builds a generated plugin into a viable archive 2`

- Snapshot  - 0
+ Received  + 5

  " info deleting the build and target directories
   info running @kbn/optimizer
   │ info initialized, 0 bundles cached
   │ info starting worker [1 bundle]
+  │ warn worker stderr Browserslist: caniuse-lite is outdated. Please run:
+  │ warn worker stderr npx browserslist@latest --update-db
+  │ warn worker stderr 
+  │ warn worker stderr Why you should do it regularly:
+  │ warn worker stderr https://github.com/browserslist/browserslist#browsers-data-updating
   │ succ 1 bundles compiled successfully after <time>
   info copying assets from `public/assets` to build
   info copying server source into the build and converting with babel
   info running yarn to install dependencies
   info compressing plugin into [fooTestPlugin-7.5.0.zip]"
    at Object.<anonymous> (/dev/shm/workspace/parallel/3/kibana/packages/kbn-plugin-helpers/src/integration_tests/build.test.ts:63:25)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at _callCircusTest (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-circus/build/run.js:212:5)
    at _runTest (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-circus/build/run.js:63:9)
    at run (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/dev/shm/workspace/parallel/3/kibana/node_modules/jest-runner/build/runTest.js:472:34)

Metrics [docs]

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
core 1014 1020 +6
fleet 1077 1075 -2
total +4

Async chunks

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

id before after diff
apm 2.7MB 2.7MB +1.0B
discover 378.6KB 378.7KB +112.0B
fleet 590.1KB 590.0KB -131.0B
lens 1021.0KB 1021.0KB -8.0B
securitySolution 4.2MB 4.2MB -191.0B
timelines 233.4KB 233.3KB -121.0B
visTypeTimeseries 636.4KB 636.5KB +112.0B
visualizations 72.0KB 71.9KB -91.0B
total -317.0B

Page load bundle

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

id before after diff
fleet 130.2KB 130.1KB -177.0B
upgradeAssistant 18.8KB 18.8KB -29.0B
total -206.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
ingest-package-policies 35 33 -2
Unknown metric groups

API count

id before after diff
core 2283 2291 +8
fleet 1178 1176 -2
total +6

History

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

cc @lukeelmers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Configuration Settings in kibana.yml Feature:Plugins release_note:deprecation Team:APM All issues that need APM UI Team support Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Fleet Team label for Observability Data Collection Fleet team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log deprecation warnings when plugins are disabled but will no longer be disable-able in 8.0