Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Migrate entire plugin to new platform #328

Merged
merged 43 commits into from
Nov 20, 2020
Merged

Migrate entire plugin to new platform #328

merged 43 commits into from
Nov 20, 2020

Conversation

ohltyler
Copy link
Contributor

@ohltyler ohltyler commented Nov 18, 2020

Issue #, if available:

Description of changes:

This PR migrates the entire plugin to the new Kibana platform, which is required for Kibana 7.10. A follow up PR will be made to upgrade the plugin to 7.10. Overview of the changes can be found here.

The main changes include:

  • Defining separate lifecycles for browser-side plugin and server-side plugin by having separate index and plugin files for each
  • Changing the overall entry point of the plugin to be in the new kibana.json manifest file
  • Migrating the browser client from Angular's IHttpService to Kibana's core HttpSetup client, and changing all request/response parsing logic accordingly
  • Changing the server's communication with the backend by registering an IClusterClient with all of the services, and changing the different route functions to be grouped into different service classes
  • Change in accessing core services: removing all legacy import ui/* statements, and accessing core services (chrome breadcrumbs, toast notifications, etc.) by consuming them via React context
  • Complete removal of all dependencies on Angular
  • Updating all unit tests and adding a core services mock to be used for components that are consuming core services

Notes:

  • All unit tests pass
  • All integration tests pass
  • Sanity tested most plugin functionality
  • Sanity tested most server/node APIs and most backend API calls
  • Checked dark mode
  • Cannot run the plugin via yarn start in the plugin repo anymore - need to run in base Kibana dir

Known issues:

  • yarn build issues - the built artifact won't work with the binary Kibana. This is fixed separately, will be included in a follow up PR

Task backlog:

  • Sanity test w/ alerting when it's finished
  • Sanity test w/ FGAC enabled
  • Update the README with any necessary yarn command changes

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ohltyler ohltyler added the version upgrade Upgrading Kibana compatibility or dependencies label Nov 18, 2020
Copy link
Contributor

@yizheliu-amazon yizheliu-amazon left a comment

Choose a reason for hiding this comment

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

thanks for the change. Please make sure commits are squashed before merging.

kibana.json Show resolved Hide resolved
kibana.json Outdated
"kibanaVersion": "7.9.1",
"configPath": ["opendistro_anomaly_detection_kibana"],
"requiredPlugins": ["navigation"],
"optionalPlugins": ["opendistro-alerting", "opendistro_alerting"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add two alerting plugins?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the naming conventions for the ES and Kibana alerting plugins. With the new naming conventions for the Kibana plugin this will need to be revisited, I'll make a note of it. Also, adding optional plugins doesn't really have much functionality at all, and we could remove altogether. See here

@ohltyler ohltyler merged commit da5ead3 into master Nov 20, 2020
@ohltyler ohltyler deleted the new-platform branch February 4, 2021 19:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
version upgrade Upgrading Kibana compatibility or dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants