[Discover] Show the fetched Discover results even when histogram request fails on some shards (#198553)

- Closes https://github.com/elastic/kibana/issues/198496 ## Summary This PR fixes an issue when the histogram request returns only a partial result (0 or greater than 0) by adding a warning icon next to the total hits counter and not blocking the whole page with "No results" message (when partial result with 0 hits from histogram). <img width="1436" alt="Screenshot 2024-10-31 at 15 45 17" src="https://github.com/user-attachments/assets/9a769fe6-bdcf-4d20-ae6e-698a5b08d76f"> ### Testing Execute the following and open `example*` data view in Discover. ``` PUT example1 PUT example1/_mapping { "properties": { "message": { "type": "text" }, "date": { "type": "date" } } } PUT example1/_doc/11 { "message": "11", "date": "2024-11-11T12:10:30Z" } PUT example1/_doc/12 { "message": "22", "date": "2024-11-12T12:10:30Z" } PUT example2 PUT example2/_mapping { "properties": { "message": { "type": "keyword" }, "date": { "type": "date" } } } PUT example2/_doc/21 { "message": "21", "date": "2024-12-01T12:10:30Z" } PUT example2/_doc/22 { "message": "22", "date": "2024-12-02T12:10:30Z" } ``` Then add `message` as a breakdown field. Notice that the histogram gets some partial results: <img width="1563" alt="Screenshot 2024-10-31 at 16 11 14" src="https://github.com/user-attachments/assets/8a53f661-38a2-48f8-b082-823de77ac4f2"> Now, add a filter for `_id: 11` and notice that the histogram request has no results (it partially failed on some shards) but Discover still renders the table: <img width="1564" alt="Screenshot 2024-10-31 at 16 11 31" src="https://github.com/user-attachments/assets/e154ab5d-c5d4-4703-abd4-7bf3cd7a15fb"> ### Checklist - [x] 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) - [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 --------- Co-authored-by: Davis McPhee <davismcphee@hotmail.com>
2025-04-24 01:38:56 -04:00 · 2024-11-06 13:45:01 +01:00 · 2024-11-06 13:45:01 +01:00 · c1e00a8871
commit c1e00a8871
parent e803f5c794
6 changed files with 119 additions and 31 deletions
--- a/src/plugins/discover/public/components/hits_counter/hits_counter.test.tsx
+++ b/src/plugins/discover/public/components/hits_counter/hits_counter.test.tsx
@ -14,8 +14,20 @@ import { findTestSubject } from '@elastic/eui/lib/test';
 import { EuiLoadingSpinner } from '@elastic/eui';
 import { BehaviorSubject } from 'rxjs';
 import { getDiscoverStateMock } from '../../__mocks__/discover_state.mock';
-import { DataTotalHits$ } from '../../application/main/state_management/discover_data_state_container';
+import {
+  DataDocuments$,
+  DataTotalHits$,
+} from '../../application/main/state_management/discover_data_state_container';
 import { FetchStatus } from '../../application/types';
+import { dataViewMock, esHitsMock } from '@kbn/discover-utils/src/__mocks__';
+import { buildDataTableRecord } from '@kbn/discover-utils';
+
+function getDocuments$(count: number = 5) {
+  return new BehaviorSubject({
+    fetchStatus: FetchStatus.COMPLETE,
+    result: esHitsMock.map((esHit) => buildDataTableRecord(esHit, dataViewMock)).slice(0, count),
+  }) as DataDocuments$;
+}

 describe('hits counter', function () {
  it('expect to render the number of hits', function () {
@ -24,6 +36,7 @@ describe('hits counter', function () {
      fetchStatus: FetchStatus.COMPLETE,
      result: 1,
    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
    const component1 = mountWithIntl(
      <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
    );
@ -45,6 +58,7 @@ describe('hits counter', function () {
      fetchStatus: FetchStatus.COMPLETE,
      result: 1899,
    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
    const component1 = mountWithIntl(
      <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
    );
@ -64,6 +78,7 @@ describe('hits counter', function () {
      fetchStatus: FetchStatus.PARTIAL,
      result: 2,
    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
    const component = mountWithIntl(
      <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
    );
@ -76,6 +91,7 @@ describe('hits counter', function () {
      fetchStatus: FetchStatus.PARTIAL,
      result: 2,
    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
    const component = mountWithIntl(
      <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
    );
@ -89,9 +105,51 @@ describe('hits counter', function () {
      fetchStatus: FetchStatus.LOADING,
      result: undefined,
    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
    const component = mountWithIntl(
      <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
    );
    expect(component.isEmptyRender()).toBe(true);
  });
+
+  it('should render discoverQueryHitsPartial when status is error', () => {
+    const stateContainer = getDiscoverStateMock({ isTimeBased: true });
+    stateContainer.dataState.data$.totalHits$ = new BehaviorSubject({
+      fetchStatus: FetchStatus.ERROR,
+      result: undefined,
+    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$(3);
+    const component = mountWithIntl(
+      <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
+    );
+    expect(component.find('[data-test-subj="discoverQueryHitsPartial"]').length).toBe(1);
+    expect(findTestSubject(component, 'discoverQueryTotalHits').text()).toBe('≥3 resultsInfo');
+    expect(component.text()).toBe('≥3 resultsInfo');
+
+    stateContainer.dataState.data$.totalHits$ = new BehaviorSubject({
+      fetchStatus: FetchStatus.ERROR,
+      result: 200,
+    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$(2);
+
+    const component2 = mountWithIntl(
+      <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
+    );
+    expect(component2.find('[data-test-subj="discoverQueryHitsPartial"]').length).toBe(1);
+    expect(findTestSubject(component2, 'discoverQueryTotalHits').text()).toBe('≥200Info');
+    expect(component2.text()).toBe(' (≥200Info)');
+
+    stateContainer.dataState.data$.totalHits$ = new BehaviorSubject({
+      fetchStatus: FetchStatus.ERROR,
+      result: 0,
+    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$(1);
+
+    const component3 = mountWithIntl(
+      <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
+    );
+    expect(component3.find('[data-test-subj="discoverQueryHitsPartial"]').length).toBe(1);
+    expect(findTestSubject(component3, 'discoverQueryTotalHits').text()).toBe('≥1Info');
+    expect(component3.text()).toBe(' (≥1Info)');
+  });
 });
--- a/src/plugins/discover/public/components/hits_counter/hits_counter.tsx
+++ b/src/plugins/discover/public/components/hits_counter/hits_counter.tsx
@ -8,7 +8,7 @@
 */

 import React from 'react';
-import { EuiFlexGroup, EuiFlexItem, EuiText, EuiLoadingSpinner } from '@elastic/eui';
+import { EuiFlexGroup, EuiFlexItem, EuiText, EuiLoadingSpinner, EuiIconTip } from '@elastic/eui';
 import { FormattedMessage, FormattedNumber } from '@kbn/i18n-react';
 import { i18n } from '@kbn/i18n';
 import { css } from '@emotion/react';
@ -29,18 +29,33 @@ export interface HitsCounterProps {
 export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }) => {
  const totalHits$ = stateContainer.dataState.data$.totalHits$;
  const totalHitsState = useDataState(totalHits$);
-  const hitsTotal = totalHitsState.result;
+  let hitsTotal = totalHitsState.result;
  const hitsStatus = totalHitsState.fetchStatus;

+  const documents$ = stateContainer.dataState.data$.documents$;
+  const documentsState = useDataState(documents$);
+  const documentsCount = documentsState.result?.length || 0;
+
  if (!hitsTotal && hitsStatus === FetchStatus.LOADING) {
    return null;
  }

+  if (
+    hitsStatus === FetchStatus.ERROR &&
+    documentsState.fetchStatus === FetchStatus.COMPLETE &&
+    documentsCount > (hitsTotal ?? 0)
+  ) {
+    // if histogram returned partial results and which are less than the fetched documents count =>
+    // override hitsTotal with the fetched documents count
+    hitsTotal = documentsCount;
+  }
+
+  const showGreaterOrEqualSign =
+    hitsStatus === FetchStatus.PARTIAL || hitsStatus === FetchStatus.ERROR;
+
  const formattedHits = (
    <span
-      data-test-subj={
-        hitsStatus === FetchStatus.PARTIAL ? 'discoverQueryHitsPartial' : 'discoverQueryHits'
-      }
+      data-test-subj={showGreaterOrEqualSign ? 'discoverQueryHitsPartial' : 'discoverQueryHits'}
    >
      <FormattedNumber value={hitsTotal ?? 0} />
    </span>
@ -55,7 +70,7 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }

  const element = (
    <EuiFlexGroup
-      gutterSize="s"
+      gutterSize="xs"
      responsive={false}
      justifyContent="center"
      alignItems="center"
@ -66,8 +81,8 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
      <EuiFlexItem grow={false} aria-live="polite" css={hitsCounterTextCss}>
        <EuiText className="eui-textTruncate" size="s">
          <strong>
-            {hitsStatus === FetchStatus.PARTIAL &&
-              (mode === HitsCounterMode.standalone ? (
+            {showGreaterOrEqualSign ? (
+              mode === HitsCounterMode.standalone ? (
                <FormattedMessage
                  id="discover.hitsCounter.partialHitsPluralTitle"
                  defaultMessage="≥{formattedHits} {hits, plural, one {result} other {results}}"
@ -79,17 +94,16 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
                  defaultMessage="≥{formattedHits}"
                  values={{ formattedHits }}
                />
-              ))}
-            {hitsStatus !== FetchStatus.PARTIAL &&
-              (mode === HitsCounterMode.standalone ? (
-                <FormattedMessage
-                  id="discover.hitsCounter.hitsPluralTitle"
-                  defaultMessage="{formattedHits} {hits, plural, one {result} other {results}}"
-                  values={{ hits: hitsTotal, formattedHits }}
-                />
-              ) : (
-                formattedHits
-              ))}
+              )
+            ) : mode === HitsCounterMode.standalone ? (
+              <FormattedMessage
+                id="discover.hitsCounter.hitsPluralTitle"
+                defaultMessage="{formattedHits} {hits, plural, one {result} other {results}}"
+                values={{ hits: hitsTotal, formattedHits }}
+              />
+            ) : (
+              formattedHits
+            )}
          </strong>
        </EuiText>
      </EuiFlexItem>
@ -103,6 +117,19 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
          />
        </EuiFlexItem>
      )}
+      {hitsStatus === FetchStatus.ERROR && (
+        <EuiFlexItem grow={false}>
+          <EuiIconTip
+            type="warning"
+            color="warning"
+            size="s"
+            content={i18n.translate('discover.hitsCounter.hitCountWarningTooltip', {
+              defaultMessage: 'Results might be incomplete',
+            })}
+            iconProps={{ css: { display: 'block' } }}
+          />
+        </EuiFlexItem>
+      )}
    </EuiFlexGroup>
  );

--- a/src/plugins/unified_histogram/public/chart/histogram.test.tsx
+++ b/src/plugins/unified_histogram/public/chart/histogram.test.tsx
@ -240,7 +240,7 @@ describe('Histogram', () => {
      onLoad(false, adapters);
    });
    expect(props.onTotalHitsChange).toHaveBeenLastCalledWith(
-      UnifiedHistogramFetchStatus.complete,
+      UnifiedHistogramFetchStatus.error,
      100
    );
    expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters });
--- a/src/plugins/unified_histogram/public/chart/histogram.tsx
+++ b/src/plugins/unified_histogram/public/chart/histogram.tsx
@ -130,9 +130,6 @@ export function Histogram({
        | undefined;
      const response = json?.rawResponse;

-      // The response can have `response?._shards.failed` but we should still be able to show hits number
-      // TODO: show shards warnings as a badge next to the total hits number
-
      if (requestFailed) {
        onTotalHitsChange?.(UnifiedHistogramFetchStatus.error, undefined);
        onChartLoad?.({ adapters: adapters ?? {} });
@ -142,10 +139,14 @@ export function Histogram({
      const adapterTables = adapters?.tables?.tables;
      const totalHits = computeTotalHits(hasLensSuggestions, adapterTables, isPlainRecord);

-      onTotalHitsChange?.(
-        isLoading ? UnifiedHistogramFetchStatus.loading : UnifiedHistogramFetchStatus.complete,
-        totalHits ?? hits?.total
-      );
+      if (response?._shards?.failed || response?.timed_out) {
+        onTotalHitsChange?.(UnifiedHistogramFetchStatus.error, totalHits);
+      } else {
+        onTotalHitsChange?.(
+          isLoading ? UnifiedHistogramFetchStatus.loading : UnifiedHistogramFetchStatus.complete,
+          totalHits ?? hits?.total
+        );
+      }

      if (response) {
        const newBucketInterval = buildBucketInterval({
--- a/test/functional/apps/discover/ccs_compatibility/_search_errors.ts
+++ b/test/functional/apps/discover/ccs_compatibility/_search_errors.ts
@ -65,7 +65,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

      // Ensure documents are still returned for the successful shards
      await retry.try(async function tryingForTime() {
-        const hitCount = await discover.getHitCount();
+        const hitCount = await discover.getHitCount({ isPartial: true });
        expect(hitCount).to.be('9,247');
      });

--- a/test/functional/page_objects/discover_page.ts
+++ b/test/functional/page_objects/discover_page.ts
@ -339,9 +339,11 @@ export class DiscoverPageObject extends FtrService {
    return await this.header.waitUntilLoadingHasFinished();
  }

-  public async getHitCount() {
+  public async getHitCount({ isPartial }: { isPartial?: boolean } = {}) {
    await this.header.waitUntilLoadingHasFinished();
-    return await this.testSubjects.getVisibleText('discoverQueryHits');
+    return await this.testSubjects.getVisibleText(
+      isPartial ? 'discoverQueryHitsPartial' : 'discoverQueryHits'
+    );
  }

  public async getHitCountInt() {