[8.16] [Data Views] Mitigate issue where `has_es_data` chec…

…k can cause Kibana to hang (elastic#200476) (elastic#201024) # Backport This will backport the following commits from `main` to `8.16`: - [[Data Views] Mitigate issue where `has_es_data` check can cause Kibana to hang (elastic#200476)](elastic#200476)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Davis McPhee <[email protected]>
kibanamachine · Nov 20, 2024 · 44f4203 · 44f4203
1 parent a6a0b31
commit 44f4203
Show file tree

Hide file tree

Showing 11 changed files with 582 additions and 29 deletions.
diff --git a/src/plugins/data_views/common/constants.ts b/src/plugins/data_views/common/constants.ts
@@ -79,3 +79,12 @@ export const EXISTING_INDICES_PATH = '/internal/data_views/_existing_indices';
 export const DATA_VIEWS_FIELDS_EXCLUDED_TIERS = 'data_views:fields_excluded_data_tiers';
 
 export const DEFAULT_DATA_VIEW_ID = 'defaultIndex';
+
+/**
+ * Valid `failureReason` attribute values for `has_es_data` API error responses
+ */
+export enum HasEsDataFailureReason {
+  localDataTimeout = 'local_data_timeout',
+  remoteDataTimeout = 'remote_data_timeout',
+  unknown = 'unknown',
+}
diff --git a/src/plugins/data_views/common/index.ts b/src/plugins/data_views/common/index.ts
@@ -13,6 +13,7 @@ export {
   META_FIELDS,
   DATA_VIEW_SAVED_OBJECT_TYPE,
   MAX_DATA_VIEW_FIELD_DESCRIPTION_LENGTH,
+  HasEsDataFailureReason,
 } from './constants';
 
 export { LATEST_VERSION } from './content_management/v1/constants';

diff --git a/src/plugins/data_views/common/types.ts b/src/plugins/data_views/common/types.ts
@@ -571,4 +571,5 @@ export interface ClientConfigType {
   scriptedFieldsEnabled?: boolean;
   dataTiersExcludedForFields?: string;
   fieldListCachingEnabled?: boolean;
+  hasEsDataTimeout: number;
 }
diff --git a/src/plugins/data_views/public/services/has_data.test.ts b/src/plugins/data_views/public/services/has_data.test.ts
@@ -10,6 +10,7 @@
 import { coreMock } from '@kbn/core/public/mocks';
 
 import { HasData } from './has_data';
+import { HttpFetchError } from '@kbn/core-http-browser-internal/src/http_fetch_error';
 
 describe('when calling hasData service', () => {
   describe('hasDataView', () => {
@@ -170,6 +171,78 @@ describe('when calling hasData service', () => {
 
         expect(await response).toBe(false);
       });
+
+      it('should return true and show an error toast when checking for remote cluster data times out', async () => {
+        const coreStart = coreMock.createStart();
+        const http = coreStart.http;
+
+        // Mock getIndices
+        const spy = jest.spyOn(http, 'get').mockImplementation(() =>
+          Promise.reject(
+            new HttpFetchError(
+              'Timeout while checking for Elasticsearch data',
+              'TimeoutError',
+              new Request(''),
+              undefined,
+              {
+                statusCode: 504,
+                message: 'Timeout while checking for Elasticsearch data',
+                attributes: {
+                  failureReason: 'remote_data_timeout',
+                },
+              }
+            )
+          )
+        );
+        const hasData = new HasData();
+        const hasDataService = hasData.start(coreStart, true);
+        const response = hasDataService.hasESData();
+
+        expect(spy).toHaveBeenCalledTimes(1);
+        expect(await response).toBe(true);
+        expect(coreStart.notifications.toasts.addDanger).toHaveBeenCalledTimes(1);
+        expect(coreStart.notifications.toasts.addDanger).toHaveBeenCalledWith({
+          title: 'Remote cluster timeout',
+          text: 'Checking for data on remote clusters timed out. One or more remote clusters may be unavailable.',
+        });
+      });
+
+      it('should return true and not show an error toast when checking for remote cluster data times out, but onRemoteDataTimeout is overridden', async () => {
+        const coreStart = coreMock.createStart();
+        const http = coreStart.http;
+
+        // Mock getIndices
+        const responseBody = {
+          statusCode: 504,
+          message: 'Timeout while checking for Elasticsearch data',
+          attributes: {
+            failureReason: 'remote_data_timeout',
+          },
+        };
+        const spy = jest
+          .spyOn(http, 'get')
+          .mockImplementation(() =>
+            Promise.reject(
+              new HttpFetchError(
+                'Timeout while checking for Elasticsearch data',
+                'TimeoutError',
+                new Request(''),
+                undefined,
+                responseBody
+              )
+            )
+          );
+        const hasData = new HasData();
+        const hasDataService = hasData.start(coreStart, true);
+        const onRemoteDataTimeout = jest.fn();
+        const response = hasDataService.hasESData({ onRemoteDataTimeout });
+
+        expect(spy).toHaveBeenCalledTimes(1);
+        expect(await response).toBe(true);
+        expect(coreStart.notifications.toasts.addDanger).not.toHaveBeenCalled();
+        expect(onRemoteDataTimeout).toHaveBeenCalledTimes(1);
+        expect(onRemoteDataTimeout).toHaveBeenCalledWith(responseBody);
+      });
     });
 
     describe('resolve/cluster not available', () => {

diff --git a/src/plugins/data_views/public/services/has_data.ts b/src/plugins/data_views/public/services/has_data.ts
@@ -8,10 +8,22 @@
  */
 
 import { CoreStart, HttpStart } from '@kbn/core/public';
-import { DEFAULT_ASSETS_TO_IGNORE } from '../../common';
+import { IHttpFetchError, ResponseErrorBody, isHttpFetchError } from '@kbn/core-http-browser';
+import { isObject } from 'lodash';
+import { i18n } from '@kbn/i18n';
+import { DEFAULT_ASSETS_TO_IGNORE, HasEsDataFailureReason } from '../../common';
 import { HasDataViewsResponse, IndicesViaSearchResponse } from '..';
 import { IndicesResponse, IndicesResponseModified } from '../types';
 
+export interface HasEsDataParams {
+  /**
+   * Callback to handle the case where checking for remote data times out.
+   * If not provided, the default behavior is to show a toast notification.
+   * @param body The error response body
+   */
+  onRemoteDataTimeout?: (body: ResponseErrorBody) => void;
+}
+
 export class HasData {
   private removeAliases = (source: IndicesResponseModified): boolean => !source.item.indices;
 
@@ -38,28 +50,55 @@ export class HasData {
       return hasLocalESData;
     };
 
-    const hasESDataViaResolveCluster = async () => {
+    const hasESDataViaResolveCluster = async (
+      onRemoteDataTimeout: (body: ResponseErrorBody) => void
+    ) => {
       try {
         const { hasEsData } = await http.get<{ hasEsData: boolean }>(
           '/internal/data_views/has_es_data',
-          {
-            version: '1',
-          }
+          { version: '1' }
         );
+
         return hasEsData;
       } catch (e) {
+        if (
+          this.isResponseError(e) &&
+          e.body?.statusCode === 504 &&
+          e.body?.attributes?.failureReason === HasEsDataFailureReason.remoteDataTimeout
+        ) {
+          onRemoteDataTimeout(e.body);
+
+          // In the case of a remote cluster timeout,
+          // we can't be sure if there is data or not,
+          // so just assume there is
+          return true;
+        }
+
         // fallback to previous implementation
         return hasESDataViaResolveIndex();
       }
     };
 
+    const showRemoteDataTimeoutToast = () =>
+      core.notifications.toasts.addDanger({
+        title: i18n.translate('dataViews.hasData.remoteDataTimeoutTitle', {
+          defaultMessage: 'Remote cluster timeout',
+        }),
+        text: i18n.translate('dataViews.hasData.remoteDataTimeoutText', {
+          defaultMessage:
+            'Checking for data on remote clusters timed out. One or more remote clusters may be unavailable.',
+        }),
+      });
+
     return {
       /**
        * Check to see if ES data exists
        */
-      hasESData: async (): Promise<boolean> => {
+      hasESData: async ({
+        onRemoteDataTimeout = showRemoteDataTimeoutToast,
+      }: HasEsDataParams = {}): Promise<boolean> => {
         if (callResolveCluster) {
-          return hasESDataViaResolveCluster();
+          return hasESDataViaResolveCluster(onRemoteDataTimeout);
         }
         return hasESDataViaResolveIndex();
       },
@@ -82,6 +121,9 @@ export class HasData {
 
   // ES Data
 
+  private isResponseError = (e: Error): e is IHttpFetchError<ResponseErrorBody> =>
+    isHttpFetchError(e) && isObject(e.body) && 'message' in e.body && 'statusCode' in e.body;
+
   private responseToItemArray = (response: IndicesResponse): IndicesResponseModified[] => {
     const { indices = [], aliases = [] } = response;
     const source: IndicesResponseModified[] = [];

diff --git a/src/plugins/data_views/server/index.ts b/src/plugins/data_views/server/index.ts
@@ -47,7 +47,6 @@ const configSchema = schema.object({
     schema.boolean({ defaultValue: false }),
     schema.never()
   ),
-
   dataTiersExcludedForFields: schema.conditional(
     schema.contextRef('serverless'),
     true,
@@ -60,6 +59,7 @@ const configSchema = schema.object({
     schema.boolean({ defaultValue: false }),
     schema.boolean({ defaultValue: true })
   ),
+  hasEsDataTimeout: schema.number({ defaultValue: 5000 }),
 });
 
 type ConfigType = TypeOf<typeof configSchema>;

diff --git a/src/plugins/data_views/server/plugin.ts b/src/plugins/data_views/server/plugin.ts
@@ -63,9 +63,11 @@ export class DataViewsServerPlugin
 
     registerRoutes({
       http: core.http,
+      logger: this.logger,
       getStartServices: core.getStartServices,
       isRollupsEnabled: () => this.rollupsEnabled,
       dataViewRestCounter,
+      hasEsDataTimeout: config.hasEsDataTimeout,
     });
 
     expressions.registerFunction(getIndexPatternLoad({ getStartServices: core.getStartServices }));