Skip to content

Conversation

@mshustov
Copy link
Contributor

Summary

Platform inits @elastic/apm-rum only if apm agent is configured. We can load the library lazily only when necessary.
Reduces the core.entry.js file size from 790Kb to 611Kb.

@mshustov mshustov added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 29, 2020
@mshustov mshustov requested a review from a team as a code owner September 29, 2020 12:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

init(apmConfig);
let i18nError: Error | undefined;
try {
await i18n.load(injectedMetadata.i18n.translationsUrl);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can run i18n and apm-agent loading in parallel, but I don't think an error during '@elastic/apm-rum' loading should lead to a global error as i18n does. btw should we mute an error for '@elastic/apm-rum' failed loading?

Copy link
Contributor

Choose a reason for hiding this comment

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

Parallel makes sense 👍 Could we also call coreSystem.setup in parallel? I suspect there may be an implicit dependency that the i18n module is initialized before setup, so maybe not.

Agreed, any errors from APM configuration should not block starting up Kibana. I think logging a warning would be nice for debugging purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parallel makes sense

@joshdover added. PTAL.

Could we also call coreSystem.setup in parallel

We shouldn't as we may want to instrument setup with apm agent.

@mshustov mshustov changed the title load apm-agent lazily load apm-rum agent lazily Sep 29, 2020
Comment on lines +20 to +26
/**
* This is the entry point used to boot the frontend when serving a application
* that lives in the Kibana Platform.
*
* Any changes to this file should be kept in sync with
* src/legacy/ui/ui_bundles/app_entry_template.js
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this comment can be deleted

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id value diff baseline
core 433 +1 432

async chunks size

id value diff baseline
core 290.0KB +134.6KB 155.4KB

distributable file count

id value diff baseline
default 45784 +3 45781
oss 26537 +3 26534

page load bundle size

id value diff baseline
core 659.4KB -130.5KB 789.9KB

History

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

@mshustov mshustov merged commit 5fd785a into elastic:master Sep 30, 2020
@mshustov mshustov deleted the load-apm-agent-lazily branch September 30, 2020 15:08
mshustov added a commit to mshustov/kibana that referenced this pull request Sep 30, 2020
* load apm-agent-lazily

* update docs

* remove outdated comment

* add test
mshustov added a commit that referenced this pull request Sep 30, 2020
* load apm-agent-lazily

* update docs

* remove outdated comment

* add test
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 30, 2020
* master: (97 commits)
  [Actions] Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier (elastic#78746)
  [Discover] Fix functional time picker test permissions (elastic#78564)
  [ML] Fixing module datafeed overrides (elastic#78925)
  Adds some missing licenses to the CSV export (elastic#78719)
  [dev/cli] ensure plugins/ and all watch source dirs exist (elastic#78973)
  [Lens] Stop using scripted metric to collect telemetry (elastic#78687)
  [Lens] fix wrong message in fields accordion (elastic#78924)
  [Enterprise Search][App Search] Credentials Logic updates (elastic#78644)
  [Monitoring] Disk usage alerting (elastic#75419)
  [SECURITY_SOLUTION] Trusted apps list expand/collapse details (elastic#78601)
  Update content on interstitial page (elastic#78881)
  chore(NA): include hjson as a prod dependency (elastic#78941)
  Fix empty meta fields input in Advanced Settings  (elastic#78576)
  [Lens] Maintain order of operations in dimension panel (elastic#78864)
  Fix plugin doc title (elastic#78880)
  load apm-rum agent lazily (elastic#78760)
  [ML] Skip full ML access permission test
  Optimize charts plugin (elastic#78922)
  ui_actions service initial docs (elastic#78902)
  skip failing suite (elastic#78942)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants