From fed9a193869739c2fae0bd7a49087fa1e216f5ac Mon Sep 17 00:00:00 2001 From: Abdul Wahab Zahid Date: Mon, 14 Oct 2024 11:26:24 +0200 Subject: [PATCH] Remember tab choice between logs explorer and discover (#194930) Closes #193321 ## Summary The PR adds the redirection point when "Discover" menu item is clicked on the sidenav in serverless (or solution nav on stateful). Based on what tab between "Discover" or "Logs Explorer" the user clicked recently, "Discover" will point to that app/tab. Previously, "Discover" would always point to "Logs Explorer" on serverless and to "Discover" on stateful. In order to implement this, a temporary app `last-used-logs-viewer` is registered in `observability-logs-explorer` plugin whose only job is to read the last stored value in local storage and perform the redirection. Doing the redirection from a temporary app should help prevent triggering unnecessary telemetry and history entries. And it should be fairly easy to undo once context aware redirection is in place. ~With this implementation, only the behavior of user clicking "Discover" on the sidenav and clicking the tabs is affected and any deeplinks from other apps or direct links should work as is.~ The tab choice will be updated even if the apps are visited via url. https://github.com/user-attachments/assets/8a0308db-9ddb-47b6-b1a5-8ed70662040d --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- packages/deeplinks/observability/constants.ts | 3 + .../deeplinks/observability/deep_links.ts | 3 + packages/deeplinks/observability/index.ts | 1 + .../locators/observability_logs_explorer.ts | 3 + .../logs_explorer_tabs.test.tsx | 26 +++- .../logs_explorer_tabs/logs_explorer_tabs.tsx | 25 +++- .../collectors/application_usage/schema.ts | 1 + src/plugins/telemetry/schema/oss_plugins.json | 131 ++++++++++++++++++ .../observability/public/navigation_tree.ts | 16 ++- .../applications/last_used_logs_viewer.tsx | 64 +++++++++ .../public/plugin.ts | 16 +++ .../observability_logs_explorer/tsconfig.json | 1 + .../public/navigation_tree.ts | 2 +- .../test_suites/observability/navigation.ts | 5 +- 14 files changed, 287 insertions(+), 10 deletions(-) create mode 100644 x-pack/plugins/observability_solution/observability_logs_explorer/public/applications/last_used_logs_viewer.tsx diff --git a/packages/deeplinks/observability/constants.ts b/packages/deeplinks/observability/constants.ts index 45868fa3a16b2..25642ba69613a 100644 --- a/packages/deeplinks/observability/constants.ts +++ b/packages/deeplinks/observability/constants.ts @@ -11,6 +11,9 @@ export const LOGS_APP_ID = 'logs'; export const OBSERVABILITY_LOGS_EXPLORER_APP_ID = 'observability-logs-explorer'; +// TODO: Remove the app once context-aware switching between discover and observability logs explorer is implemented +export const LAST_USED_LOGS_VIEWER_APP_ID = 'last-used-logs-viewer'; + export const OBSERVABILITY_OVERVIEW_APP_ID = 'observability-overview'; export const METRICS_APP_ID = 'metrics'; diff --git a/packages/deeplinks/observability/deep_links.ts b/packages/deeplinks/observability/deep_links.ts index 088b9c866c03d..1253b4e889fcf 100644 --- a/packages/deeplinks/observability/deep_links.ts +++ b/packages/deeplinks/observability/deep_links.ts @@ -12,6 +12,7 @@ import { LOGS_APP_ID, METRICS_APP_ID, OBSERVABILITY_LOGS_EXPLORER_APP_ID, + LAST_USED_LOGS_VIEWER_APP_ID, OBSERVABILITY_ONBOARDING_APP_ID, OBSERVABILITY_OVERVIEW_APP_ID, SYNTHETICS_APP_ID, @@ -24,6 +25,7 @@ import { type LogsApp = typeof LOGS_APP_ID; type ObservabilityLogsExplorerApp = typeof OBSERVABILITY_LOGS_EXPLORER_APP_ID; +type LastUsedLogsViewerApp = typeof LAST_USED_LOGS_VIEWER_APP_ID; type ObservabilityOverviewApp = typeof OBSERVABILITY_OVERVIEW_APP_ID; type MetricsApp = typeof METRICS_APP_ID; type ApmApp = typeof APM_APP_ID; @@ -38,6 +40,7 @@ type InventoryApp = typeof INVENTORY_APP_ID; export type AppId = | LogsApp | ObservabilityLogsExplorerApp + | LastUsedLogsViewerApp | ObservabilityOverviewApp | ObservabilityOnboardingApp | ApmApp diff --git a/packages/deeplinks/observability/index.ts b/packages/deeplinks/observability/index.ts index 7185a4cfbcd6f..8bedc43f5d6a8 100644 --- a/packages/deeplinks/observability/index.ts +++ b/packages/deeplinks/observability/index.ts @@ -10,6 +10,7 @@ export { LOGS_APP_ID, OBSERVABILITY_LOGS_EXPLORER_APP_ID, + LAST_USED_LOGS_VIEWER_APP_ID, OBSERVABILITY_ONBOARDING_APP_ID, OBSERVABILITY_OVERVIEW_APP_ID, AI_ASSISTANT_APP_ID, diff --git a/packages/deeplinks/observability/locators/observability_logs_explorer.ts b/packages/deeplinks/observability/locators/observability_logs_explorer.ts index 037acb7d946ad..ae4cbc12cec6d 100644 --- a/packages/deeplinks/observability/locators/observability_logs_explorer.ts +++ b/packages/deeplinks/observability/locators/observability_logs_explorer.ts @@ -49,3 +49,6 @@ export interface ObsLogsExplorerDataViewLocatorParams extends DatasetLocatorPara */ id: string; } + +// To store the last used logs viewer (either of discover or observability-logs-explorer) +export const OBS_LOGS_EXPLORER_LOGS_VIEWER_KEY = 'obs-logs-explorer:lastUsedViewer'; diff --git a/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.test.tsx b/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.test.tsx index 1782dc7ad6ac7..e353fe1971ec9 100644 --- a/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.test.tsx +++ b/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.test.tsx @@ -10,10 +10,21 @@ import userEvent from '@testing-library/user-event'; import { render, screen } from '@testing-library/react'; import React from 'react'; +import { DISCOVER_APP_ID } from '@kbn/deeplinks-analytics'; +import { + ALL_DATASETS_LOCATOR_ID, + OBSERVABILITY_LOGS_EXPLORER_APP_ID, +} from '@kbn/deeplinks-observability'; import { discoverServiceMock } from '../../__mocks__/services'; import { LogsExplorerTabs, LogsExplorerTabsProps } from './logs_explorer_tabs'; import { DISCOVER_APP_LOCATOR } from '../../../common'; -import { ALL_DATASETS_LOCATOR_ID } from '@kbn/deeplinks-observability'; + +const mockSetLastUsedViewer = jest.fn(); +jest.mock('react-use/lib/useLocalStorage', () => { + return jest.fn((key: string, _initialValue: string) => { + return [undefined, mockSetLastUsedViewer]; // Always use undefined as the initial value + }); +}); const createMockLocator = (id: string) => ({ navigate: jest.fn(), @@ -46,11 +57,12 @@ describe('LogsExplorerTabs', () => { }, } as unknown as typeof discoverServiceMock; - render(); + const { unmount } = render(); return { mockDiscoverLocator, mockLogsExplorerLocator, + unmount, }; }; @@ -86,4 +98,14 @@ describe('LogsExplorerTabs', () => { await userEvent.click(getDiscoverTab()); expect(mockDiscoverLocator.navigate).toHaveBeenCalledWith({}); }); + + it('should update the last used viewer in local storage for selectedTab', async () => { + const { unmount } = renderTabs('discover'); + expect(mockSetLastUsedViewer).toHaveBeenCalledWith(DISCOVER_APP_ID); + + unmount(); + mockSetLastUsedViewer.mockClear(); + renderTabs('logs-explorer'); + expect(mockSetLastUsedViewer).toHaveBeenCalledWith(OBSERVABILITY_LOGS_EXPLORER_APP_ID); + }); }); diff --git a/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.tsx b/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.tsx index 1eec001464d2a..c7082c21344ac 100644 --- a/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.tsx +++ b/src/plugins/discover/public/components/logs_explorer_tabs/logs_explorer_tabs.tsx @@ -8,9 +8,16 @@ */ import { EuiTab, EuiTabs, useEuiTheme } from '@elastic/eui'; -import { AllDatasetsLocatorParams, ALL_DATASETS_LOCATOR_ID } from '@kbn/deeplinks-observability'; +import { DISCOVER_APP_ID } from '@kbn/deeplinks-analytics'; +import { + AllDatasetsLocatorParams, + ALL_DATASETS_LOCATOR_ID, + OBS_LOGS_EXPLORER_LOGS_VIEWER_KEY, + OBSERVABILITY_LOGS_EXPLORER_APP_ID, +} from '@kbn/deeplinks-observability'; import { i18n } from '@kbn/i18n'; -import React, { MouseEvent } from 'react'; +import React, { MouseEvent, useEffect } from 'react'; +import useLocalStorage from 'react-use/lib/useLocalStorage'; import { DiscoverAppLocatorParams, DISCOVER_APP_LOCATOR } from '../../../common'; import type { DiscoverServices } from '../../build_services'; @@ -29,6 +36,10 @@ export const LogsExplorerTabs = ({ services, selectedTab }: LogsExplorerTabsProp const discoverUrl = discoverLocator?.getRedirectUrl(emptyParams); const logsExplorerUrl = logsExplorerLocator?.getRedirectUrl(emptyParams); + const [lastUsedViewer, setLastUsedViewer] = useLocalStorage< + typeof DISCOVER_APP_ID | typeof OBSERVABILITY_LOGS_EXPLORER_APP_ID + >(OBS_LOGS_EXPLORER_LOGS_VIEWER_KEY, OBSERVABILITY_LOGS_EXPLORER_APP_ID); + const navigateToDiscover = createNavigateHandler(() => { if (selectedTab !== 'discover') { discoverLocator?.navigate(emptyParams); @@ -41,6 +52,16 @@ export const LogsExplorerTabs = ({ services, selectedTab }: LogsExplorerTabsProp } }); + useEffect(() => { + if (selectedTab === 'discover' && lastUsedViewer !== DISCOVER_APP_ID) { + setLastUsedViewer(DISCOVER_APP_ID); + } + + if (selectedTab === 'logs-explorer' && lastUsedViewer !== OBSERVABILITY_LOGS_EXPLORER_APP_ID) { + setLastUsedViewer(OBSERVABILITY_LOGS_EXPLORER_APP_ID); + } + }, [setLastUsedViewer, lastUsedViewer, selectedTab]); + return ( { + ReactDOM.render( + + + , + appParams.element + ); + + return () => { + ReactDOM.unmountComponentAtNode(appParams.element); + }; +}; + +export const LastUsedLogsViewerRedirect = ({ core }: { core: CoreStart }) => { + const location = useLocation(); + const path = `${location.pathname}${location.search}`; + const [lastUsedLogsViewApp] = useLocalStorage< + typeof DISCOVER_APP_ID | typeof OBSERVABILITY_LOGS_EXPLORER_APP_ID + >(OBS_LOGS_EXPLORER_LOGS_VIEWER_KEY, OBSERVABILITY_LOGS_EXPLORER_APP_ID); + + if ( + lastUsedLogsViewApp && + lastUsedLogsViewApp !== DISCOVER_APP_ID && + lastUsedLogsViewApp !== OBSERVABILITY_LOGS_EXPLORER_APP_ID + ) { + throw new Error( + `Invalid last used logs viewer app: "${lastUsedLogsViewApp}". Allowed values are "${DISCOVER_APP_ID}" and "${OBSERVABILITY_LOGS_EXPLORER_APP_ID}"` + ); + } + + useEffect(() => { + if (lastUsedLogsViewApp === DISCOVER_APP_ID) { + core.application.navigateToApp(DISCOVER_APP_ID, { replace: true, path }); + } + + if (lastUsedLogsViewApp === OBSERVABILITY_LOGS_EXPLORER_APP_ID) { + core.application.navigateToApp(OBSERVABILITY_LOGS_EXPLORER_APP_ID, { replace: true, path }); + } + }, [core, path, lastUsedLogsViewApp]); + + return <>; +}; diff --git a/x-pack/plugins/observability_solution/observability_logs_explorer/public/plugin.ts b/x-pack/plugins/observability_solution/observability_logs_explorer/public/plugin.ts index 798a03da0ebdf..2e6ab0aeeaa0f 100644 --- a/x-pack/plugins/observability_solution/observability_logs_explorer/public/plugin.ts +++ b/x-pack/plugins/observability_solution/observability_logs_explorer/public/plugin.ts @@ -97,6 +97,22 @@ export class ObservabilityLogsExplorerPlugin }, }); + // App used solely to redirect to either "/app/observability-logs-explorer" or "/app/discover" + // based on the last used app value in localStorage + core.application.register({ + id: 'last-used-logs-viewer', + title: logsExplorerAppTitle, + visibleIn: [], + mount: async (appMountParams: AppMountParameters) => { + const [coreStart] = await core.getStartServices(); + const { renderLastUsedLogsViewerRedirect } = await import( + './applications/last_used_logs_viewer' + ); + + return renderLastUsedLogsViewerRedirect(coreStart, appMountParams); + }, + }); + core.analytics.registerEventType(DATA_RECEIVED_TELEMETRY_EVENT); // Register Locators diff --git a/x-pack/plugins/observability_solution/observability_logs_explorer/tsconfig.json b/x-pack/plugins/observability_solution/observability_logs_explorer/tsconfig.json index 6ea751aaed3de..be2b3c9efdff6 100644 --- a/x-pack/plugins/observability_solution/observability_logs_explorer/tsconfig.json +++ b/x-pack/plugins/observability_solution/observability_logs_explorer/tsconfig.json @@ -50,6 +50,7 @@ "@kbn/core-analytics-browser", "@kbn/react-hooks", "@kbn/logs-data-access-plugin", + "@kbn/deeplinks-analytics", ], "exclude": [ "target/**/*" diff --git a/x-pack/plugins/serverless_observability/public/navigation_tree.ts b/x-pack/plugins/serverless_observability/public/navigation_tree.ts index e3c61ec0b29e3..5df900ee46812 100644 --- a/x-pack/plugins/serverless_observability/public/navigation_tree.ts +++ b/x-pack/plugins/serverless_observability/public/navigation_tree.ts @@ -24,7 +24,7 @@ export const navigationTree: NavigationTreeDefinition = { title: i18n.translate('xpack.serverlessObservability.nav.discover', { defaultMessage: 'Discover', }), - link: 'observability-logs-explorer', + link: 'last-used-logs-viewer', // avoid duplicate "Discover" breadcrumbs breadcrumbStatus: 'hidden', renderAs: 'item', diff --git a/x-pack/test_serverless/functional/test_suites/observability/navigation.ts b/x-pack/test_serverless/functional/test_suites/observability/navigation.ts index ce6b68cd618a1..bcd9c031714f7 100644 --- a/x-pack/test_serverless/functional/test_suites/observability/navigation.ts +++ b/x-pack/test_serverless/functional/test_suites/observability/navigation.ts @@ -35,9 +35,10 @@ export default function ({ getPageObject, getService }: FtrProviderContext) { await svlCommonNavigation.sidenav.expectSectionClosed('project_settings_project_nav'); // navigate to the logs explorer tab by default - await svlCommonNavigation.sidenav.clickLink({ deepLinkId: 'observability-logs-explorer' }); + // 'last-used-logs-viewer' is wrapper app to handle the navigation between logs explorer and discover + await svlCommonNavigation.sidenav.clickLink({ deepLinkId: 'last-used-logs-viewer' }); await svlCommonNavigation.sidenav.expectLinkActive({ - deepLinkId: 'observability-logs-explorer', + deepLinkId: 'last-used-logs-viewer', }); await svlCommonNavigation.breadcrumbs.expectBreadcrumbExists({ deepLinkId: 'observability-logs-explorer',