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] Make URL syncing optional in the Log View state machine #154061

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

Kerry350
Copy link
Contributor

@Kerry350 Kerry350 commented Mar 30, 2023

Summary

This fixes #154030 (and other uses of the Log Stream embeddable component).

The embeddable component calls useLogView() directly, and this causes issues with context dependencies for URL syncing from the consumers.

This PR makes the URL actions / services optional within the machine.

Uses:

  • <LogViewProvider /> used for all main Logs pages (stream, anomalies, categories) has full URL syncing ✅
  • <LogViewProvider /> used within our Logs alert editor does not have URL syncing (not needed) ❌
  • useLogView() as used by the embeddable component does not have URL syncing ❌
  • useLogView() as used by RedirectToNodeLogs does not have URL syncing (not needed, URL syncing kicks in after redirect) ❌

The default / pure implementation of initializeFromUrl just does a send({ type: 'INITIALIZED_FROM_URL', logViewReference: null }) as the state machine needs to transition to it's initialized state and is already set up to use the initial context reference if there's no reference obtained from the URL.

Examples:

Screenshot 2023-03-30 at 15 23 48

Screenshot 2023-03-30 at 15 24 39

@Kerry350 Kerry350 added bug Fixes for quality problems that affect the customer experience Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes labels Mar 30, 2023
@Kerry350 Kerry350 self-assigned this Mar 30, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #21 / dashboard Reporting Dashboard Reporting Screenshots Sample data from Kibana 7.6 PNG file matches the baseline image
  • [job] [logs] FTR Configs #59 / maps app maps loaded from sample data ecommerce "before all" hook for "should load layers"

Metrics [docs]

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.4MB 1.4MB +522.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 432 435 +3

Total ESLint disabled count

id before after diff
securitySolution 512 515 +3

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

cc @Kerry350

@Kerry350 Kerry350 marked this pull request as ready for review March 30, 2023 16:16
@Kerry350 Kerry350 requested a review from a team as a code owner March 30, 2023 16:16
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick fix 👏

@Kerry350 Kerry350 merged commit 6842039 into elastic:main Mar 31, 2023
@kibanamachine kibanamachine added v8.8.0 backport:skip This commit does not require backporting labels Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience Feature:Logs UI Logs UI feature 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 v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infrastructure UI] urlStateStorage is missing when integrating LogsStream
6 participants