Open flash1293 opened 4 months ago
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)
cc @Kerry350
This should be part of the o11y root profile resolution - the method should check the state and add in the ad-hoc data view in case it doesn't exist.
This looks like something that will need to have access to the discover stateContainer
to add the newly created ad-hoc data view, similar to how it was done with Logs Explorer.
@davismcphee does any extension already provide access to the internal state management of Discover? I couldn't find any exposure of it through the contextual awareness implementation, which might be a blocker for this task. This also raises a second question, more technical, around the Discover state management and the profile resolution: is the state shared when the profile resolution process occurs? I think so but I'd like to confirm with you, and in that case I think it might require a clean-up mechanism on the profile resolution since we don't want the ad-hoc data view to stay registered once a different profile is resolved.
Let me know your thoughts, happy do chat more on this 👌
@tonyghiani Originally I thought we might be able to just create an ad hoc data view in the dataViews
service during resolution, and it would just work in Discover, but you're correct that we'll need a way to do this since we maintain a separate adHocDataViews
array in the Discover state.
is the state shared when the profile resolution process occurs?
Yes also correct, the stateContainer
is shared between profile resolutions. We can't currently switch root profiles after the initial load because they're based on the solution view type, but it's technically possible.
I'd prefer not to expose the stateContainer
to profiles since it wasn't designed as a public interface, and doing that would make it harder to change Discover's internal state management. I think the pattern we've been using of passing pieces of state as needed to extensions, and an actions
object for manipulating state, is easier to maintain and works for now at least.
So I think we'll need to do something to make this work in Discover. We've talked about this briefly before, but would it make sense to tie it in with the default ES|QL query work as a "default data source"? I'm thinking it could return an index pattern + time field for data view mode or a default query for ES|QL mode. We'd need to maintain the default data view logic in Discover for now, but they'd otherwise function the same I'd think. I can think of a couple ways this could work:
defaultDataSource
as a property of the RootProfileContext
returned by resolve
.getDefaultDataSource
extension point and expose it only to root profiles.getDefaultAppState
extension to include dataSource
, although that one is also exposed at the data source level, so we'd need to think it through a bit.In any case, exposing this capability through the context awareness framework means we could generate the data view ID in Discover and ensure it gets cleaned up on profile change. WDYT?
Regarding the exposure of stateContainer
to profiles, yours is a fair concern and sounds good to me to rely on the exposed actions to update the internal states.
So I think we'll need to do something to make this work in Discover. We've talked about this briefly before, but would it make sense to tie it in with the default ES|QL query work as a "default data source"? I think it could return an index pattern + time field for data view mode or a default query for ES|QL mode. We'd need to maintain the default data view logic in Discover for now, but they'd otherwise function the same I'd think.
I think this approach is a bit different from the original specs, we don't want to affect the default source on data-view directly but only provide a direct access point to the log sources settings for the user to explore. If I understood correctly, your suggestion would bind this data view to both the default data view in KQL and the default query in ESQL, which is not the aim of this change. It is something we could consider incrementally if we realize it makes sense to align them, but I'd rather start simple without magically affecting the user default data view choice when the o11y profile resolves.
Given these specs, I was thinking about something more of a lifecycle hook on the profile resolution, like adding an onMatch
(or any better name) event listener that profiles can use to perform custom initialization logic when a specific profile is matched.
Ideally, it would give access to the state actions, which would work well for this case, where we can decide to add an ad-hoc data view and clean it up once the profile is unmounted. I'm thinking about something like this:
export const createObservabilityRootProfileProvider = (
services: ProfileProviderServices
): RootProfileProvider => ({
profileId: 'observability-root-profile',
profile: {},
resolve: (params) => { /* ... */ },
onMatch: async ({actions}) => {
const dataView = createLogsDataView() // Holds logic to create data view based on settings, etc..
await actions.appendAdHocDataView(dataView)
// Clean-up function when the profile switches, similarly to use effects.
return async () => {
await actions.removeAdHocDataView(dataView.id)
}
}
});
Does it makes sense? It feels a good to have control on the awareness framework, as use cases might get more complex and require this.
I was thinking in the data view case it wouldn't set the default data view, just add a default one to the list, but I agree it makes sense to focus on this in isolation for now and align later if needed.
I'd be a bit hesitant to extend the framework in the suggested way since adding lifecycle hooks that can affect state imperatively could make it harder for Discover to control the initialization flow, which is important to get right. I feel like this case still fits well into the extension point model, and could be supported with something dedicated like a getAdHocDataViews
extension. That way consumers could provide an array of ad hoc data view specs, and Discover would be able to append them and remove them at the right times.
We could probably still accomplish this using the lifecycle hook by storing the array separately and only applying it at the right time in Discover, but I think it would add more complexity than is needed for this.
@davismcphee
I feel like this case still fits well into the extension point model, and could be supported with something dedicated like a getAdHocDataViews extension. That way consumers could provide an array of ad hoc data view specs, and Discover would be able to append them and remove them at the right times.
I don't have all the details to know in advance if that would work for any use case, but it would work for this specific one 👌
My proposal was mostly oriented to a more flexible API where the consumer has control over the lifecycle of the owned profile (by solution, I mean). This would give the chance to enhance the profile experience without the need to have a specific extension point each time, which requires more work and prioritization on the Discover side, blocking the work on the solutions' side.
I'm not saying the granular extension points are a bad choice, it surely prevents leaking too much control to the consumers. My point is mainly about having a more framework-oriented API to control some flows, where feature registration and clean-up are not respectively delegated to the consumer and Discover, which would also produce some undesired side-effects if the performed clean-up in Discover is not the intended one from the consumer.
If you feel the getAdHocDataViews
is the best way to go in terms of simplicity for this use case, I'm good with that 👌 Let's keep in mind the concern on the clean-up of the ad-hoc data views and flexibility of the framework, you surely have more context of what are the requirements from different solutions and what are the trade-offs 👍
As part of https://github.com/elastic/observability-dev/issues/3498 , a central setting is introduced that signifies where observability-related logs can be found. The intent is to replace the existing logs setting of the logs stream app and align across as many places as possible, so the user doesn't have to configure logs data access in multiple places.
One of these places is KQL-based Discover - Users have a flexible tool with data views to configure what data to show. To make it easy to always get back to all logs (in the sense of the central observability setting), there should be an implicit ad-hoc data view synced to the setting that's always available:
This gives the user an easy way to always look at all logs.