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+] Refactor state and URL persistence of Log Explorer #170200

Merged
merged 77 commits into from
Dec 11, 2023

Conversation

Kerry350
Copy link
Contributor

@Kerry350 Kerry350 commented Oct 31, 2023

📝 Summary

This attempts to prevent the embedded Discover component from performing any side-effects outside of its component hierarchy. It also introduces a clean "controller" API between the <LogExplorer> component and the observability logs explorer app.

❤️ Notes for reviewers outside of the Logs UX team

Sorry for messing with your code-bases. I tried to keep the changes minimal.

  • Changes to the discover plugin: Currently the persistence of the state to the URL is hard-coded in Discover and seems to be deeply entangled with the state management. In order to be able to mount it as a component that doesn't have page-wide side-effects, this allows the consumer to optionally specify the state storage instance. That way they can pass an in-memory state storage to decouple the Discover component from the URL.
  • Changes to the kibana_utils package: The kibana url state storage utils themselves have some quirky behavior due to which they don't always access the URL via the passed history instance, but read from window.location directly. These small changes try to prevent that access from happening in Discover without interfering too much with other usages.
  • Changes to the SLO test: The test didn't mock the location correctly and was therefore impacted by above quirk.

🕵️‍♀️ Notes for Logs UX reviewers

Static and dynamic composition

image

Logs Explorer component

Goals

Decoupling from global state: The primary goal is to decouple the <LogExplorer> component from direct dependencies on the URL and other page-wide side effects. By achieving this decoupling, the <LogExplorer> becomes more modular, easier to test, and enables its use in other applications without interfering with their page state.

Stable and Strictly Typed Public API: Introduce a public API for the <LogExplorer> component. This API will provide functionalities to subscribe to the component's state and to invoke actions for state modifications. Implement a stable and strictly typed representation of the component’s public state, so the type checker can ensure the correct usage by all consuming apps.

Architecture

The architecture of the <LogExplorer> component has been designed to provide a clear and modular structure, with a focus on separation of concerns and flexibility in state management. This structure is primarily composed of the uncontrolled <LogExplorer> component and a separately instantiable controller.

Uncontrolled <LogExplorer> component: The <LogExplorer> is designed as an uncontrolled component. This design choice aligns with how its state management is structured and is consistent with the nature of the components it encompasses, many of which are uncontrolled themselves.

Separately instantiable controller: A controller is introduced to encapsulate and manage the business logic associated with the <LogExplorer> component. The controller centralizes the business logic, separating it from the UI layer. This provides flexibility in managing different instances of the <LogExplorer> with potentially varying logic or state requirements. The controller thereby offers a single entry point for customizations, allowing consumers to tailor the state and side-effect management as well as the behavior and presentation of sub-components.

Observability Logs Explorer App

Goals

URL persistence: Implement a versioned data structure for URL persistence, eliminating the need for the previously used workaround involving the data view ID.

Preparation for Saved Object Persistence: This versioned data structure would also be suitable for future persistence in a saved object.

Changes

Introduction of versioned URL schema: This new schema will standardize how URL-based state is managed, providing a clear and consistent mechanism for encoding and decoding state information in the URL. The versioning will allow us to evolve the data structure in a backwards-compatible way incorporate features added or changed in the future.

Update to locators: This modification ensures that all parts of the application that handle URL generation and interpretation are aligned with the new versioned schema. As part of this the sort parameter was removed, because its meaning in the Logs Explorer is not clear.

Page-level statechart implementation: This introduces a page-level statechart to orchestrate the initialization and instantiation of the <LogExplorer> controller. The statechart approach provides a structured and predictable way to manage the various states and transitions associated with the <LogExplorer> component. This ensures a stable and controlled flow from initialization to full instantiation.

Refinement of page structure: This refinement ensures that the page structure accurately reflects the various states as defined in the statechart. By adhering to type-safety principles, the application reduces the likelihood of runtime errors and improves maintainability. The translation between the states and component properties is performed light-weight connector components similar to the pattern established in redux.

App statechart

image

@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.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@Kerry350 Kerry350 force-pushed the 165255-extract-URL-state-persistence branch from 2340a87 to b1d2c35 Compare October 31, 2023 21:51
@weltenwort weltenwort self-assigned this Nov 3, 2023
@weltenwort
Copy link
Member

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@weltenwort
Copy link
Member

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@botelastic botelastic bot added the Team:obs-ux-management Observability Management User Experience Team label Dec 5, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Obs Ux Changes LGTM !

@weltenwort
Copy link
Member

weltenwort commented Dec 8, 2023

the test failures seem to be unrelated

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Data Discovery changes LGTM! I did some quick local testing for any impact to Discover in the O11y serverless project, and it was working as expected.

Only one thing I noticed: the additional state being passed through "Open in Discover" is great, but this PR undoes the changes in #171467 to remove the Discover icon (a design request) and add sort to the transferred state (I'd recommend keeping this if columns continue to be included, otherwise it's confusing). But I'll leave it up to your team to decide what you want to do here since the Discover changes look good regardless.

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.

All the latest changes LGTM, thanks again for this work! 👏

@weltenwort
Copy link
Member

Thank you for the review, @davismcphee!

this PR undoes the changes in #171467

I know I clobbered some the changes 😇 We're going to iterate on the state-passing links overall now that the basis is in.

@vadimkibana vadimkibana requested a review from Dosant December 11, 2023 13:04
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

kibana_utils changes lgtm.
I think would be great to go a bit further and completely remove window.location path from there, I might follow up on that

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
logExplorer 646 666 +20
observabilityLogExplorer 132 173 +41
total +61

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/deeplinks-observability 19 21 +2
@kbn/xstate-utils 12 13 +1
logExplorer 26 83 +57
observabilityLogExplorer 15 18 +3
total +63

Async chunks

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

id before after diff
discover 593.8KB 593.9KB +130.0B
logExplorer 360.6KB 360.7KB +97.0B
observabilityLogExplorer 89.3KB 138.0KB +48.6KB
total +48.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
logExplorer 8 16 +8

Page load bundle

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

id before after diff
kibanaUtils 73.7KB 73.7KB +35.0B
logExplorer 39.1KB 38.9KB -150.0B
observabilityLogExplorer 32.9KB 11.4KB -21.5KB
total -21.7KB
Unknown metric groups

API count

id before after diff
@kbn/deeplinks-observability 29 31 +2
@kbn/xstate-utils 12 13 +1
logExplorer 26 83 +57
observabilityLogExplorer 15 18 +3
total +63

async chunk count

id before after diff
logExplorer 6 5 -1
observabilityLogExplorer 1 2 +1
total -0

ESLint disabled line counts

id before after diff
@kbn/deeplinks-observability 1 3 +2

Total ESLint disabled count

id before after diff
@kbn/deeplinks-observability 1 3 +2

History

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

cc @weltenwort

@weltenwort weltenwort merged commit dbabd6d into elastic:main Dec 11, 2023
36 checks passed
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Dec 11, 2023
yngrdyn added a commit that referenced this pull request Jan 31, 2024
## 📝 Summary

This PR contains `dataset-quality` plugin state management, we have
decided to go with xstate.
The general idea, and following
#170200 as inspiration, we wanted
to detach `dataset-quality` plugin state from its consumers, in this way
our plugin wouldn't perform side effect, such as updating routes,
outside its scope.

The flow of the information now looks like

<img width="543" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/6a723f5b-047b-4f81-aef5-dbfce1ac0181">


Current internal state of the plugin is fairly simple but it's
envisioned to grow in a near future with for example filters, flyout
options, etc.

## 💡 Notes for Logs UX reviewers

### Dataset Quality plugin

#### Goals

**Decoupling from global state**: The primary goal is to decouple the
`<DatasetQuality>` component from direct dependencies on the URL and
other page-wide side effects. Decoupling from global state enables its
use in other applications without interfering with their page state.

**Stable and Strictly Typed Public API**: Introduce a public API for the
`<DatasetQuality>` component. This API provides consumers the ability to
subscribe to the component's state and/or initialize the state from the
outside.

#### Architecture

The architecture of the `<DatasetQuality>` plugin has been designed to
provide a modular structure, with a focus on robust state management.
This structure is primarily composed of the uncontrolled
`<DatasetQuality>` component and a separately instantiable controller.

**Uncontrolled `<DatasetQuality>` component**: The `<DatasetQuality>` is
designed as an uncontrolled component. All the logic around this
component is handled by hooks, such as `useDatasetQualityTable` with the
ability to change or replace the underlying dependencies.

**Separately instantiable controller**: A controller is introduced to
encapsulate and manage the business logic associated with the
`<DatasetQuality>` component. The controller centralizes the business
logic, separating it from the UI layer. This provides flexibility in
managing different instances of the `<DatasetQuality>` and reusability
from different consumers.

#### App statechart

<img width="1041" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/7da9e424-8577-4a5e-8c3e-46fb1df83948">

### Observability Logs Explorer App

#### Goals

**URL persistence**: Implement a versioned data structure for URL
persistence, this give us the flexibility to extend or change the app
state without workarounds. The general idea of having the public state
of the dataset quality plugin stored in the URL of this consumer is the
ability to share an exact state with colleagues, customers, etc.

#### Changes

**Introduction of versioned URL schema**: This new schema will
standarize how URL-based state is managed, providing a clear and
consistent mechanism for encoding and decoding state information in the
URL. The versioning will allow us to evolve the data structure in a
backwards-compatible way incorporate features added or changed in the
future.

**Page-level statechart implementation**: This introduces a page-level
statechart to orchestrate the initialization and instantiation of the
`<DatasetQuality>` controller.

#### App statechart

<img width="936" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/e3f1c61b-322b-4054-a30e-157eadb6de6b">

---------

Co-authored-by: kibanamachine <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
## 📝 Summary

This PR contains `dataset-quality` plugin state management, we have
decided to go with xstate.
The general idea, and following
elastic#170200 as inspiration, we wanted
to detach `dataset-quality` plugin state from its consumers, in this way
our plugin wouldn't perform side effect, such as updating routes,
outside its scope.

The flow of the information now looks like

<img width="543" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/6a723f5b-047b-4f81-aef5-dbfce1ac0181">


Current internal state of the plugin is fairly simple but it's
envisioned to grow in a near future with for example filters, flyout
options, etc.

## 💡 Notes for Logs UX reviewers

### Dataset Quality plugin

#### Goals

**Decoupling from global state**: The primary goal is to decouple the
`<DatasetQuality>` component from direct dependencies on the URL and
other page-wide side effects. Decoupling from global state enables its
use in other applications without interfering with their page state.

**Stable and Strictly Typed Public API**: Introduce a public API for the
`<DatasetQuality>` component. This API provides consumers the ability to
subscribe to the component's state and/or initialize the state from the
outside.

#### Architecture

The architecture of the `<DatasetQuality>` plugin has been designed to
provide a modular structure, with a focus on robust state management.
This structure is primarily composed of the uncontrolled
`<DatasetQuality>` component and a separately instantiable controller.

**Uncontrolled `<DatasetQuality>` component**: The `<DatasetQuality>` is
designed as an uncontrolled component. All the logic around this
component is handled by hooks, such as `useDatasetQualityTable` with the
ability to change or replace the underlying dependencies.

**Separately instantiable controller**: A controller is introduced to
encapsulate and manage the business logic associated with the
`<DatasetQuality>` component. The controller centralizes the business
logic, separating it from the UI layer. This provides flexibility in
managing different instances of the `<DatasetQuality>` and reusability
from different consumers.

#### App statechart

<img width="1041" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/7da9e424-8577-4a5e-8c3e-46fb1df83948">

### Observability Logs Explorer App

#### Goals

**URL persistence**: Implement a versioned data structure for URL
persistence, this give us the flexibility to extend or change the app
state without workarounds. The general idea of having the public state
of the dataset quality plugin stored in the URL of this consumer is the
ability to share an exact state with colleagues, customers, etc.

#### Changes

**Introduction of versioned URL schema**: This new schema will
standarize how URL-based state is managed, providing a clear and
consistent mechanism for encoding and decoding state information in the
URL. The versioning will allow us to evolve the data structure in a
backwards-compatible way incorporate features added or changed in the
future.

**Page-level statechart implementation**: This introduces a page-level
statechart to orchestrate the initialization and instantiation of the
`<DatasetQuality>` controller.

#### App statechart

<img width="936" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/e3f1c61b-322b-4054-a30e-157eadb6de6b">

---------

Co-authored-by: kibanamachine <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
## 📝 Summary

This PR contains `dataset-quality` plugin state management, we have
decided to go with xstate.
The general idea, and following
elastic#170200 as inspiration, we wanted
to detach `dataset-quality` plugin state from its consumers, in this way
our plugin wouldn't perform side effect, such as updating routes,
outside its scope.

The flow of the information now looks like

<img width="543" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/6a723f5b-047b-4f81-aef5-dbfce1ac0181">


Current internal state of the plugin is fairly simple but it's
envisioned to grow in a near future with for example filters, flyout
options, etc.

## 💡 Notes for Logs UX reviewers

### Dataset Quality plugin

#### Goals

**Decoupling from global state**: The primary goal is to decouple the
`<DatasetQuality>` component from direct dependencies on the URL and
other page-wide side effects. Decoupling from global state enables its
use in other applications without interfering with their page state.

**Stable and Strictly Typed Public API**: Introduce a public API for the
`<DatasetQuality>` component. This API provides consumers the ability to
subscribe to the component's state and/or initialize the state from the
outside.

#### Architecture

The architecture of the `<DatasetQuality>` plugin has been designed to
provide a modular structure, with a focus on robust state management.
This structure is primarily composed of the uncontrolled
`<DatasetQuality>` component and a separately instantiable controller.

**Uncontrolled `<DatasetQuality>` component**: The `<DatasetQuality>` is
designed as an uncontrolled component. All the logic around this
component is handled by hooks, such as `useDatasetQualityTable` with the
ability to change or replace the underlying dependencies.

**Separately instantiable controller**: A controller is introduced to
encapsulate and manage the business logic associated with the
`<DatasetQuality>` component. The controller centralizes the business
logic, separating it from the UI layer. This provides flexibility in
managing different instances of the `<DatasetQuality>` and reusability
from different consumers.

#### App statechart

<img width="1041" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/7da9e424-8577-4a5e-8c3e-46fb1df83948">

### Observability Logs Explorer App

#### Goals

**URL persistence**: Implement a versioned data structure for URL
persistence, this give us the flexibility to extend or change the app
state without workarounds. The general idea of having the public state
of the dataset quality plugin stored in the URL of this consumer is the
ability to share an exact state with colleagues, customers, etc.

#### Changes

**Introduction of versioned URL schema**: This new schema will
standarize how URL-based state is managed, providing a clear and
consistent mechanism for encoding and decoding state information in the
URL. The versioning will allow us to evolve the data structure in a
backwards-compatible way incorporate features added or changed in the
future.

**Page-level statechart implementation**: This introduces a page-level
statechart to orchestrate the initialization and instantiation of the
`<DatasetQuality>` controller.

#### App statechart

<img width="936" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/e3f1c61b-322b-4054-a30e-157eadb6de6b">

---------

Co-authored-by: kibanamachine <[email protected]>
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 Feature:LogsExplorer Logs Explorer feature release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team Team:obs-ux-management Observability Management User Experience Team v8.13.0
Projects
None yet