[Discover][Observability] Register Logs Explorer top navigation

gbamparop commented 2 months ago

📓 Summary

🛑 Postponed until https://github.com/elastic/kibana/issues/182229

The top navigation in Logs Explorer includes links to the Observability onboarding guides as well as actions to create rules and SLOs while maintaining context. This functionality will be moved to Discover for Observability projects.

✔ Acceptance Criteria

The top navigation bar should be registered as a feature using the DiscoverFeaturesRegistry provided by the discover-shared plugin
The top navigation bar in Discover will contain a link to add data through the Observability onboarding guides in Observability projects
The top navigation bar in Discover will contain actions to create custom threshold rules and SLOs in Observability projects

❓ Open questions

Will the link to the feedback form be part of the new bar?

elasticmachine commented 2 months ago

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

tonyghiani commented 2 months ago

Will the link to the feedback form be part of the new bar?

@gbamparop My take is that we should keep it, aligning to a single actions design as the one we used for the project view since the beginning:

This also means moving the beta badge (for the time we will still keep it), but the effort doesn't change since we already have this template in place for the project view top bar. Wdyt?

gbamparop commented 2 months ago

@ninoslavmiskovic @alex-fedotyev what are your thoughts about having a different form per project type? The goal in this case would be to get feedback for the logs flavour.

ninoslavmiskovic commented 2 months ago

@gbamparop Do you mean per data type/source e.g. when users are exploring logs then they get one survey, and when they are exploring security events in Discover, then they get another ?

This might be too granular. I think we should have one form for "One discover" and then we can have a question.. What kind of data are you exploring.. Logs, Metrics, Traces etc...

ninoslavmiskovic commented 2 months ago

Be aware that serverless has an overall "Give feedback"

This means that there are 2 x Give feedback..

We need to consider a better UI for this.

tonyghiani commented 2 months ago

@gbamparop I agree having two feedback CTA might be confusing, shall we remove the current link we have for Logs Explorer in favour of a One Discover link once the MVP is ready as @ninoslavmiskovic suggested?

tonyghiani commented 2 months ago

Another point to discuss is whether we want to keep the link "Open to Discover". With the solution navbar in place and the tabs for "Discover" and "Logs Explorer" (which we'll soon remove), it seems redundant to have this link in place, although it's true it has a different function than the Discover tab navigation, since this link open the current selection and filters in the Discover tab.

I don't think there are many more benefits now in bringing the user in vanilla Discover switching tab with this link, given also that we'll soon have a single experience for all of this. My 2 cents is that we can remove it, wdyt? @gbamparop @ruflin

ruflin commented 2 months ago

I suggest to remove the Give Feedback block all together as it was used rarely so far.

If a user clicks on Discover tab now, is the behaviour identical to the "Open in Discover"? If yes, agree on removing it.

flash1293 commented 2 months ago

I agree that having two buttons up there is a little weird, but the behavior of keeping the current filters is pretty nice.

Also we don't know whether we have the two tabs yet in all cases (on stateful it relies on the new navigation). Is it a lot of work to keep the current "Open in Discover" logic for now? If not, I think it's OK just retaining it and removing it with the final merge of the apps.

tonyghiani commented 2 months ago

If a user clicks on Discover tab now, is the behaviour identical to the "Open in Discover"? If yes, agree on removing it.

@ruflin

Click on the tab => Load discover with default data view and no filters
Click on the "Open in Discover" link => Load discover with current Logs Explorer search (dataset + filter).

Different behaviours, although it's not specified in any way that "Open in Discover" moves to discover with the current search from Logs Explorer. I find it quite confusing now that they appear on the same topnav.

gbamparop commented 1 month ago

This might be too granular. I think we should have one form for "One discover" and then we can have a question.. What kind of data are you exploring.. Logs, Metrics, Traces etc...

+1 if we can have one form. Good point for serverless, we could also consider removing it as Nicolas suggested, we haven't received many responses yet but we have recently changed the email validation settings.

removing it with the final merge of the apps.

This sounds good to me.

ruflin commented 1 month ago

I would bring the behaviour from "Open in Discover" to "Click on Tab" if possible that users keep state and remove the button. Is this complex to do? Otherwise, lets keep it for now temporary until the two apps have merged.

tonyghiani commented 1 month ago

I would bring the behaviour from "Open in Discover" to "Click on Tab" if possible that users keep state and remove the button. Is this complex to do? Otherwise, lets keep it for now temporary until the two apps have merged.

This makes a lot of sense, although @flash1293 made a good point about still having the classic navigation bar without the tabs, so let's keep it temporary until the merge 👌

tonyghiani commented 1 month ago

I now spent some time on the codebase for this task, here are some considerations that made me consider postponing this work until we remove the tabs for some limits it currently has:

1. There are currently 4 different scenarios for displaying the top nav. The current implementation between for tabs and navbar menu treats it as a unique topbar, so we cannot act and factor out the specific navbar menu without affecting the tabs' implementation.

	Tabs	Navbar menu
ProjectNav - Search - Discover	❌	Discover
ProjectNav - Obs - Discover	✅	Discover
ProjectNav - Obs - Logs Explorer	✅	Logs Explorer
ClassicNav - Obs - Logs Explorer	❌	Logs Explorer

This requires quite a lot of work to split the tabs and the navbar menu customization.

2. The logic to keep the filters applied on the "Open in Discover" menu item strongly depends on the data source selection and the Logs Explorer state logic, trying to register it as an external menu item in Discover would over-engineer the implementation, which I believe is not worth to mess it up now since it does its work well and will soon be removed.

3. With the context awareness in place, this observability navbar menu will become one of the available menus directly moved into Discover, it doesn't require the overhead of registering this externally since we'll be able to know in which context it should be displayed directly in Discover.

As soon as we remove the tabs, we'll need to touch this part anyway to resolve the Obs logs navbar menu, but it will be much easier since it will just result in a cut/paste work from Logs Explorer to Discover for the remaining menu items.

Also, all the above-described work wouldn't benefit the user experience if we do it now as there are no perceived changes on the UX, but it would just add more noise we have to clean up when removing the tabs.

My take is that we should pause this work until we move forward with the next steps on the One Discover context resolution, please let me know if there is any concern around this.

ninoslavmiskovic commented 1 month ago

I think it makes sense to not invest in this until we are merging Log Explorer and Discover as part of "One Discover", and then revisit it again.

elastic / kibana