-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 Explorer] Support logs data views #176078
[Logs Explorer] Support logs data views #176078
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
874d168
to
524d93d
Compare
...s/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.test.ts
Show resolved
Hide resolved
...lugins/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.ts
Outdated
Show resolved
Hide resolved
...lugins/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.ts
Show resolved
Hide resolved
|
||
export class DataViewDescriptor { | ||
id: DataViewDescriptorType['id']; | ||
dataType: DataViewDescriptorType['dataType']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: this property will be useful to differentiate between different data views and group them by its value.
* 2.0. | ||
*/ | ||
|
||
export const buildIndexPatternRegExp = (basePatterns: string[]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Starting from a single base pattern such as logs
, metrics
, etc.., this function allows to create a regular expression to test an index pattern for:
- word boundaries and subsequent paths (
logs-
,logs-system.syslog-default
, etc...) - local and remote clusters (
logs-
,remote_cluster:logs-*
, etc...) - multiple patterns, supporting above features (
logs-*,remote_cluster:logs-*
, etc...)
if (!('discoverStateContainer' in context)) return; | ||
const { discoverStateContainer } = context; | ||
|
||
return discoverStateContainer.actions.onChangeDataView( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: this action fetches and sets the complete data view object by its ID using the Discover pre-existing logic.
/oblt-deploy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very solid work overall, I had a good time reading it! 👏 The changes to the state machines make them more consistent. It's also good to see how the discover
plugin was removed as a dependency throughout the selector hierarchy. This will make its re-use in other locations easier.
I left just a few questions and suggestions below.
And thanks for updating the storybook, which was very helpful in testing the loading and error states.
...s/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.test.ts
Show resolved
Hide resolved
...lugins/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.ts
Show resolved
Hide resolved
...lugins/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.ts
Outdated
Show resolved
Hide resolved
...lugins/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.ts
Outdated
Show resolved
Hide resolved
...lugins/observability_solution/logs_explorer/common/data_views/models/data_view_descriptor.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/observability_solution/logs_explorer/public/controller/create_controller.ts
Outdated
Show resolved
Hide resolved
...gs_explorer/public/state_machines/logs_explorer_controller/src/services/data_view_service.ts
Show resolved
Hide resolved
): ActionFunction<LogsExplorerControllerContext, LogsExplorerControllerEvent> => | ||
(context, event) => { | ||
if (event.type === 'UPDATE_DATASET_SELECTION' && isDataViewSelection(event.data)) { | ||
return redirectToDiscover({ context, datasetSelection: event.data, discover }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way the action is implemented (and how the selector acted before) would cause the page-wide side-effect of navigating to a different URL. Is that desirable when the logs explorer is used in an embedded fashion? Should we delegate that to the consumer?
If so, how do we do that? 🤔 Two options come to mind:
- a customization that customizes the action that happens here (like the one used for the ai assistant)
- emitting an event on the (yet unused)
event$
observable that the consumer can react to
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point here and both the proposed solutions LGTM.
On the other hand, what do you think should be the fallback behaviour when the consumer doesn't apply the customization or doesn't react to the event?
My concern is we might end up with a broken UX when the user clicks on a non-logs data view if the consumer embedding the LogsExplorer doesn't react to the emitted event in $event
(assuming we go with the second option).
One option to keep the default behaviour without forcing the user to react to the emitted event, but still provide the flexibility to perform custom logic and prevent the default behaviour if necessary could be to provide something like a preventDefault()
method as part of the emitted event, in a browser-fashion way.
What I mean is, in case we'll extend the set of events emitted by the controller, to have something like this:
export type LogsExplorerPublicEvent =
| {
type: 'REDIRECT_TO_DISCOVER';
data: { context, eventPayload };
preventDefault: () => void;
}
| {
type: 'ANOTHER_EVENT';
data: { ... };
preventDefault: () => void;
}
| ...
This is just one possible approach, but it might be more convoluted than other options, what do you think?
Or did I misunderstand your suggestion and you meant something different? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm I've been thinking more about the approach I proposed for preventing the default behaviour as we might have more subscribers on the same emitted event, which makes it impractical to decide whether the default behaviour should be skipped or not...
Adding a customization point would work, but it still leaves the discussion open about what happens if the consumer doesn't register the customization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on a offline conversation with Felix, we'll implement a customization point to delegate the redirect to the consumer in a shorted upcoming pull request.
isDataViewSelection(context.datasetSelection) && | ||
context.datasetSelection.selection.dataView.isUnknownDataType() | ||
) { | ||
return redirectToDiscover({ context, datasetSelection: context.datasetSelection, discover }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, should this side-effect happen here? Or should it be delegated to the consumer too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same work mentioned here will apply to this part.
...gs_explorer/public/state_machines/logs_explorer_controller/src/services/selection_service.ts
Show resolved
Hide resolved
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
History
To update your PR or re-run it, just comment with: |
// Apply base patterns union for local and remote clusters | ||
const localAndRemotePatternGroup = `((${basePatternGroup})|([^:,\\s]+:${basePatternGroup}))`; | ||
// Handle trailing comma and multiple pattern concatenation | ||
return new RegExp(`^${localAndRemotePatternGroup}(,${localAndRemotePatternGroup})*(,$|$)`, 'i'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really loved the explanatory way of creating the regex 💯
## 📓 Summary Closes elastic#175767 🧪 You can access a live deployment of this PR [here](https://issue-deploy-kibana-pr176078.kb.us-west2.gcp.elastic-cloud.com/app/observability-logs-explorer/). There has been a lot of talking around supporting selection for data views and staying on the Logs Explorer for those concerning logs data streams. This work supports selecting and exploring Discover data views on Logs Explorer. This is currently limited to logs data views. https://github.com/elastic/kibana/assets/34506779/cccd6863-e1c1-4fa6-a530-9aed5d8d97c1 ## Next steps We had already an offline conversation with the team about how naming the selector, selection modes and related entities is becoming inconsistent and more difficult to maintain. To keep this PR narrowed to the data views support, an upcoming PR will focus on renaming according to the new selector's responsibilities. ## Core changes ### DataViewDescriptor The `DataViewDescriptor` instance is a way to describe a data view in the context of Logs Explorer. This does not represent a DataView object complete of fields and all the details provided with a DataView instance, but it instead encapsulates the logic around identifying what data type a data view is about, as well as defining the logic to use it with the new `dataView` selection mode. It creates a new instance starting from a `DataViewListItem` object, which is a minimal object provided by the dataViews service to list existing data views. ### LogExplorerController The `LogExplorerController` state machine now handles the selected entry depending on its type, triggering different flows. There are 3 different journeys depending on the selected entry: - For a data view entry which is not about logs, the page redirects to Discover with the data view selected - For a data view entry about logs, the data loads in the Logs Explorer, switching to the persisted DataView. - For a dataset entry, an ad-hoc data view loads in the Logs Explorer. To avoid updating twice the data view (once during initialization, and immediately after during selection validation), the validation flow has been anticipated and restructured to follow different flows, depending on the selection type. <img width="1953" alt="Screenshot 2024-02-09 at 12 02 10" src="https://github.com/elastic/kibana/assets/34506779/a63201f5-67b2-4890-8823-6ffd6691e249"> ### Dataset selector The selector state machine unifies the selection handler and expands the selection modes, adding a new `dataView` mode which handles logs data view selections. <img width="1281" alt="Screenshot 2024-02-05 at 16 24 31" src="https://github.com/elastic/kibana/assets/34506779/80e04331-7b93-40fc-af1d-32ef4ef705f5"> --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
## 📓 Summary This work is a follow-up of the newly introduced [support to logs backed data views](#176078) in Logs Explorer. In [a comment](#176078 (comment)) from the PR mentioned above, we discussed delegating to the consumers <LogsExplorer /> the responsibility to redirect to discover when selecting a non-logs data view, to prevent hard-coding a page-wide side-effect of navigating to a different URL. This introduces a new customization interface for the LogsExplorer that controls specific actions, starting from the first added event `onUknownDataViewSelection`. In case the consumers of this component do not provide the event handler, the data view entry in the data source selector will appear disabled and will not be clickable. <img width="412" alt="Screenshot 2024-02-13 at 15 45 30" src="https://github.com/elastic/kibana/assets/34506779/7933007d-75a0-4094-bf55-a3361637f4d1"> ## Example When creating the controller to pass into the LogsExplorer component, we can specify the event for handling the discover navigation as follow: ```ts createLogsExplorerController({ customizations: { events: { onUknownDataViewSelection: (context) => { /* ... */ }, }, }, }); ``` A use case for such usage is, for instance, that some consumers might want to prompt the user somehow before performing the navigation, or simply they don't want to do any navigation. --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
## 📓 Summary Closes elastic#175767 🧪 You can access a live deployment of this PR [here](https://issue-deploy-kibana-pr176078.kb.us-west2.gcp.elastic-cloud.com/app/observability-logs-explorer/). There has been a lot of talking around supporting selection for data views and staying on the Logs Explorer for those concerning logs data streams. This work supports selecting and exploring Discover data views on Logs Explorer. This is currently limited to logs data views. https://github.com/elastic/kibana/assets/34506779/cccd6863-e1c1-4fa6-a530-9aed5d8d97c1 ## Next steps We had already an offline conversation with the team about how naming the selector, selection modes and related entities is becoming inconsistent and more difficult to maintain. To keep this PR narrowed to the data views support, an upcoming PR will focus on renaming according to the new selector's responsibilities. ## Core changes ### DataViewDescriptor The `DataViewDescriptor` instance is a way to describe a data view in the context of Logs Explorer. This does not represent a DataView object complete of fields and all the details provided with a DataView instance, but it instead encapsulates the logic around identifying what data type a data view is about, as well as defining the logic to use it with the new `dataView` selection mode. It creates a new instance starting from a `DataViewListItem` object, which is a minimal object provided by the dataViews service to list existing data views. ### LogExplorerController The `LogExplorerController` state machine now handles the selected entry depending on its type, triggering different flows. There are 3 different journeys depending on the selected entry: - For a data view entry which is not about logs, the page redirects to Discover with the data view selected - For a data view entry about logs, the data loads in the Logs Explorer, switching to the persisted DataView. - For a dataset entry, an ad-hoc data view loads in the Logs Explorer. To avoid updating twice the data view (once during initialization, and immediately after during selection validation), the validation flow has been anticipated and restructured to follow different flows, depending on the selection type. <img width="1953" alt="Screenshot 2024-02-09 at 12 02 10" src="https://github.com/elastic/kibana/assets/34506779/a63201f5-67b2-4890-8823-6ffd6691e249"> ### Dataset selector The selector state machine unifies the selection handler and expands the selection modes, adding a new `dataView` mode which handles logs data view selections. <img width="1281" alt="Screenshot 2024-02-05 at 16 24 31" src="https://github.com/elastic/kibana/assets/34506779/80e04331-7b93-40fc-af1d-32ef4ef705f5"> --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
## 📓 Summary This work is a follow-up of the newly introduced [support to logs backed data views](elastic#176078) in Logs Explorer. In [a comment](elastic#176078 (comment)) from the PR mentioned above, we discussed delegating to the consumers <LogsExplorer /> the responsibility to redirect to discover when selecting a non-logs data view, to prevent hard-coding a page-wide side-effect of navigating to a different URL. This introduces a new customization interface for the LogsExplorer that controls specific actions, starting from the first added event `onUknownDataViewSelection`. In case the consumers of this component do not provide the event handler, the data view entry in the data source selector will appear disabled and will not be clickable. <img width="412" alt="Screenshot 2024-02-13 at 15 45 30" src="https://github.com/elastic/kibana/assets/34506779/7933007d-75a0-4094-bf55-a3361637f4d1"> ## Example When creating the controller to pass into the LogsExplorer component, we can specify the event for handling the discover navigation as follow: ```ts createLogsExplorerController({ customizations: { events: { onUknownDataViewSelection: (context) => { /* ... */ }, }, }, }); ``` A use case for such usage is, for instance, that some consumers might want to prompt the user somehow before performing the navigation, or simply they don't want to do any navigation. --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
## 📓 Summary Closes #177703 This work supports a new Logs Explorer UI setting to customize the data view patterns which allow to browse data views directly in Logs Explorer. It's a follow-up work of #176078, where we introduced a way to recognise logs-backed data views and explore their entires without redirecting to Discover. The previously hard-coded list of allowed patterns (`logs, auditbeat, filebeat, winbeat`) is now used as the default value for the UI setting "**Logs Explorer allowed data views**", where is possible to add more base patterns or full indices that should be available for exploration in Logs Explorer. ## 🎥 Demo https://github.com/elastic/kibana/assets/34506779/b2d43c2d-e0a3-4315-9d4c-be512a01245f ## 👣 Next steps - Create a dashboard to track the updates on this setting and link it to the team docs. --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
📓 Summary
Closes #175767
🧪 You can access a live deployment of this PR here.
There has been a lot of talking around supporting selection for data views and staying on the Logs Explorer for those concerning logs data streams.
This work supports selecting and exploring Discover data views on Logs Explorer. This is currently limited to logs data views.
Screen.Recording.2024-02-01.at.16.19.03.mov
Next steps
We had already an offline conversation with the team about how naming the selector, selection modes and related entities is becoming inconsistent and more difficult to maintain.
To keep this PR narrowed to the data views support, an upcoming PR will focus on renaming according to the new selector's responsibilities.
Core changes
DataViewDescriptor
The
DataViewDescriptor
instance is a way to describe a data view in the context of Logs Explorer. This does not represent a DataView object complete of fields and all the details provided with a DataView instance, but it instead encapsulates the logic around identifying what data type a data view is about, as well as defining the logic to use it with the newdataView
selection mode.It creates a new instance starting from a
DataViewListItem
object, which is a minimal object provided by the dataViews service to list existing data views.LogExplorerController
The
LogExplorerController
state machine now handles the selected entry depending on its type, triggering different flows. There are 3 different journeys depending on the selected entry:To avoid updating twice the data view (once during initialization, and immediately after during selection validation), the validation flow has been anticipated and restructured to follow different flows, depending on the selection type.
Dataset selector
The selector state machine unifies the selection handler and expands the selection modes, adding a new
dataView
mode which handles logs data view selections.