[Search Session] Fix dangling search sessions (#102927) (#104281)

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddableeditorstate.md

@@ -18,5 +18,6 @@ export interface EmbeddableEditorState
 |  --- | --- | --- |
 |  [embeddableId](./kibana-plugin-plugins-embeddable-public.embeddableeditorstate.embeddableid.md) | <code>string</code> |  |
 |  [originatingApp](./kibana-plugin-plugins-embeddable-public.embeddableeditorstate.originatingapp.md) | <code>string</code> |  |
+|  [searchSessionId](./kibana-plugin-plugins-embeddable-public.embeddableeditorstate.searchsessionid.md) | <code>string</code> | Pass current search session id when navigating to an editor, Editors could use it continue previous search session |
 |  [valueInput](./kibana-plugin-plugins-embeddable-public.embeddableeditorstate.valueinput.md) | <code>EmbeddableInput</code> |  |
 

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddableeditorstate.searchsessionid.md

@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [EmbeddableEditorState](./kibana-plugin-plugins-embeddable-public.embeddableeditorstate.md) &gt; [searchSessionId](./kibana-plugin-plugins-embeddable-public.embeddableeditorstate.searchsessionid.md)
+
+## EmbeddableEditorState.searchSessionId property
+
+Pass current search session id when navigating to an editor, Editors could use it continue previous search session
+
+<b>Signature:</b>
+
+```typescript
+searchSessionId?: string;
+```

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablepackagestate.md

@@ -18,5 +18,6 @@ export interface EmbeddablePackageState
 |  --- | --- | --- |
 |  [embeddableId](./kibana-plugin-plugins-embeddable-public.embeddablepackagestate.embeddableid.md) | <code>string</code> |  |
 |  [input](./kibana-plugin-plugins-embeddable-public.embeddablepackagestate.input.md) | <code>Optional&lt;EmbeddableInput, 'id'&gt; &#124; Optional&lt;SavedObjectEmbeddableInput, 'id'&gt;</code> |  |
+|  [searchSessionId](./kibana-plugin-plugins-embeddable-public.embeddablepackagestate.searchsessionid.md) | <code>string</code> | Pass current search session id when navigating to an editor, Editors could use it continue previous search session |
 |  [type](./kibana-plugin-plugins-embeddable-public.embeddablepackagestate.type.md) | <code>string</code> |  |
 

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablepackagestate.searchsessionid.md

@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [EmbeddablePackageState](./kibana-plugin-plugins-embeddable-public.embeddablepackagestate.md) &gt; [searchSessionId](./kibana-plugin-plugins-embeddable-public.embeddablepackagestate.searchsessionid.md)
+
+## EmbeddablePackageState.searchSessionId property
+
+Pass current search session id when navigating to an editor, Editors could use it continue previous search session
+
+<b>Signature:</b>
+
+```typescript
+searchSessionId?: string;
+```

src/plugins/dashboard/public/application/dashboard_app.tsx

@@ -40,6 +40,7 @@ export function DashboardApp({
     embeddable,
     onAppLeave,
     uiSettings,
+    data,
   } = useKibana<DashboardAppServices>().services;
 
   const kbnUrlStateStorage = useMemo(
@@ -98,6 +99,13 @@ export function DashboardApp({
     ]);
   }, [chrome, dashboardState.title, dashboardState.viewMode, redirectTo, savedDashboardId]);
 
+  // clear search session when leaving dashboard route
+  useEffect(() => {
+    return () => {
+      data.search.session.clear();
+    };
+  }, [data.search.session]);
+
   return (
     <>
       {isCompleteDashboardAppState(dashboardAppState) && (

src/plugins/dashboard/public/application/dashboard_router.tsx

@@ -260,6 +260,7 @@ export async function mountApp({
   }
   render(app, element);
   return () => {
+    dataStart.search.session.clear();
     unlistenParentHistory();
     unmountComponentAtNode(element);
     appUnMounted();

src/plugins/dashboard/public/application/lib/build_dashboard_container.ts

@@ -64,6 +64,11 @@ export const buildDashboardContainer = async ({
     getLatestDashboardState,
     canStoreSearchSession: dashboardCapabilities.storeSearchSession,
   });
+
+  if (incomingEmbeddable?.searchSessionId) {
+    session.continue(incomingEmbeddable?.searchSessionId);
+  }
+
   const searchSessionIdFromURL = getSearchSessionIdFromURL(history);
   if (searchSessionIdFromURL) {
     session.restore(searchSessionIdFromURL);

src/plugins/dashboard/public/application/listing/dashboard_listing.tsx

@@ -87,11 +87,6 @@ export const DashboardListing = ({
     };
   }, [title, savedObjectsClient, redirectTo, data.query, kbnUrlStateStorage]);
 
-  // clear dangling session because they are not required here
-  useEffect(() => {
-    data.search.session.clear();
-  }, [data.search.session]);
-
   const hideWriteControls = dashboardCapabilities.hideWriteControls;
   const listingLimit = savedObjects.settings.getListingLimit();
   const defaultFilter = title ? `"${title}"` : '';

src/plugins/dashboard/public/application/top_nav/dashboard_top_nav.tsx

@@ -204,10 +204,11 @@ export function DashboardTopNav({
         path,
         state: {
           originatingApp: DashboardConstants.DASHBOARDS_ID,
+          searchSessionId: data.search.session.getSessionId(),
         },
       });
     },
-    [trackUiMetric, stateTransferService]
+    [stateTransferService, data.search.session, trackUiMetric]
   );
 
   const clearAddPanel = useCallback(() => {

src/plugins/data/public/public.api.md

@@ -2789,7 +2789,7 @@ export interface WaitUntilNextSessionCompletesOptions {
 // src/plugins/data/public/index.ts:433:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
 // src/plugins/data/public/index.ts:436:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
 // src/plugins/data/public/query/state_sync/connect_to_query_state.ts:34:5 - (ae-forgotten-export) The symbol "FilterStateStore" needs to be exported by the entry point index.d.ts
-// src/plugins/data/public/search/session/session_service.ts:56:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts
+// src/plugins/data/public/search/session/session_service.ts:62:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts
 
 // (No @packageDocumentation comment for this package)
 

src/plugins/data/public/search/session/mocks.ts

@@ -47,5 +47,6 @@ export function getSessionServiceMock(): jest.Mocked<ISessionService> {
     isSessionStorageReady: jest.fn(() => true),
     getSearchSessionIndicatorUiConfig: jest.fn(() => ({ isDisabled: () => ({ disabled: false }) })),
     hasAccess: jest.fn(() => true),
+    continue: jest.fn(),
   };
 }

src/plugins/data/public/search/session/session_service.test.ts

@@ -98,12 +98,12 @@ describe('Session service', () => {
       expect(nowProvider.reset).toHaveBeenCalled();
     });
 
-    it("Can clear other apps' session", async () => {
+    it("Can't clear other apps' session", async () => {
       sessionService.start();
       expect(sessionService.getSessionId()).not.toBeUndefined();
       currentAppId$.next('change');
       sessionService.clear();
-      expect(sessionService.getSessionId()).toBeUndefined();
+      expect(sessionService.getSessionId()).not.toBeUndefined();
     });
 
     it("Can start a new session in case there is other apps' stale session", async () => {
@@ -161,6 +161,72 @@ describe('Session service', () => {
     });
   });
 
+  it('Can continue previous session from another app', async () => {
+    sessionService.start();
+    const sessionId = sessionService.getSessionId();
+
+    sessionService.clear();
+    currentAppId$.next('change');
+    sessionService.continue(sessionId!);
+
+    expect(sessionService.getSessionId()).toBe(sessionId);
+  });
+
+  it('Calling clear() more than once still allows previous session from another app to continue', async () => {
+    sessionService.start();
+    const sessionId = sessionService.getSessionId();
+
+    sessionService.clear();
+    sessionService.clear();
+
+    currentAppId$.next('change');
+    sessionService.continue(sessionId!);
+
+    expect(sessionService.getSessionId()).toBe(sessionId);
+  });
+
+  it('Continue drops storage configuration', () => {
+    sessionService.start();
+    const sessionId = sessionService.getSessionId();
+
+    sessionService.enableStorage({
+      getName: async () => 'Name',
+      getUrlGeneratorData: async () => ({
+        urlGeneratorId: 'id',
+        initialState: {},
+        restoreState: {},
+      }),
+    });
+
+    expect(sessionService.isSessionStorageReady()).toBe(true);
+
+    sessionService.clear();
+
+    sessionService.continue(sessionId!);
+
+    expect(sessionService.isSessionStorageReady()).toBe(false);
+  });
+
+  // it might be that search requests finish after the session is cleared and before it was continued,
+  // to avoid "infinite loading" state after we continue the session we have to drop pending searches
+  it('Continue drops client side loading state', async () => {
+    const sessionId = sessionService.start();
+
+    sessionService.trackSearch({ abort: () => {} });
+    expect(state$.getValue()).toBe(SearchSessionState.Loading);
+
+    sessionService.clear(); // even allow to call clear multiple times
+
+    expect(state$.getValue()).toBe(SearchSessionState.None);
+
+    sessionService.continue(sessionId!);
+    expect(sessionService.getSessionId()).toBe(sessionId);
+
+    // the original search was never `untracked`,
+    // but we still consider this a completed session until new search fire
+    expect(state$.getValue()).toBe(SearchSessionState.Completed);
+  });
+
   test('getSearchOptions infers isRestore & isStored from state', async () => {
     const sessionId = sessionService.start();
     const someOtherId = 'some-other-id';

src/plugins/data/public/search/session/session_service.ts

@@ -20,6 +20,7 @@ import { ConfigSchema } from '../../../config';
 import {
   createSessionStateContainer,
   SearchSessionState,
+  SessionStateInternal,
   SessionMeta,
   SessionStateContainer,
 } from './search_session_state';
@@ -35,6 +36,11 @@ export interface TrackSearchDescriptor {
   abort: () => void;
 }
 
+/**
+ * Represents a search session state in {@link SessionService} in any given moment of time
+ */
+export type SessionSnapshot = SessionStateInternal<TrackSearchDescriptor>;
+
 /**
  * Provide info about current search session to be stored in the Search Session saved object
  */
@@ -88,6 +94,13 @@ export class SessionService {
 
   private toastService?: ToastService;
 
+  /**
+   * Holds snapshot of last cleared session so that it can be continued
+   * Can be used to re-use a session between apps
+   * @private
+   */
+  private lastSessionSnapshot?: SessionSnapshot;
+
   constructor(
     initializerContext: PluginInitializerContext<ConfigSchema>,
     getStartServices: StartServicesAccessor,
@@ -128,6 +141,21 @@ export class SessionService {
       this.subscription.add(
         coreStart.application.currentAppId$.subscribe((newAppName) => {
           this.currentApp = newAppName;
+          if (!this.getSessionId()) return;
+
+          // Apps required to clean up their sessions before unmounting
+          // Make sure that apps don't leave sessions open by throwing an error in DEV mode
+          const message = `Application '${
+            this.state.get().appName
+          }' had an open session while navigating`;
+          if (initializerContext.env.mode.dev) {
+            coreStart.fatalErrors.add(message);
+          } else {
+            // this should never happen in prod because should be caught in dev mode
+            // in case this happen we don't want to throw fatal error, as most likely possible bugs are not that critical
+            // eslint-disable-next-line no-console
+            console.warn(message);
+          }
         })
       );
     });
@@ -158,6 +186,7 @@ export class SessionService {
   public destroy() {
     this.subscription.unsubscribe();
     this.clear();
+    this.lastSessionSnapshot = undefined;
   }
 
   /**
@@ -198,7 +227,9 @@ export class SessionService {
    */
   public start() {
     if (!this.currentApp) throw new Error('this.currentApp is missing');
+
     this.state.transitions.start({ appName: this.currentApp });
+
     return this.getSessionId()!;
   }
 
@@ -211,10 +242,52 @@ export class SessionService {
     this.refreshSearchSessionSavedObject();
   }
 
+  /**
+   * Continue previous search session
+   * Can be used to share a running search session between different apps, so they can reuse search cache
+   *
+   * This is different from {@link restore} as it reuses search session state and search results held in client memory instead of restoring search results from elasticsearch
+   * @param sessionId
+   */
+  public continue(sessionId: string) {
+    if (this.lastSessionSnapshot?.sessionId === sessionId) {
+      this.state.set({
+        ...this.lastSessionSnapshot,
+        // have to change a name, so that current app can cancel a session that it continues
+        appName: this.currentApp,
+        // also have to drop all pending searches which are used to derive client side state of search session indicator,
+        // if we weren't dropping this searches, then we would get into "infinite loading" state when continuing a session that was cleared with pending searches
+        // possible solution to this problem is to refactor session service to support multiple sessions
+        pendingSearches: [],
+      });
+      this.lastSessionSnapshot = undefined;
+    } else {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `Continue search session: last known search session id: "${this.lastSessionSnapshot?.sessionId}", but received ${sessionId}`
+      );
+    }
+  }
+
   /**
    * Cleans up current state
    */
   public clear() {
+    // make sure apps can't clear other apps' sessions
+    const currentSessionApp = this.state.get().appName;
+    if (currentSessionApp && currentSessionApp !== this.currentApp) {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `Skip clearing session "${this.getSessionId()}" because it belongs to a different app. current: "${
+          this.currentApp
+        }", owner: "${currentSessionApp}"`
+      );
+      return;
+    }
+
+    if (this.getSessionId()) {
+      this.lastSessionSnapshot = this.state.get();
+    }
     this.state.transitions.clear();
     this.searchSessionInfoProvider = undefined;
     this.searchSessionIndicatorUiConfig = undefined;

src/plugins/embeddable/public/lib/actions/edit_panel_action.ts

@@ -111,6 +111,7 @@ export class EditPanelAction implements Action<ActionContext> {
           originatingApp: this.currentAppId,
           valueInput: byValueMode ? this.getExplicitInput({ embeddable }) : undefined,
           embeddableId: embeddable.id,
+          searchSessionId: embeddable.getInput().searchSessionId,
         };
         return { app, path, state };
       }

src/plugins/embeddable/public/lib/state_transfer/types.ts

@@ -19,6 +19,12 @@ export interface EmbeddableEditorState {
   originatingApp: string;
   embeddableId?: string;
   valueInput?: EmbeddableInput;
+
+  /**
+   * Pass current search session id when navigating to an editor,
+   * Editors could use it continue previous search session
+   */
+  searchSessionId?: string;
 }
 
 export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState {
@@ -35,6 +41,12 @@ export interface EmbeddablePackageState {
   type: string;
   input: Optional<EmbeddableInput, 'id'> | Optional<SavedObjectEmbeddableInput, 'id'>;
   embeddableId?: string;
+
+  /**
+   * Pass current search session id when navigating to an editor,
+   * Editors could use it continue previous search session
+   */
+  searchSessionId?: string;
 }
 
 export function isEmbeddablePackageState(state: unknown): state is EmbeddablePackageState {

src/plugins/embeddable/public/public.api.md

@@ -365,6 +365,7 @@ export interface EmbeddableEditorState {
     embeddableId?: string;
     // (undocumented)
     originatingApp: string;
+    searchSessionId?: string;
     // (undocumented)
     valueInput?: EmbeddableInput;
 }
@@ -467,6 +468,7 @@ export interface EmbeddablePackageState {
     embeddableId?: string;
     // (undocumented)
     input: Optional<EmbeddableInput, 'id'> | Optional<SavedObjectEmbeddableInput, 'id'>;
+    searchSessionId?: string;
     // (undocumented)
     type: string;
 }

x-pack/plugins/lens/public/app_plugin/mounter.tsx

@@ -161,6 +161,7 @@ export async function mountApp(
           embeddableId: isCopied ? undefined : embeddableEditorIncomingState.embeddableId,
           type: LENS_EMBEDDABLE_TYPE,
           input,
+          searchSessionId: data.search.session.getSessionId(),
         },
       });
     } else {
@@ -178,6 +179,10 @@ export async function mountApp(
   if (!initialContext) {
     data.query.filterManager.setAppFilters([]);
   }
+
+  if (embeddableEditorIncomingState?.searchSessionId) {
+    data.search.session.continue(embeddableEditorIncomingState.searchSessionId);
+  }
   const { datasourceMap, visualizationMap } = instance;
 
   const initialDatasourceId = getInitialDatasourceId(datasourceMap);
@@ -298,6 +303,7 @@ export async function mountApp(
     params.element
   );
   return () => {
+    data.search.session.clear();
     unmountComponentAtNode(params.element);
     unlistenParentHistory();
     lensStore.dispatch(navigateAway());

x-pack/test/search_sessions_integration/config.ts

@@ -22,6 +22,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
 
     testFiles: [
       resolve(__dirname, './tests/apps/dashboard/async_search'),
+      resolve(__dirname, './tests/apps/dashboard/session_sharing'),
       resolve(__dirname, './tests/apps/discover'),
       resolve(__dirname, './tests/apps/lens'),
       resolve(__dirname, './tests/apps/management/search_sessions'),

x-pack/test/search_sessions_integration/tests/apps/dashboard/session_sharing/index.ts

@@ -0,0 +1,26 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { FtrProviderContext } from '../../../../ftr_provider_context';
+
+export default function ({ loadTestFile, getService, getPageObjects }: FtrProviderContext) {
+  const kibanaServer = getService('kibanaServer');
+  const esArchiver = getService('esArchiver');
+  const PageObjects = getPageObjects(['common']);
+
+  describe('Search session sharing', function () {
+    this.tags('ciGroup3');
+
+    before(async () => {
+      await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/logstash_functional');
+      await kibanaServer.uiSettings.replace({ defaultIndex: 'logstash-*' });
+      await PageObjects.common.navigateToApp('dashboard');
+    });
+
+    loadTestFile(require.resolve('./lens'));
+  });
+}

x-pack/test/search_sessions_integration/tests/apps/dashboard/session_sharing/lens.ts

@@ -0,0 +1,71 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import expect from '@kbn/expect';
+import { FtrProviderContext } from '../../../../ftr_provider_context';
+
+export default function ({ getService, getPageObjects }: FtrProviderContext) {
+  const esArchiver = getService('esArchiver');
+  const dashboardPanelActions = getService('dashboardPanelActions');
+  const dashboardAddPanel = getService('dashboardAddPanel');
+  const find = getService('find');
+  const testSubjects = getService('testSubjects');
+  const PageObjects = getPageObjects(['header', 'common', 'dashboard', 'timePicker', 'lens']);
+
+  // Dashboard shares a search session with lens when navigating to and from by value lens to hit search cache
+  // https://github.com/elastic/kibana/issues/99310
+  describe('Search session sharing with lens', () => {
+    before(async () => {
+      await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/lens/basic');
+      await PageObjects.common.navigateToApp('dashboard');
+      await PageObjects.dashboard.preserveCrossAppState();
+    });
+
+    after(async () => {
+      await esArchiver.unload('x-pack/test/functional/es_archives/lens/basic');
+    });
+
+    // NOTE: This test doesn't check if the cache was actually hit, but just checks if the same search session id is used
+    // so it doesn't give the 100% confidence that cache-hit improvement works https://github.com/elastic/kibana/issues/99310
+    // but if it fails, we for sure know it doesn't work
+    it("should share search session with by value lens and don't share with by reference", async () => {
+      // Add a by ref lens panel to a new dashboard
+      const lensTitle = 'Artistpreviouslyknownaslens';
+      await PageObjects.dashboard.clickNewDashboard();
+      await dashboardAddPanel.clickOpenAddPanel();
+      await dashboardAddPanel.filterEmbeddableNames(lensTitle);
+      await find.clickByButtonText(lensTitle);
+      await dashboardAddPanel.closeAddPanel();
+      await PageObjects.lens.goToTimeRange();
+      await PageObjects.dashboard.waitForRenderComplete();
+
+      // Navigating to lens and back should create a new session
+      const byRefSessionId = await dashboardPanelActions.getSearchSessionIdByTitle(lensTitle);
+      await dashboardPanelActions.openContextMenu();
+      await dashboardPanelActions.clickEdit();
+      await PageObjects.lens.saveAndReturn();
+      await PageObjects.dashboard.waitForRenderComplete();
+      const newByRefSessionId = await dashboardPanelActions.getSearchSessionIdByTitle(lensTitle);
+
+      expect(byRefSessionId).not.to.eql(newByRefSessionId);
+
+      // Convert to by-value
+      const byRefPanel = await testSubjects.find('embeddablePanelHeading-' + lensTitle);
+      await dashboardPanelActions.unlinkFromLibary(byRefPanel);
+      await PageObjects.dashboard.waitForRenderComplete();
+      const byValueSessionId = await dashboardPanelActions.getSearchSessionIdByTitle(lensTitle);
+
+      // Navigating to lens and back should keep the session
+      await dashboardPanelActions.openContextMenu();
+      await dashboardPanelActions.clickEdit();
+      await PageObjects.lens.saveAndReturn();
+      await PageObjects.dashboard.waitForRenderComplete();
+      const newByValueSessionId = await dashboardPanelActions.getSearchSessionIdByTitle(lensTitle);
+      expect(byValueSessionId).to.eql(newByValueSessionId);
+    });
+  });
+}