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

[Logs UI] Replace dependencies in the infra bundle #91503

Merged
merged 4 commits into from
Feb 19, 2021

Conversation

afgomez
Copy link
Contributor

@afgomez afgomez commented Feb 16, 2021

Summary

Part of #89025

Replace some of the dependencies in the infra bundle for more lightweight alternatives.

  • mustache. The library was only used in the createFormatter helper function, and only to substitute a token with a value. This can be achieved with String.prototype.replaceAll().

  • url. Used in the <LegacyApp> and useLinkProps to construct URLs. The URLs generated are already heavily constructed in previous steps in the code, so this module was used only for concatenation.

@afgomez afgomez added Feature:Logs UI Logs UI feature performance release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services technical debt Improvement of the software architecture and operational architecture v8.0.0 labels Feb 16, 2021
@afgomez afgomez force-pushed the 89025-infra-bundle-diet branch from ac12294 to d54de59 Compare February 16, 2021 18:12
@afgomez afgomez marked this pull request as ready for review February 17, 2021 12:52
@afgomez afgomez requested review from a team as code owners February 17, 2021 12:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

LGTM for Ops.

@weltenwort weltenwort self-requested a review February 17, 2021 16:50
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Seems like a nice win. The only limitation is that the metric formatter template is now less powerful, but I couldn't find anything beyond the simple interpolation.

Thank you!

@@ -34,5 +33,5 @@ export const createFormatter = (format: InventoryFormatterType, template: string
}
const fmtFn = FORMATTERS[format];
const value = fmtFn(Number(val));
return Mustache.render(template, { value });
return template.replace(/{{value}}/g, value);
Copy link
Member

Choose a reason for hiding this comment

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

💭 Too bad replaceAll() is not yet available in our node version. It would allow us to avoid constructing a regex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep :( I initially used that but the CI crashed

@afgomez afgomez enabled auto-merge (squash) February 19, 2021 12:00
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1045 1037 -8

Async chunks

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

id before after diff
infra 1.9MB 1.9MB -47.0B

Page load bundle

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

id before after diff
infra 194.4KB 165.0KB -29.4KB

History

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

@afgomez afgomez merged commit 4d34a13 into elastic:master Feb 19, 2021
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 19, 2021
* master: (111 commits)
  [Logs UI] Replace dependencies in the infra bundle (elastic#91503)
  [Search Source] Do not request unmapped fields if source filters are provided (elastic#91921)
  [APM] Kql Search Bar suggests values outside the selected time range (elastic#91918)
  Refactored component edit policy tests into separate folders and using client integration testing setup (elastic#91657)
  [Fleet] Don't error on missing package_assets value (elastic#91744)
  [Lens] Pass used histogram interval to chart (elastic#91370)
  [Indexpattern management] Use indexPatterns Service instead of savedObjects client (elastic#91839)
  [Security Solutions] Fixes Cypress tests for indicator match by making the selectors more specific (elastic#91947)
  [CI] backportrc can skip CI (elastic#91886)
  Revert "[SOM] fix flaky suites (elastic#91809)"
  [Fleet] Install Elastic Agent integration by default during setup (elastic#91676)
  [Fleet] Silently swallow 404 errors when deleting ingest pipelines (elastic#91778)
  [data.search] Use incrementCounter for search telemetry (elastic#91230)
  [Fleet] Bootstrap functional test suite (elastic#91898)
  [Alerts][Docs] Added API documentation for alerts plugin (elastic#91067)
  Use correct environment in anomaly detection setup link (elastic#91877)
  [FTSR] Convert to tasks and add jest/api integration suites (elastic#91770)
  [CI] Build and publish storybooks (elastic#87701)
  docs: add PHP agent info to docs (elastic#91773)
  [DOCS] Adds and updates Visualization advanced settings (elastic#91904)
  ...
afgomez pushed a commit to afgomez/kibana that referenced this pull request Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logs UI Logs UI feature performance release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services technical debt Improvement of the software architecture and operational architecture v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants