Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [Data Views] Mitigate issue where `has_es_data` check can cause Kibana to hang (#200476) #201025

Merged
merged 1 commit into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/plugins/data_views/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}
1 change: 1 addition & 0 deletions src/plugins/data_views/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data_views/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,5 @@ export interface ClientConfigType {
scriptedFieldsEnabled?: boolean;
dataTiersExcludedForFields?: string;
fieldListCachingEnabled?: boolean;
hasEsDataTimeout: number;
}
73 changes: 73 additions & 0 deletions src/plugins/data_views/public/services/has_data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
56 changes: 49 additions & 7 deletions src/plugins/data_views/public/services/has_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
},
Expand All @@ -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[] = [];
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data_views/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const configSchema = schema.object({
schema.boolean({ defaultValue: false }),
schema.never()
),

dataTiersExcludedForFields: schema.conditional(
schema.contextRef('serverless'),
true,
Expand All @@ -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>;
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data_views/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
Expand Down
Loading