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

[Alerting] adds lazy loading of alerting UI components #65060

Merged
merged 18 commits into from
May 7, 2020

Conversation

gmmorris
Copy link
Contributor

@gmmorris gmmorris commented May 4, 2020

Summary

This PR adds code splitting into the Alerting Trigger UI by lazy loading the React Components needed for this plugin.

This include two key changes:

  1. The plugin lazy loads its routes so that they don't get loaded at setup time.
  2. This changes the way in which plugins can register their ActionType Components so they have to specify a lazy loaded component rather than a regular React component - this allows us to avoid loading all these components at setup time and defer this to when the Flyout/Triggers UI is loaded.

addresses #64315

AlertTypes & Components imported by other plugins will be addressed in a followup PR which will close the issue

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@gmmorris gmmorris added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels May 4, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@gmmorris
Copy link
Contributor Author

gmmorris commented May 5, 2020

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

merge conflict between base and head

* master: (133 commits)
  Cleanup Typescript index pattern field editor / Expression functions for bucket agg (elastic#65254)
  Removes legacy infra plugin and moves saved objects registration to NP (elastic#64848)
  Added support for docLinks plugin in Connectors forms and missing save capabilities for modal dialog (elastic#64986)
  [SIEM] Removes prebuilt rules number dependency (elastic#65128)
  [Maps] add categorical palettes with 20 and 30 categories (elastic#64701)
  [CI] Slack alerts - Elasticsearch snapshot failures (elastic#64724)
  [Uptime] Console errors in case index missing (elastic#65115)
  [SIEM][CASE] Dynamic fields mapping based on connector (elastic#64412)
  [test/functional] Tsfy page objects (elastic#64887)
  [Maps] [Telemetry] Track geo_point and geo_shape index patterns separately (elastic#65195)
  [Maps] Add global fit to data (elastic#64702)
  Visualize: Reload on ui state change and fix ui state for tsvb (elastic#63699)
  [SIEM] [Cases] External service selection per case (elastic#64775)
  [Uptime] Set ML anomaly look-back to 2w (from 24h) / Add spinner (elastic#65055)
  [Metrics UI] Remove APM Hard Dependency (elastic#64952)
  [Ingest] Datastream list: add icons and dashboard links (elastic#65048)
  disable plugins. they could access ES via SO repository (elastic#65242)
  Feature fleet enrollment instructions (elastic#65176)
  [SIEM] Adds 'Configure connector' Cypress test (elastic#64807)
  [TSVB] Fix std deviation band mode (elastic#64413)
  ...
* master: (72 commits)
  add tsvb tests to Firefox suite (elastic#65425)
  Fix flaky ServerMetricsCollector integration test (elastic#65420)
  [APM] Custom links section inside the Actions menu is showing outside of the menu (elastic#65428)
  [ML] Adds docs_per_second to transform edit form. (elastic#65365)
  update apm index pattern (elastic#65424)
  add direct build command (elastic#65431)
  [ML] Adding daily_model_snapshot_retention_after_days to types and schemas (elastic#65417)
  [chore] Improve request cancelation handling in vis embeddable (elastic#65057)
  [Alerting] migrates acceptance and functional test fixtures to KP (elastic#64888)
  [ML] Fixes reordering in view by selection when overall cell selected (elastic#65290)
  Additional branding updates (elastic#64712)
  Remove redundant formatting of percentage column (elastic#64948)
  [SIEM][CASE] Configuration pages UI redesign (elastic#65355)
  New nav (elastic#64018)
  [Ingest pipelines] Address copy feedback (elastic#65175)
  bug fixing (elastic#65387)
  skip whole suite blocking snapshots (elastic#65377)
  add related event generation to ancestor nodes (fixes a bug) (elastic#64950)
  [Canvas] move files from legacy/plugins to plugins (elastic#65283)
  [SIEM] template timeline UI (elastic#64439)
  ...
@gmmorris gmmorris marked this pull request as ready for review May 7, 2020 08:36
@gmmorris gmmorris requested review from a team as code owners May 7, 2020 08:36
@gmmorris gmmorris added the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label May 7, 2020
* master: (42 commits)
  fix SavedObjectMigrationMap type (elastic#65569)
  [Uptime] Improve cert flaky test (elastic#65458)
  [Uptime] Fix monitor list result runtime type, ip can be null (elastic#65532)
  [APM] Agent configuration: Bug makes it possible to create invalid configurations (elastic#65508)
  [APM] Remove link from active page in the breadcrumb (elastic#65473)
  [SIEM] Fixes test flakiness (elastic#65510)
  [ESLint] update @kbn/eslint/no-restricted-paths rule to allow imports mocks from folder (elastic#65471)
  Migrate test plugins ⇒ NP (kbn_tp_run_pipeline) (elastic#64780)
  move core provier to NP. allows to run tests on every page (elastic#64929)
  Extended alerting documentation with information about using Kibana keystore and action types for preconfigured connectors (elastic#65201)
  [functional tests] add some missing awaits (elastic#65566)
  Fixed create new connector from alert flyout form throw an error messages in external plugins. (elastic#65539)
  [SIEM] [Cases] External services not getting all comments bug fix (elastic#65307)
  Migrate Coordinate Maps to NP (elastic#64668)
  Updating Canvas location in nav (elastic#65519)
  [SIEM][Lists] Fixes up contracts to work outside of requests
  [Lens] Remove "inside only" option for treemap labels (elastic#65363)
  [Uptime] Add TLS alert functional test (elastic#65303)
  Fix z-index of kbnLoadingIndicator (elastic#65521)
  Fixed indice assertion to loop through expected keys (elastic#64684)
  ...
Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

I looked through the code that touches SIEM. Looks good! I tested locally without any problems.

@mikecote mikecote self-requested a review May 7, 2020 12:22
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

LGTM! Great work, just a few comments about the spinners.

The PR should also have the v7.9.0 label instead of v7.8.0. The release_note:dev_docs also requires a level 1 header in the PR description named "Dev Docs" with a description of the changes developers will have to make (example: #19061). It can also have a release_note:fix for fixing the bundle size.

@gmmorris gmmorris added v7.9.0 release_note:fix and removed v7.8.0 release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels May 7, 2020
* master:
  [ML] Transforms: Fix API error message display for edit flyout. (elastic#65494)
  [SIEM][Detection Engine] Fixes import bug with non existent signals index (elastic#65595)
  [Lens] Use rules of hooks with linting (elastic#65593)
  [ML] Migrate server side Mocha tests to Jest. (elastic#65651)
  Fixes the client to setup SSL with the CA certificates for testing (elastic#65598)
  reduce uptime plugin initial bundle size (elastic#65257)
  [ML] Consolidating shared types and util functions (elastic#65247)
  [Drilldowns] Preserve state when selecting different action factory (elastic#65074)
  Migrate test plugins ⇒ NP (kbn_tp_embeddable_explorer) (elastic#64756)
  Move remaining home assets to the new platform (elastic#65053)
  Change the copy and the id from blacklist to block list for consistency (elastic#65419)
  [ML] Hide selector helper in Anomaly Explorer swimlane (elastic#65522)
  [ML] Fix the limit control on the Anomaly explorer page (elastic#65459)
  [Mappings editor] Add component integration tests (elastic#63853)
  [Logs + Metrics UI] Prevent component errors from breaking the whole UI (elastic#65456)
  [Logs UI] Disable search bar when live stream is on. (elastic#65491)
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@YulNaumenko YulNaumenko self-requested a review May 7, 2020 16:30
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM. Great work!

@gmmorris gmmorris merged commit a79ab6d into elastic:master May 7, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 7, 2020
This PR adds code splitting into the Alerting Trigger UI by lazy loading the React Components needed for this plugin.

This include two key changes:

1. The plugin lazy loads its routes so that they don't get loaded at setup time.
2. This changes the way in which plugins can register their ActionType Components so they have to specify a lazy loaded component rather than a regular React component - this allows us to avoid loading all these components at setup time and defer this to when the Flyout/Triggers UI is loaded.
gmmorris added a commit that referenced this pull request May 7, 2020
This PR adds code splitting into the Alerting Trigger UI by lazy loading the React Components needed for this plugin.

This include two key changes:

1. The plugin lazy loads its routes so that they don't get loaded at setup time.
2. This changes the way in which plugins can register their ActionType Components so they have to specify a lazy loaded component rather than a regular React component - this allows us to avoid loading all these components at setup time and defer this to when the Flyout/Triggers UI is loaded.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants