[Discover] Refactor Unified Histogram refetching logic for text based languages (#159438)

## Summary

This PR refactors and simplifies the Unified Histogram refetching logic
for text based languages in Discover to minimize unnecessary refetches
while ensuring the total hits and Lens visualization stay in sync with
the results. It also introduces a new set of functional tests to track
the number of search requests sent from Discover under various scenarios
to help reduce the chance of regressions in data fetching logic.

Flaky test runner x 100:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2375.

Note: the flaky test runner has a couple of failures, but they all have
the error `expected testSubject(globalLoadingIndicator-hidden) to
exist`, which is also the same error as these three recently skipped
tests related to text based language mode:
- #159083
- #159167
- #159194

This suggests the failures aren't related to this PR, and I don't think
they should prevent us from merging this. But it also means we have a
critical issue related to text based languages that needs to be
addressed asap, possibly this one: #158893.

Fixes #158819.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>

src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx

@@ -43,6 +43,8 @@ function getStateContainer(savedSearch?: SavedSearch) {
     hideChart: false,
   });
 
+  stateContainer.internalState.transitions.setDataView(dataViewMock);
+
   return stateContainer;
 }
 

src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx

@@ -51,7 +51,6 @@ export const DiscoverHistogramLayout = ({
   return (
     <UnifiedHistogramContainer
       {...unifiedHistogramProps}
-      dataView={dataView}
       searchSessionId={searchSessionId}
       requestAdapter={dataState.inspectorAdapters.requests}
       resizeRef={resizeRef}

src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx

@@ -152,7 +152,7 @@ describe('useDiscoverHistogram', () => {
       act(() => {
         hook.result.current.ref(api);
       });
-      expect(api.state$.subscribe).toHaveBeenCalledTimes(3);
+      expect(api.state$.subscribe).toHaveBeenCalledTimes(2);
     });
 
     it('should sync Unified Histogram state with the state container', async () => {

src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts

@@ -14,17 +14,23 @@ import {
 } from '@kbn/unified-histogram-plugin/public';
 import { isEqual } from 'lodash';
 import { useCallback, useEffect, useRef, useMemo, useState } from 'react';
-import { distinctUntilChanged, filter, map, Observable, pairwise, startWith } from 'rxjs';
+import {
+  debounceTime,
+  distinctUntilChanged,
+  filter,
+  map,
+  merge,
+  Observable,
+  pairwise,
+  startWith,
+} from 'rxjs';
 import useObservable from 'react-use/lib/useObservable';
-import type { Suggestion } from '@kbn/lens-plugin/public';
-import useLatest from 'react-use/lib/useLatest';
 import type { RequestAdapter } from '@kbn/inspector-plugin/common';
 import { useDiscoverServices } from '../../../../hooks/use_discover_services';
 import { getUiActions } from '../../../../kibana_services';
 import { FetchStatus } from '../../../types';
 import { useDataState } from '../../hooks/use_data_state';
 import type { InspectorAdapters } from '../../hooks/use_inspector';
-import type { DataDocuments$ } from '../../services/discover_data_state_container';
 import { checkHitCount, sendErrorTo } from '../../hooks/use_saved_search_messages';
 import type { DiscoverStateContainer } from '../../services/discover_state';
 import { addLog } from '../../../../utils/add_log';
@@ -155,22 +161,6 @@ export const useDiscoverHistogram = ({
     };
   }, [stateContainer.appState.state$, unifiedHistogram]);
 
-  /**
-   * Columns
-   */
-
-  // Update the columns only when documents are fetched so the Lens suggestions
-  // don't constantly change when the user modifies the table columns
-  const columnsObservable = useMemo(
-    () => createColumnsObservable(savedSearchData$.documents$),
-    [savedSearchData$.documents$]
-  );
-
-  const columns = useObservable(
-    columnsObservable,
-    savedSearchData$.documents$.getValue().textBasedQueryColumns?.map(({ name }) => name) ?? []
-  );
-
   /**
    * Total hits
    */
@@ -232,89 +222,89 @@ export const useDiscoverHistogram = ({
     timefilter.getTime()
   );
 
+  // When in text based language mode, update the data view, query, and
+  // columns only when documents are done fetching so the Lens suggestions
+  // don't frequently change, such as when the user modifies the table
+  // columns, which would trigger unnecessary refetches.
+  const textBasedFetchComplete$ = useMemo(
+    () => createFetchCompleteObservable(stateContainer),
+    [stateContainer]
+  );
+
+  const {
+    dataView: textBasedDataView,
+    query: textBasedQuery,
+    columns,
+  } = useObservable(textBasedFetchComplete$, {
+    dataView: stateContainer.internalState.getState().dataView!,
+    query: stateContainer.appState.getState().query,
+    columns:
+      savedSearchData$.documents$.getValue().textBasedQueryColumns?.map(({ name }) => name) ?? [],
+  });
+
   /**
    * Data fetching
    */
 
-  const savedSearchFetch$ = stateContainer.dataState.fetch$;
-  const skipDiscoverRefetch = useRef<boolean>();
-  const skipLensSuggestionRefetch = useRef<boolean>();
-  const usingLensSuggestion = useLatest(isPlainRecord && !hideChart);
+  const skipRefetch = useRef<boolean>();
 
   // Skip refetching when showing the chart since Lens will
   // automatically fetch when the chart is shown
   useEffect(() => {
-    if (skipDiscoverRefetch.current === undefined) {
-      skipDiscoverRefetch.current = false;
+    if (skipRefetch.current === undefined) {
+      skipRefetch.current = false;
     } else {
-      skipDiscoverRefetch.current = !hideChart;
+      skipRefetch.current = !hideChart;
     }
   }, [hideChart]);
 
-  // Trigger a unified histogram refetch when savedSearchFetch$ is triggered
+  // Handle unified histogram refetching
   useEffect(() => {
-    const subscription = savedSearchFetch$.subscribe(() => {
-      if (!skipDiscoverRefetch.current) {
-        addLog('Unified Histogram - Discover refetch');
-        unifiedHistogram?.refetch();
+    if (!unifiedHistogram) {
+      return;
+    }
+
+    let fetch$: Observable<string>;
+
+    // When in text based language mode, we refetch under two conditions:
+    // 1. When the current Lens suggestion changes. This syncs the visualization
+    //    with the user's selection.
+    // 2. When the documents are done fetching. This is necessary because we don't
+    //    have access to the latest columns until after the documents are fetched,
+    //    which are required to get the latest Lens suggestion, which would trigger
+    //    a refetch anyway and result in multiple unnecessary fetches.
+    if (isPlainRecord) {
+      fetch$ = merge(
+        createCurrentSuggestionObservable(unifiedHistogram.state$).pipe(map(() => 'lens')),
+        textBasedFetchComplete$.pipe(map(() => 'discover'))
+      ).pipe(debounceTime(50));
+    } else {
+      fetch$ = stateContainer.dataState.fetch$.pipe(map(() => 'discover'));
+    }
+
+    const subscription = fetch$.subscribe((source) => {
+      if (!skipRefetch.current) {
+        if (source === 'discover') addLog('Unified Histogram - Discover refetch');
+        if (source === 'lens') addLog('Unified Histogram - Lens suggestion refetch');
+        unifiedHistogram.refetch();
       }
 
-      skipDiscoverRefetch.current = false;
+      skipRefetch.current = false;
     });
 
     return () => {
       subscription.unsubscribe();
     };
-  }, [savedSearchFetch$, unifiedHistogram, usingLensSuggestion]);
+  }, [isPlainRecord, stateContainer.dataState.fetch$, textBasedFetchComplete$, unifiedHistogram]);
 
-  // Reload the chart when the current suggestion changes
-  const [currentSuggestion, setCurrentSuggestion] = useState<Suggestion>();
-
-  useEffect(() => {
-    if (!skipLensSuggestionRefetch.current && currentSuggestion && usingLensSuggestion.current) {
-      addLog('Unified Histogram - Lens suggestion refetch');
-      unifiedHistogram?.refetch();
-    }
-
-    skipLensSuggestionRefetch.current = false;
-  }, [currentSuggestion, unifiedHistogram, usingLensSuggestion]);
-
-  useEffect(() => {
-    const subscription = createCurrentSuggestionObservable(unifiedHistogram?.state$)?.subscribe(
-      setCurrentSuggestion
-    );
-
-    return () => {
-      subscription?.unsubscribe();
-    };
-  }, [unifiedHistogram]);
-
-  // When the data view or query changes, which will trigger a current suggestion change,
-  // skip the next refetch since we want to wait for the columns to update first, which
-  // doesn't happen until after the documents are fetched
-  const dataViewId = useInternalStateSelector((state) => state.dataView?.id);
-  const skipFetchParams = useRef({ dataViewId, query });
-
-  useEffect(() => {
-    const newSkipFetchParams = { dataViewId, query };
-
-    if (isEqual(skipFetchParams.current, newSkipFetchParams)) {
-      return;
-    }
-
-    skipFetchParams.current = newSkipFetchParams;
-
-    if (usingLensSuggestion.current) {
-      skipLensSuggestionRefetch.current = true;
-      skipDiscoverRefetch.current = true;
-    }
-  }, [dataViewId, query, usingLensSuggestion]);
+  const dataView = useInternalStateSelector((state) => state.dataView!);
 
   return {
     ref,
     getCreationOptions,
     services: { ...services, uiActions: getUiActions() },
-    query,
+    dataView: isPlainRecord ? textBasedDataView : dataView,
+    query: isPlainRecord ? textBasedQuery : query,
     filters,
     timeRange,
     relativeTimeRange,
@@ -389,11 +379,15 @@ const createAppStateObservable = (state$: Observable<DiscoverAppState>) => {
   );
 };
 
-const createColumnsObservable = (documents$: DataDocuments$) => {
-  return documents$.pipe(
+const createFetchCompleteObservable = (stateContainer: DiscoverStateContainer) => {
+  return stateContainer.dataState.data$.documents$.pipe(
     distinctUntilChanged((prev, curr) => prev.fetchStatus === curr.fetchStatus),
     filter(({ fetchStatus }) => fetchStatus === FetchStatus.COMPLETE),
-    map(({ textBasedQueryColumns }) => textBasedQueryColumns?.map(({ name }) => name) ?? [])
+    map(({ textBasedQueryColumns }) => ({
+      dataView: stateContainer.internalState.getState().dataView!,
+      query: stateContainer.appState.getState().query!,
+      columns: textBasedQueryColumns?.map(({ name }) => name) ?? [],
+    }))
   );
 };
 
@@ -404,8 +398,8 @@ const createTotalHitsObservable = (state$?: Observable<UnifiedHistogramState>) =
   );
 };
 
-const createCurrentSuggestionObservable = (state$?: Observable<UnifiedHistogramState>) => {
-  return state$?.pipe(
+const createCurrentSuggestionObservable = (state$: Observable<UnifiedHistogramState>) => {
+  return state$.pipe(
     map((state) => state.currentSuggestion),
     distinctUntilChanged(isEqual)
   );

test/functional/apps/discover/group3/_request_counts.ts

@@ -0,0 +1,247 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import expect from '@kbn/expect';
+import { FtrProviderContext } from '../ftr_provider_context';
+
+export default function ({ getService, getPageObjects }: FtrProviderContext) {
+  const esArchiver = getService('esArchiver');
+  const kibanaServer = getService('kibanaServer');
+  const PageObjects = getPageObjects([
+    'common',
+    'discover',
+    'timePicker',
+    'header',
+    'unifiedSearch',
+    'settings',
+  ]);
+  const testSubjects = getService('testSubjects');
+  const browser = getService('browser');
+  const monacoEditor = getService('monacoEditor');
+  const filterBar = getService('filterBar');
+  const queryBar = getService('queryBar');
+  const elasticChart = getService('elasticChart');
+
+  describe('discover request counts', function describeIndexTests() {
+    before(async function () {
+      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
+      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/long_window_logstash');
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover');
+      await kibanaServer.importExport.load(
+        'test/functional/fixtures/kbn_archiver/long_window_logstash_index_pattern'
+      );
+      await kibanaServer.uiSettings.replace({
+        defaultIndex: 'logstash-*',
+        'bfetch:disable': true,
+        'discover:enableSql': true,
+      });
+      await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings();
+    });
+
+    after(async () => {
+      await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
+      await kibanaServer.savedObjects.cleanStandardList();
+      await kibanaServer.uiSettings.replace({});
+    });
+
+    beforeEach(async () => {
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.header.waitUntilLoadingHasFinished();
+    });
+
+    const getSearchCount = async (type: 'ese' | 'sql') => {
+      const requests = await browser.execute(() =>
+        performance
+          .getEntries()
+          .filter((entry: any) => ['fetch', 'xmlhttprequest'].includes(entry.initiatorType))
+      );
+      return requests.filter((entry) => entry.name.endsWith(`/internal/search/${type}`)).length;
+    };
+
+    const waitForLoadingToFinish = async () => {
+      await PageObjects.header.waitUntilLoadingHasFinished();
+      await PageObjects.discover.waitForDocTableLoadingComplete();
+      await elasticChart.canvasExists();
+    };
+
+    const expectSearches = async (type: 'ese' | 'sql', expected: number, cb: Function) => {
+      await browser.execute(async () => {
+        performance.clearResourceTimings();
+      });
+      let searchCount = await getSearchCount(type);
+      expect(searchCount).to.be(0);
+      await cb();
+      await waitForLoadingToFinish();
+      searchCount = await getSearchCount(type);
+      expect(searchCount).to.be(expected);
+    };
+
+    const getSharedTests = ({
+      type,
+      savedSearch,
+      query1,
+      query2,
+      setQuery,
+      skipSavedSearchTests,
+    }: {
+      type: 'ese' | 'sql';
+      savedSearch: string;
+      query1: string;
+      query2: string;
+      setQuery: (query: string) => Promise<void>;
+      skipSavedSearchTests?: boolean;
+    }) => {
+      it('should send 2 search requests (documents + chart) on page load', async () => {
+        await browser.refresh();
+        await browser.execute(async () => {
+          performance.setResourceTimingBufferSize(Number.MAX_SAFE_INTEGER);
+        });
+        await waitForLoadingToFinish();
+        const searchCount = await getSearchCount(type);
+        expect(searchCount).to.be(2);
+      });
+
+      it('should send 2 requests (documents + chart) when refreshing', async () => {
+        await expectSearches(type, 2, async () => {
+          await queryBar.clickQuerySubmitButton();
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when changing the query', async () => {
+        await expectSearches(type, 2, async () => {
+          await setQuery(query1);
+          await queryBar.clickQuerySubmitButton();
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when changing the time range', async () => {
+        await expectSearches(type, 2, async () => {
+          await PageObjects.timePicker.setAbsoluteRange(
+            'Sep 21, 2015 @ 06:31:44.000',
+            'Sep 23, 2015 @ 00:00:00.000'
+          );
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when toggling the chart visibility', async () => {
+        await expectSearches(type, 2, async () => {
+          await PageObjects.discover.toggleChartVisibility();
+        });
+        await expectSearches(type, 2, async () => {
+          await PageObjects.discover.toggleChartVisibility();
+        });
+      });
+
+      if (!skipSavedSearchTests) {
+        it('should send 2 requests for saved search changes', async () => {
+          await setQuery(query1);
+          await queryBar.clickQuerySubmitButton();
+          await PageObjects.timePicker.setAbsoluteRange(
+            'Sep 21, 2015 @ 06:31:44.000',
+            'Sep 23, 2015 @ 00:00:00.000'
+          );
+          await waitForLoadingToFinish();
+          // creating the saved search
+          await expectSearches(type, 2, async () => {
+            await PageObjects.discover.saveSearch(savedSearch);
+          });
+          // resetting the saved search
+          await setQuery(query2);
+          await queryBar.clickQuerySubmitButton();
+          await waitForLoadingToFinish();
+          await expectSearches(type, 2, async () => {
+            await PageObjects.discover.clickResetSavedSearchButton();
+          });
+          // clearing the saved search
+          await expectSearches(type, 2, async () => {
+            await testSubjects.click('discoverNewButton');
+            await waitForLoadingToFinish();
+          });
+          // loading the saved search
+          await expectSearches(type, 2, async () => {
+            await PageObjects.discover.loadSavedSearch(savedSearch);
+          });
+        });
+      }
+    };
+
+    describe('data view mode', () => {
+      const type = 'ese';
+
+      getSharedTests({
+        type,
+        savedSearch: 'data view test',
+        query1: 'bytes > 1000',
+        query2: 'bytes < 2000',
+        setQuery: (query) => queryBar.setQuery(query),
+      });
+
+      it('should send 2 requests (documents + chart) when adding a filter', async () => {
+        await expectSearches(type, 2, async () => {
+          await filterBar.addFilter({
+            field: 'extension',
+            operation: 'is',
+            value: 'jpg',
+          });
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when sorting', async () => {
+        await expectSearches(type, 2, async () => {
+          await PageObjects.discover.clickFieldSort('@timestamp', 'Sort Old-New');
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when changing to a breakdown field without an other bucket', async () => {
+        await expectSearches(type, 2, async () => {
+          await PageObjects.discover.chooseBreakdownField('type');
+        });
+      });
+
+      it('should send 3 requests (documents + chart + other bucket) when changing to a breakdown field with an other bucket', async () => {
+        await expectSearches(type, 3, async () => {
+          await PageObjects.discover.chooseBreakdownField('extension.raw');
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when changing the chart interval', async () => {
+        await expectSearches(type, 2, async () => {
+          await PageObjects.discover.setChartInterval('Day');
+        });
+      });
+
+      it('should send 2 requests (documents + chart) when changing the data view', async () => {
+        await expectSearches(type, 2, async () => {
+          await PageObjects.discover.selectIndexPattern('long-window-logstash-*');
+        });
+      });
+    });
+
+    describe('SQL mode', () => {
+      const type = 'sql';
+
+      beforeEach(async () => {
+        await PageObjects.discover.selectTextBaseLang('SQL');
+        monacoEditor.setCodeEditorValue('SELECT count(*) FROM "logstash-*" WHERE bytes > 1000');
+        await queryBar.clickQuerySubmitButton();
+        await waitForLoadingToFinish();
+      });
+
+      getSharedTests({
+        type,
+        savedSearch: 'sql test',
+        query1: 'SELECT type, count(*) FROM "logstash-*" WHERE bytes > 1000 GROUP BY type',
+        query2: 'SELECT type, count(*) FROM "logstash-*" WHERE bytes < 2000 GROUP BY type',
+        setQuery: (query) => monacoEditor.setCodeEditorValue(query),
+        // TODO: We get 4 requests instead of 2 for the saved search test in SQL mode,
+        // so we're skipping them for now until we fix the issue.
+        skipSavedSearchTests: true,
+      });
+    });
+  });
+}

test/functional/apps/discover/group3/index.ts

@@ -22,5 +22,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
 
     loadTestFile(require.resolve('./_drag_drop'));
     loadTestFile(require.resolve('./_sidebar'));
+    loadTestFile(require.resolve('./_request_counts'));
   });
 }