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+] Implement Logs Dataset selector #159907

Conversation

tonyghiani
Copy link
Contributor

@tonyghiani tonyghiani commented Jun 19, 2023

📓 Summary

Closes https://github.com/elastic/observability-dev/issues/2655

This PR introduces a customized log consumption experience in the Discover plugin. By leveraging the new discover_log_explorer plugin and utilizing the discover.customize functionality, we have curated a more tailored user experience.

The key feature of this implementation is the DatasetSelector component, which replaces the original Discover DataViewPicker. It handles the retrieval, rendering, and navigation of integrations and data streams related to logs, providing an improved user interface.

This PR involves significant development efforts, including the creation of the discover_log_explorer plugin, implementation of services, state machines, custom hooks, and enhancements to presentational components. The following overview will help reviewers understand the responsibilities of each component in this implementation.

demo-selector-fhd.mov

DatasetsService & DatasetsClient

The DatasetsService is introduced, a crucial component that mediates access to the newly implemented DatasetsClient. During the plugin's lifecycle, the DatasetsService exposes a client property through its start() method, providing convenient access to a DatasetsClient instance.

The DatasetsClient is responsible for abstracting the data fetching process for two endpoints: the integrations endpoint and the data streams listing endpoint. These endpoints are utilized to populate the selector options in the user interface. To facilitate this, the DatasetsClient exposes the findIntegrations and findDatasets methods, which handle the respective data fetching.

Discover Customization

The critical part of this work consists of where the customization is applied.
Inside the public/plugin.tsx, we lazy load and create, injecting the required dependencies, the CustomDatasetSelector, which already encapsulates all the logic required to make the selector work with the external APIs.
We kept separating the data fetching logic from how the selector works, and all the data and events are passed into the UI component with properties.

discover.customize(
  DISCOVER_LOG_EXPLORER_PROFILE_ID,
  ({ customizations, stateContainer }) => {

    customizations.set({
      id: 'search_bar',
      CustomDataViewPicker: createLazyCustomDatasetSelector({
        datasetsClient: datasetsService.client,
        stateContainer,
      }),
    });
    ...

Data fetching state machines & custom hooks

To handle the data fetching of integrations and unmanaged data streams, we created two different state machines to separately handle the related action for each dataset, such as remote search, in-memory search, error handling etc.

Integration machine and useIntegrations

The integrations state machine handles automatic data fetching of the resources and additionally provides transitions for loading more integrations, searching integrations by HTTP request, searching locally into integration streams, and all the related loading and error handling states.

It is then interpreted inside the useIntegrations custom hook, which exposes the fetched data and handlers for all the above-mentioned actions.

Screenshot 2023-05-30 at 09 44 42

Datasets machine and useDatasets

Similar to the integrations state machine, but simplified since the data streams search can only happen with HTTP requests and there is no pagination that requires to handle the load of more entries.

It is interpreted inside the useDatasets custom hook, which also exposes the fetched data and handlers for the available actions.

Screenshot 2023-05-30 at 09 45 11

DatasetSelector

The DatasetSelector component contains all the logic that manages the navigation and searches across the different panels that render integrations, integrations' streams or unmanaged streams.
As the datasets come from different APIs or are performed in-memory, the search work follow this logic:

  • When listing the integrations list (first level of the EuiContextMenu), the search is done with an HTTP request.
  • When listing the data streams list for a specific integration (second level of the EuiContextMenu), the search is done in-memory, filtering and sorting directly in the client.
  • When listing the unmanaged data streams list (second level of the EuiContextMenu), the search is done again with an HTTP request.

To handle these possible user journeys correctly without side effects, we created another state machine and exposed its actions with an internal useDatasetSelector custom hook.

Screenshot 2023-05-30 at 09 46 04

Next steps

This component will change quite a lot until we won't get to a final design. As soon as a first solid mvp is defined for production, a complete test for the component will be implemented, among with a more generic functional test for the core customization features.

Marco Antonio Ghiani and others added 30 commits June 19, 2023 12:25
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.

I left a few final comments below. Thanks for following up on the previous ones.

One thing I noticed during testing is that a user might want to reload the list of integrations or the list of uncategorized datasets even when no error occured before. During testing, for example, I started a shipper in the background but its new data stream didn't show up until I navigated away and back. Any idea how we could allow triggering a reload without cluttering the UI?

@tonyghiani
Copy link
Contributor Author

tonyghiani commented Jun 27, 2023

One thing I noticed during testing is that a user might want to reload the list of integrations or the list of uncategorized datasets even when no error occured before. During testing, for example, I started a shipper in the background but its new data stream didn't show up until I navigated away and back. Any idea how we could allow triggering a reload without cluttering the UI?

The data are not reloaded after switching panel because this ensures that we can keep in memory the loaded integrations after some scrolling, without losing the loaded entries.
Some other benefits allow us to keep the current search results while navigating between panels without the need to perform each time the search and allow for enabling the client-side cache for those entries, which is pretty handful when performing multiple searches.

I honestly didn't think about the case the user would like to see the newly available dataset or integrations in an almost real-time manner, due to the fact that ideally the installed integrations and unmanaged dataset shouldn't change very often.
I think the trade-off in re-fetching the API results or reducing the cache TTL won't be in favour of the smooth user experience we provide now, compared to the need of the user to refresh the page (which is something they eventually do anyway after expecting to see new results) (my 2 cents, I'm mostly talking from a developer perspective, but I'm sure that for other apps' use cases, having faster feedback on new content is critical 🤓 ).

If we want to provide anyway a way for the user to refresh the list without reloading the page, I can't think of any behind-the-scene immediate solution, while having a small call to action such a button can give them this option straight away. Maybe something like this?

Screenshot 2023-06-27 at 19 08 36

@weltenwort
Copy link
Member

Yes, maybe my testing procedure was not exactly representative of normal usage. Once we have the selection state restoration in place a reload will be less jarring anyway.

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.

Amazing work overall, very well done 👏

Aside from the great readability I was pleasantly surprised to see that you managed to not increase the size of any other bundle in any significant way. 🎉

As agreed upon via other channels, there are few high-priority follow-ups that we will want to implement next:

The merge of this PR will unblock the implementation of

Thank you for all the effort and care you invested into this!

@@ -28,6 +28,7 @@ export const storybookAliases = {
dashboard: 'src/plugins/dashboard/.storybook',
data: 'src/plugins/data/.storybook',
discover: 'src/plugins/discover/.storybook',
discover_log_explorer: 'x-pack/plugins/discover_log_explorer/.storybook',
Copy link
Member

Choose a reason for hiding this comment

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

Could we also add it to .buildkite/scripts/steps/storybooks/build_and_upload.ts as recently documented in #160473?

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discoverLogExplorer - 132 +132

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
discover 68 71 +3
fleet 1071 1073 +2
total +5

Async chunks

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

id before after diff
discover 529.8KB 529.8KB -8.0B
discoverLogExplorer - 184.2KB +184.2KB
fleet 980.7KB 979.9KB -813.0B
lens 1.3MB 1.3MB -8.0B
unifiedSearch 212.9KB 212.9KB +16.0B
total +183.4KB

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
discover 8 14 +6
fleet 35 36 +1
total +7

Page load bundle

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

id before after diff
discoverLogExplorer - 4.3KB +4.3KB
fleet 132.8KB 133.7KB +883.0B
stackAlerts 19.1KB 19.1KB -8.0B
total +5.2KB
Unknown metric groups

API count

id before after diff
discover 88 97 +9
fleet 1187 1189 +2
total +11

async chunk count

id before after diff
discoverLogExplorer - 5 +5

ESLint disabled in files

id before after diff
discoverLogExplorer - 2 +2

ESLint disabled line counts

id before after diff
discoverLogExplorer - 3 +3
enterpriseSearch 14 16 +2
securitySolution 413 417 +4
total +9

Total ESLint disabled count

id before after diff
discoverLogExplorer - 5 +5
enterpriseSearch 15 17 +2
securitySolution 492 496 +4
total +11

History

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

@tonyghiani tonyghiani merged commit 6a0d6de into elastic:main Jun 28, 2023
@tonyghiani tonyghiani deleted the 2655-implement-log-data-stream-selector-rebased branch June 28, 2023 13:20
@kibanamachine kibanamachine added v8.10.0 backport:skip This commit does not require backporting labels Jun 28, 2023
tonyghiani added a commit that referenced this pull request Jul 20, 2023
## 📓  Summary

Closes #160425 

After the [first implementation of the log-explorer
profile](#159907), we wanted to
restore the selection of the dataset for a user when landing on the
Discover log-explorer profile.

Since we create an ad-hoc data view for Discover starting from the
dataset details, we needed to develop a system for intercepting the
`index` query parameter (which is used by Discover as the source of
truth for restoring a data view), create our ad-hoc data view and store
in the URL an encoded ID with the required details to restore the
selection.

The following video shows the user journey for:
- Landing on the log-explorer profile with no index param, nothing to
restore and fallback to All log datasets.
- Landing on the log-explorer profile invalid index param, notify about
failure and fallback to All log datasets.
- Select a different dataset, applies the new data view and update the
URL. When the URL is accessed directly, restore and initialize the data
view for the selection.
- Navigate back and forth in the browser history, restoring the
selection and data view on `index` param changes.


https://github.com/elastic/kibana/assets/34506779/37a212ee-08e4-4e54-8e42-1d739c38f164

## 💡 Reviewer hints

To have better control over the page selection and the restore process,
we prepared the DatasetSelector component for [being controlled by the
parent component](#160971).
Having that ready, we now implemented a new top-level state machine with
the following responsibilities:
- Re-initialize (decompress/decode) the dataset selection from the
`index` query params.
- Derive and set into Discover state a new ad-hoc data view.
- Keep track of new dataset selection changes and update the URL state
and the current data view.

<img width="1224" alt="log-explorer-machine"
src="https://github.com/elastic/kibana/assets/34506779/67e3ff17-dc3f-4dcf-b6c0-f40dbbea2d44">

We found a race condition between the Discover URL initialization + data
view initialization against the log-explorer profile customizations
being applied.
To guarantee we correctly initialize the state machine and restore the
selection before Discover goes through its initialization steps, we need
to wait for the customization service to exist in Discover so that also
the customization callbacks are successfully invoked.

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
## 📓  Summary

Closes elastic#160425 

After the [first implementation of the log-explorer
profile](elastic#159907), we wanted to
restore the selection of the dataset for a user when landing on the
Discover log-explorer profile.

Since we create an ad-hoc data view for Discover starting from the
dataset details, we needed to develop a system for intercepting the
`index` query parameter (which is used by Discover as the source of
truth for restoring a data view), create our ad-hoc data view and store
in the URL an encoded ID with the required details to restore the
selection.

The following video shows the user journey for:
- Landing on the log-explorer profile with no index param, nothing to
restore and fallback to All log datasets.
- Landing on the log-explorer profile invalid index param, notify about
failure and fallback to All log datasets.
- Select a different dataset, applies the new data view and update the
URL. When the URL is accessed directly, restore and initialize the data
view for the selection.
- Navigate back and forth in the browser history, restoring the
selection and data view on `index` param changes.


https://github.com/elastic/kibana/assets/34506779/37a212ee-08e4-4e54-8e42-1d739c38f164

## 💡 Reviewer hints

To have better control over the page selection and the restore process,
we prepared the DatasetSelector component for [being controlled by the
parent component](elastic#160971).
Having that ready, we now implemented a new top-level state machine with
the following responsibilities:
- Re-initialize (decompress/decode) the dataset selection from the
`index` query params.
- Derive and set into Discover state a new ad-hoc data view.
- Keep track of new dataset selection changes and update the URL state
and the current data view.

<img width="1224" alt="log-explorer-machine"
src="https://github.com/elastic/kibana/assets/34506779/67e3ff17-dc3f-4dcf-b6c0-f40dbbea2d44">

We found a race condition between the Discover URL initialization + data
view initialization against the log-explorer profile customizations
being applied.
To guarantee we correctly initialize the state machine and restore the
selection before Discover goes through its initialization steps, we need
to wait for the customization service to exist in Discover so that also
the customization callbacks are successfully invoked.

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
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 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.