From 28b8f404e782f512e8673da0e8007236affdb8b3 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 29 Jun 2022 15:53:25 +0200 Subject: [PATCH 1/8] [ML] Adds API license check. --- x-pack/plugins/aiops/kibana.json | 5 +++- x-pack/plugins/aiops/server/lib/license.ts | 12 ++++++++++ x-pack/plugins/aiops/server/plugin.ts | 23 ++++++++++++++++--- .../server/routes/explain_log_rate_spikes.ts | 7 ++++++ x-pack/plugins/aiops/server/types.ts | 8 ++++++- 5 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugins/aiops/server/lib/license.ts diff --git a/x-pack/plugins/aiops/kibana.json b/x-pack/plugins/aiops/kibana.json index 2d1e60bca74e3..558e5b475c452 100755 --- a/x-pack/plugins/aiops/kibana.json +++ b/x-pack/plugins/aiops/kibana.json @@ -9,7 +9,10 @@ "description": "AIOps plugin maintained by ML team.", "server": true, "ui": true, - "requiredPlugins": ["data"], + "requiredPlugins": [ + "data", + "licensing" + ], "optionalPlugins": [], "requiredBundles": ["kibanaReact"], "extraPublicDirs": ["common"] diff --git a/x-pack/plugins/aiops/server/lib/license.ts b/x-pack/plugins/aiops/server/lib/license.ts new file mode 100644 index 0000000000000..6e2173ab21ef8 --- /dev/null +++ b/x-pack/plugins/aiops/server/lib/license.ts @@ -0,0 +1,12 @@ +/* + * 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 type { ILicense, LicenseType } from '@kbn/licensing-plugin/common/types'; + +export function isActiveLicense(licenseType: LicenseType, license?: ILicense): boolean { + return (license && license.isActive && license.hasAtLeast(licenseType)) || false; +} diff --git a/x-pack/plugins/aiops/server/plugin.ts b/x-pack/plugins/aiops/server/plugin.ts index 56a2a8bb58bab..29b4b79884d8c 100755 --- a/x-pack/plugins/aiops/server/plugin.ts +++ b/x-pack/plugins/aiops/server/plugin.ts @@ -5,12 +5,16 @@ * 2.0. */ +import { Subscription } from 'rxjs'; + import { PluginInitializerContext, CoreSetup, CoreStart, Plugin, Logger } from '@kbn/core/server'; import type { DataRequestHandlerContext } from '@kbn/data-plugin/server'; import { AIOPS_ENABLED } from '../common'; +import { isActiveLicense } from './lib/license'; import { + AiopsLicense, AiopsPluginSetup, AiopsPluginStart, AiopsPluginSetupDeps, @@ -22,19 +26,29 @@ export class AiopsPlugin implements Plugin { private readonly logger: Logger; + private licenseSubscription: Subscription | null = null; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); } - public setup(core: CoreSetup, deps: AiopsPluginSetupDeps) { + public setup(core: CoreSetup, plugins: AiopsPluginSetupDeps) { this.logger.debug('aiops: Setup'); + + // Subscribe to license changes and store the current license in `currentLicense`. + // This way we can pass on license changes to the route factory having always + // the current license because it's stored in a mutable attribute. + const aiopsLicense: AiopsLicense = { isActivePlatinumLicense: false }; + this.licenseSubscription = plugins.licensing.license$.subscribe(async (license) => { + aiopsLicense.isActivePlatinumLicense = isActiveLicense('platinum', license); + }); + const router = core.http.createRouter(); // Register server side APIs if (AIOPS_ENABLED) { core.getStartServices().then(([_, depsStart]) => { - defineExplainLogRateSpikesRoute(router, this.logger); + defineExplainLogRateSpikesRoute(router, aiopsLicense, this.logger); }); } @@ -46,5 +60,8 @@ export class AiopsPlugin return {}; } - public stop() {} + public stop() { + this.logger.debug('aiops: Stop'); + this.licenseSubscription?.unsubscribe(); + } } diff --git a/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts b/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts index 4ab33e06a42c2..ac96937342961 100644 --- a/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts +++ b/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts @@ -20,6 +20,8 @@ import { import { API_ENDPOINT } from '../../common/api'; import type { ChangePoint } from '../../common/types'; +import type { AiopsLicense } from '../types'; + import { fetchFieldCandidates } from './queries/fetch_field_candidates'; import { fetchChangePointPValues } from './queries/fetch_change_point_p_values'; @@ -29,6 +31,7 @@ const PROGRESS_STEP_P_VALUES = 0.8; export const defineExplainLogRateSpikesRoute = ( router: IRouter, + license: AiopsLicense, logger: Logger ) => { router.post( @@ -39,6 +42,10 @@ export const defineExplainLogRateSpikesRoute = ( }, }, async (context, request, response) => { + if (!license.isActivePlatinumLicense) { + response.forbidden(); + } + const client = (await context.core).elasticsearch.client.asCurrentUser; const controller = new AbortController(); diff --git a/x-pack/plugins/aiops/server/types.ts b/x-pack/plugins/aiops/server/types.ts index 3d27a9625db4c..8678fe186d56c 100755 --- a/x-pack/plugins/aiops/server/types.ts +++ b/x-pack/plugins/aiops/server/types.ts @@ -5,10 +5,12 @@ * 2.0. */ -import { PluginSetup, PluginStart } from '@kbn/data-plugin/server'; +import type { PluginSetup, PluginStart } from '@kbn/data-plugin/server'; +import type { LicensingPluginStart } from '@kbn/licensing-plugin/server'; export interface AiopsPluginSetupDeps { data: PluginSetup; + licensing: LicensingPluginStart; } export interface AiopsPluginStartDeps { @@ -26,3 +28,7 @@ export interface AiopsPluginSetup {} */ // eslint-disable-next-line @typescript-eslint/no-empty-interface export interface AiopsPluginStart {} + +export interface AiopsLicense { + isActivePlatinumLicense: boolean; +} From c684889b66f67628d5635adf5cd358a080aa3223 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 09:58:13 +0200 Subject: [PATCH 2/8] [ML] Adds tests to check 403 forbidden response without proper license. --- .../test/api_integration/apis/aiops/index.ts | 2 +- .../api_integration_basic/apis/aiops/index.ts | 17 ++++++ .../apis/aiops/permissions.ts | 57 +++++++++++++++++++ .../test/api_integration_basic/apis/index.ts | 1 + 4 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 x-pack/test/api_integration_basic/apis/aiops/index.ts create mode 100644 x-pack/test/api_integration_basic/apis/aiops/permissions.ts diff --git a/x-pack/test/api_integration/apis/aiops/index.ts b/x-pack/test/api_integration/apis/aiops/index.ts index f202a7e26ddbb..24a0391cf4877 100644 --- a/x-pack/test/api_integration/apis/aiops/index.ts +++ b/x-pack/test/api_integration/apis/aiops/index.ts @@ -11,7 +11,7 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('AIOps', function () { - this.tags(['ml']); + this.tags(['aiops']); if (AIOPS_ENABLED) { loadTestFile(require.resolve('./explain_log_rate_spikes')); diff --git a/x-pack/test/api_integration_basic/apis/aiops/index.ts b/x-pack/test/api_integration_basic/apis/aiops/index.ts new file mode 100644 index 0000000000000..579276afb2a41 --- /dev/null +++ b/x-pack/test/api_integration_basic/apis/aiops/index.ts @@ -0,0 +1,17 @@ +/* + * 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 }: FtrProviderContext) { + describe('aiops basic license', function () { + this.tags(['aiops', 'walterra']); + + // The aiops API should return forbidden when called without a trial/platinum license. + loadTestFile(require.resolve('./permissions')); + }); +} diff --git a/x-pack/test/api_integration_basic/apis/aiops/permissions.ts b/x-pack/test/api_integration_basic/apis/aiops/permissions.ts new file mode 100644 index 0000000000000..8c6686750e2f8 --- /dev/null +++ b/x-pack/test/api_integration_basic/apis/aiops/permissions.ts @@ -0,0 +1,57 @@ +/* + * 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 fetch from 'node-fetch'; +import { format as formatUrl } from 'url'; + +import expect from '@kbn/expect'; + +import type { ApiExplainLogRateSpikes } from '@kbn/aiops-plugin/common/api'; + +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default ({ getService }: FtrProviderContext) => { + const supertest = getService('supertest'); + const config = getService('config'); + const kibanaServerUrl = formatUrl(config.get('servers.kibana')); + + const requestBody: ApiExplainLogRateSpikes['body'] = { + baselineMax: 1561719083292, + baselineMin: 1560954147006, + deviationMax: 1562254538692, + deviationMin: 1561986810992, + end: 2147483647000, + index: 'ft_ecommerce', + kuery: '', + start: 0, + timeFieldName: 'order_date', + }; + + describe('POST /internal/aiops/explain_log_rate_spikes', () => { + it('should return permission denied without streaming', async () => { + await supertest + .post(`/internal/aiops/explain_log_rate_spikes`) + .set('kbn-xsrf', 'kibana') + .send(requestBody) + .expect(403); + }); + + it('should return permission denied with streaming', async () => { + const response = await fetch(`${kibanaServerUrl}/internal/aiops/explain_log_rate_spikes`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'kbn-xsrf': 'stream', + }, + body: JSON.stringify(requestBody), + }); + + expect(response.ok).to.be(false); + expect(response.status).to.be(403); + }); + }); +}; diff --git a/x-pack/test/api_integration_basic/apis/index.ts b/x-pack/test/api_integration_basic/apis/index.ts index 31176c55ac3ca..2e4e91db7d6ef 100644 --- a/x-pack/test/api_integration_basic/apis/index.ts +++ b/x-pack/test/api_integration_basic/apis/index.ts @@ -9,6 +9,7 @@ import { FtrProviderContext } from '../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('apis', function () { + loadTestFile(require.resolve('./aiops')); loadTestFile(require.resolve('./transform')); loadTestFile(require.resolve('./security_solution')); }); From 145047c5d1856d33b5ed88272e206970677cc126 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 10:04:15 +0200 Subject: [PATCH 3/8] [ML] Adds status check. --- .../test/api_integration/apis/aiops/explain_log_rate_spikes.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts b/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts index 015fbc97a3872..6d5789224a683 100644 --- a/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts +++ b/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts @@ -129,6 +129,9 @@ export default ({ getService }: FtrProviderContext) => { body: JSON.stringify(requestBody), }); + expect(response.ok).to.be(true); + expect(response.status).to.be(200); + const stream = response.body; expect(stream).not.to.be(null); From c205a256fcc4a95e5733c852cb6784363a84cdef Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 12:01:37 +0200 Subject: [PATCH 4/8] [ML] Improve API level error handling. --- .../src/lib/stream_factory.test.ts | 84 +++++++++++-------- .../ml/aiops_utils/src/lib/stream_factory.ts | 28 ++++--- .../actions.ts} | 37 ++++---- .../api/explain_log_rate_spikes/index.ts | 17 ++++ .../api/explain_log_rate_spikes/schema.ts | 25 ++++++ .../server/routes/explain_log_rate_spikes.ts | 35 +++++--- .../queries/fetch_change_point_p_values.ts | 6 +- .../routes/queries/fetch_field_candidates.ts | 6 +- .../apis/aiops/explain_log_rate_spikes.ts | 39 +++++++++ .../test/api_integration/apis/aiops/index.ts | 2 +- 10 files changed, 191 insertions(+), 88 deletions(-) rename x-pack/plugins/aiops/common/api/{explain_log_rate_spikes.ts => explain_log_rate_spikes/actions.ts} (64%) create mode 100644 x-pack/plugins/aiops/common/api/explain_log_rate_spikes/index.ts create mode 100644 x-pack/plugins/aiops/common/api/explain_log_rate_spikes/schema.ts diff --git a/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.test.ts b/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.test.ts index 53cd5f332f805..a0c5212244ad6 100644 --- a/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.test.ts +++ b/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.test.ts @@ -7,6 +7,8 @@ import * as zlib from 'zlib'; +import type { Logger } from '@kbn/logging'; + import { streamFactory } from './stream_factory'; interface MockItem { @@ -24,8 +26,14 @@ const mockItem2: MockItem = { }; describe('streamFactory', () => { + let mockLogger: Logger; + + beforeEach(() => { + mockLogger = { error: jest.fn() } as unknown as Logger; + }); + it('should encode and receive an uncompressed string based stream', async () => { - const { end, push, responseWithHeaders } = streamFactory({}); + const { end, push, responseWithHeaders } = streamFactory({}, mockLogger); push('push1'); push('push2'); @@ -41,7 +49,7 @@ describe('streamFactory', () => { }); it('should encode and receive an uncompressed NDJSON based stream', async () => { - const { DELIMITER, end, push, responseWithHeaders } = streamFactory({}); + const { DELIMITER, end, push, responseWithHeaders } = streamFactory({}, mockLogger); push(mockItem1); push(mockItem2); @@ -74,9 +82,12 @@ describe('streamFactory', () => { // without the need for additional custom code. it('should encode and receive a compressed string based stream', (done) => { (async () => { - const { end, push, responseWithHeaders } = streamFactory({ - 'accept-encoding': 'gzip', - }); + const { end, push, responseWithHeaders } = streamFactory( + { + 'accept-encoding': 'gzip', + }, + mockLogger + ); push('push1'); push('push2'); @@ -104,9 +115,12 @@ describe('streamFactory', () => { it('should encode and receive a compressed NDJSON based stream', (done) => { (async () => { - const { DELIMITER, end, push, responseWithHeaders } = streamFactory({ - 'accept-encoding': 'gzip', - }); + const { DELIMITER, end, push, responseWithHeaders } = streamFactory( + { + 'accept-encoding': 'gzip', + }, + mockLogger + ); push(mockItem1); push(mockItem2); @@ -140,49 +154,49 @@ describe('streamFactory', () => { })(); }); - it('should throw when a string based stream receives a non-string chunk', async () => { - const { push } = streamFactory({}); + it('should log an error when a string based stream receives a non-string chunk', async () => { + const { push } = streamFactory({}, mockLogger); // First push initializes the stream as string based. - expect(() => { - push('push1'); - }).not.toThrow(); + push('push1'); + expect(mockLogger.error).toHaveBeenCalledTimes(0); // Second push is again a string and should not throw. - expect(() => { - push('push2'); - }).not.toThrow(); + push('push2'); + expect(mockLogger.error).toHaveBeenCalledTimes(0); // Third push is not a string and should trigger an error. - expect(() => { - push({ myObject: 'push3' } as unknown as string); - }).toThrow('Must not push non-string chunks to a string based stream.'); + push({ myObject: 'push3' } as unknown as string); + expect(mockLogger.error).toHaveBeenCalledTimes(1); + expect(mockLogger.error).toHaveBeenCalledWith( + 'Must not push non-string chunks to a string based stream.' + ); }); - it('should throw when an NDJSON based stream receives a string chunk', async () => { - const { push } = streamFactory({}); + it('should log an error when an NDJSON based stream receives a string chunk', async () => { + const { push } = streamFactory({}, mockLogger); // First push initializes the stream as NDJSON based. - expect(() => { - push(mockItem1); - }).not.toThrow(); + push(mockItem1); + expect(mockLogger.error).toHaveBeenCalledTimes(0); // Second push is again a valid object and should not throw. - expect(() => { - push(mockItem1); - }).not.toThrow(); + push(mockItem1); + expect(mockLogger.error).toHaveBeenCalledTimes(0); // Third push is a string and should trigger an error. - expect(() => { - push('push3' as unknown as MockItem); - }).toThrow('Must not push raw string chunks to an NDJSON based stream.'); + push('push3' as unknown as MockItem); + expect(mockLogger.error).toHaveBeenCalledTimes(1); + expect(mockLogger.error).toHaveBeenCalledWith( + 'Must not push raw string chunks to an NDJSON based stream.' + ); }); - it('should throw for undefined as push value', async () => { - const { push } = streamFactory({}); + it('should log an error for undefined as push value', async () => { + const { push } = streamFactory({}, mockLogger); - expect(() => { - push(undefined as unknown as string); - }).toThrow('Stream chunk must not be undefined.'); + push(undefined as unknown as string); + expect(mockLogger.error).toHaveBeenCalledTimes(1); + expect(mockLogger.error).toHaveBeenCalledWith('Stream chunk must not be undefined.'); }); }); diff --git a/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.ts b/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.ts index 291890138ea40..9df9702eb0870 100644 --- a/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.ts +++ b/x-pack/packages/ml/aiops_utils/src/lib/stream_factory.ts @@ -8,6 +8,8 @@ import { Stream } from 'stream'; import * as zlib from 'zlib'; +import type { Logger } from '@kbn/logging'; + import { acceptCompression } from './accept_compression'; /** @@ -30,7 +32,6 @@ type StreamType = 'string' | 'ndjson'; interface StreamFactoryReturnType { DELIMITER: string; end: () => void; - error: (errorText: string) => void; push: (d: T) => void; responseWithHeaders: { body: zlib.Gzip | ResponseStream; @@ -47,7 +48,10 @@ interface StreamFactoryReturnType { * @param headers - Request headers. * @returns An object with stream attributes and methods. */ -export function streamFactory(headers: Headers): StreamFactoryReturnType; +export function streamFactory( + headers: Headers, + logger: Logger +): StreamFactoryReturnType; /** * Sets up a response stream with support for gzip compression depending on provided * request headers. Any non-string data pushed to the stream will be stream as NDJSON. @@ -55,23 +59,22 @@ export function streamFactory(headers: Headers): StreamFactoryReturn * @param headers - Request headers. * @returns An object with stream attributes and methods. */ -export function streamFactory(headers: Headers): StreamFactoryReturnType { +export function streamFactory( + headers: Headers, + logger: Logger +): StreamFactoryReturnType { let streamType: StreamType; const isCompressed = acceptCompression(headers); const stream = isCompressed ? zlib.createGzip() : new ResponseStream(); - function error(errorText: string) { - stream.emit('error', errorText); - } - function end() { stream.end(); } function push(d: T) { if (d === undefined) { - error('Stream chunk must not be undefined.'); + logger.error('Stream chunk must not be undefined.'); return; } // Initialize the stream type with the first push to the stream, @@ -79,10 +82,10 @@ export function streamFactory(headers: Headers): StreamFactoryRetur if (streamType === undefined) { streamType = typeof d === 'string' ? 'string' : 'ndjson'; } else if (streamType === 'string' && typeof d !== 'string') { - error('Must not push non-string chunks to a string based stream.'); + logger.error('Must not push non-string chunks to a string based stream.'); return; } else if (streamType === 'ndjson' && typeof d === 'string') { - error('Must not push raw string chunks to an NDJSON based stream.'); + logger.error('Must not push raw string chunks to an NDJSON based stream.'); return; } @@ -90,7 +93,8 @@ export function streamFactory(headers: Headers): StreamFactoryRetur const line = typeof d !== 'string' ? `${JSON.stringify(d)}${DELIMITER}` : d; stream.write(line); } catch (e) { - error(`Could not serialize or stream data chunk: ${e.toString()}`); + logger.error(`Could not serialize or stream data chunk: ${e.toString()}`); + return; } // Calling .flush() on a compression stream will @@ -111,5 +115,5 @@ export function streamFactory(headers: Headers): StreamFactoryRetur : {}), }; - return { DELIMITER, end, error, push, responseWithHeaders }; + return { DELIMITER, end, push, responseWithHeaders }; } diff --git a/x-pack/plugins/aiops/common/api/explain_log_rate_spikes.ts b/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/actions.ts similarity index 64% rename from x-pack/plugins/aiops/common/api/explain_log_rate_spikes.ts rename to x-pack/plugins/aiops/common/api/explain_log_rate_spikes/actions.ts index 096851a31bdb0..cd6eb734da503 100644 --- a/x-pack/plugins/aiops/common/api/explain_log_rate_spikes.ts +++ b/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/actions.ts @@ -5,29 +5,11 @@ * 2.0. */ -import { schema, TypeOf } from '@kbn/config-schema'; - -import type { ChangePoint } from '../types'; - -export const aiopsExplainLogRateSpikesSchema = schema.object({ - start: schema.number(), - end: schema.number(), - kuery: schema.string(), - timeFieldName: schema.string(), - includeFrozen: schema.maybe(schema.boolean()), - /** Analysis selection time ranges */ - baselineMin: schema.number(), - baselineMax: schema.number(), - deviationMin: schema.number(), - deviationMax: schema.number(), - /** The index to query for log rate spikes */ - index: schema.string(), -}); - -export type AiopsExplainLogRateSpikesSchema = TypeOf; +import type { ChangePoint } from '../../types'; export const API_ACTION_NAME = { ADD_CHANGE_POINTS: 'add_change_points', + ERROR: 'error', UPDATE_LOADING_STATE: 'update_loading_state', } as const; export type ApiActionName = typeof API_ACTION_NAME[keyof typeof API_ACTION_NAME]; @@ -37,7 +19,7 @@ interface ApiActionAddChangePoints { payload: ChangePoint[]; } -export function addChangePoints( +export function addChangePointsAction( payload: ApiActionAddChangePoints['payload'] ): ApiActionAddChangePoints { return { @@ -46,6 +28,18 @@ export function addChangePoints( }; } +interface ApiActionError { + type: typeof API_ACTION_NAME.ERROR; + payload: string; +} + +export function errorAction(payload: ApiActionError['payload']): ApiActionError { + return { + type: API_ACTION_NAME.ERROR, + payload, + }; +} + interface ApiActionUpdateLoadingState { type: typeof API_ACTION_NAME.UPDATE_LOADING_STATE; payload: { @@ -66,4 +60,5 @@ export function updateLoadingStateAction( export type AiopsExplainLogRateSpikesApiAction = | ApiActionAddChangePoints + | ApiActionError | ApiActionUpdateLoadingState; diff --git a/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/index.ts b/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/index.ts new file mode 100644 index 0000000000000..eb37d6e489be7 --- /dev/null +++ b/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/index.ts @@ -0,0 +1,17 @@ +/* + * 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. + */ + +export { + addChangePointsAction, + errorAction, + updateLoadingStateAction, + API_ACTION_NAME, +} from './actions'; +export type { AiopsExplainLogRateSpikesApiAction } from './actions'; + +export { aiopsExplainLogRateSpikesSchema } from './schema'; +export type { AiopsExplainLogRateSpikesSchema } from './schema'; diff --git a/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/schema.ts b/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/schema.ts new file mode 100644 index 0000000000000..9c4928d310701 --- /dev/null +++ b/x-pack/plugins/aiops/common/api/explain_log_rate_spikes/schema.ts @@ -0,0 +1,25 @@ +/* + * 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 { schema, TypeOf } from '@kbn/config-schema'; + +export const aiopsExplainLogRateSpikesSchema = schema.object({ + start: schema.number(), + end: schema.number(), + kuery: schema.string(), + timeFieldName: schema.string(), + includeFrozen: schema.maybe(schema.boolean()), + /** Analysis selection time ranges */ + baselineMin: schema.number(), + baselineMax: schema.number(), + deviationMin: schema.number(), + deviationMax: schema.number(), + /** The index to query for log rate spikes */ + index: schema.string(), +}); + +export type AiopsExplainLogRateSpikesSchema = TypeOf; diff --git a/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts b/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts index ac96937342961..23b1d7a8af0dc 100644 --- a/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts +++ b/x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts @@ -7,13 +7,15 @@ import { chunk } from 'lodash'; -import type { IRouter, Logger } from '@kbn/core/server'; +import type { IRouter } from '@kbn/core/server'; +import type { Logger } from '@kbn/logging'; import type { DataRequestHandlerContext } from '@kbn/data-plugin/server'; import { streamFactory } from '@kbn/aiops-utils'; import { - addChangePoints, + addChangePointsAction, aiopsExplainLogRateSpikesSchema, + errorAction, updateLoadingStateAction, AiopsExplainLogRateSpikesApiAction, } from '../../common/api/explain_log_rate_spikes'; @@ -43,7 +45,7 @@ export const defineExplainLogRateSpikesRoute = ( }, async (context, request, response) => { if (!license.isActivePlatinumLicense) { - response.forbidden(); + return response.forbidden(); } const client = (await context.core).elasticsearch.client.asCurrentUser; @@ -62,7 +64,8 @@ export const defineExplainLogRateSpikesRoute = ( }); const { end, push, responseWithHeaders } = streamFactory( - request.headers + request.headers, + logger ); // Async IIFE to run the analysis while not blocking returning `responseWithHeaders`. @@ -75,7 +78,14 @@ export const defineExplainLogRateSpikesRoute = ( }) ); - const { fieldCandidates } = await fetchFieldCandidates(client, request.body); + let fieldCandidates: Awaited>; + try { + fieldCandidates = await fetchFieldCandidates(client, request.body); + } catch (e) { + push(errorAction(e.toString())); + end(); + return; + } if (fieldCandidates.length > 0) { loaded += LOADED_FIELD_CANDIDATES; @@ -103,11 +113,14 @@ export const defineExplainLogRateSpikesRoute = ( const fieldCandidatesChunks = chunk(fieldCandidates, chunkSize); for (const fieldCandidatesChunk of fieldCandidatesChunks) { - const { changePoints: pValues } = await fetchChangePointPValues( - client, - request.body, - fieldCandidatesChunk - ); + let pValues: Awaited>; + try { + pValues = await fetchChangePointPValues(client, request.body, fieldCandidatesChunk); + } catch (e) { + push(errorAction(e.toString())); + end(); + return; + } if (pValues.length > 0) { pValues.forEach((d) => { @@ -118,7 +131,7 @@ export const defineExplainLogRateSpikesRoute = ( loaded += (1 / fieldCandidatesChunks.length) * PROGRESS_STEP_P_VALUES; if (pValues.length > 0) { - push(addChangePoints(pValues)); + push(addChangePointsAction(pValues)); } push( updateLoadingStateAction({ diff --git a/x-pack/plugins/aiops/server/routes/queries/fetch_change_point_p_values.ts b/x-pack/plugins/aiops/server/routes/queries/fetch_change_point_p_values.ts index 0504d98ea0ab8..f8b088b76a713 100644 --- a/x-pack/plugins/aiops/server/routes/queries/fetch_change_point_p_values.ts +++ b/x-pack/plugins/aiops/server/routes/queries/fetch_change_point_p_values.ts @@ -90,7 +90,7 @@ export const fetchChangePointPValues = async ( esClient: ElasticsearchClient, params: AiopsExplainLogRateSpikesSchema, fieldNames: string[] -) => { +): Promise => { const result: ChangePoint[] = []; for (const fieldName of fieldNames) { @@ -119,7 +119,5 @@ export const fetchChangePointPValues = async ( } } - return { - changePoints: uniqBy(result, (d) => `${d.fieldName},${d.fieldValue}`), - }; + return uniqBy(result, (d) => `${d.fieldName},${d.fieldValue}`); }; diff --git a/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.ts b/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.ts index df92f370fd175..78d154435c4a6 100644 --- a/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.ts +++ b/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.ts @@ -45,7 +45,7 @@ export const getRandomDocsRequest = ( export const fetchFieldCandidates = async ( esClient: ElasticsearchClient, params: AiopsExplainLogRateSpikesSchema -): Promise<{ fieldCandidates: string[] }> => { +): Promise => { const { index } = params; // Get all supported fields const respMapping = await esClient.fieldCaps({ @@ -78,7 +78,5 @@ export const fetchFieldCandidates = async ( } }); - return { - fieldCandidates: [...finalFieldCandidates], - }; + return [...finalFieldCandidates]; }; diff --git a/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts b/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts index 6d5789224a683..be65531da0b02 100644 --- a/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts +++ b/x-pack/test/api_integration/apis/aiops/explain_log_rate_spikes.ts @@ -36,7 +36,10 @@ export default ({ getService }: FtrProviderContext) => { const expected = { chunksLength: 7, actionsLength: 6, + noIndexChunksLength: 3, + noIndexActionsLength: 2, actionFilter: 'add_change_points', + errorFilter: 'error', changePoints: [ { fieldName: 'day_of_week', @@ -167,5 +170,41 @@ export default ({ getService }: FtrProviderContext) => { }); } }); + + it('should return an error for non existing index without streaming', async () => { + const resp = await supertest + .post(`/internal/aiops/explain_log_rate_spikes`) + .set('kbn-xsrf', 'kibana') + .send({ + ...requestBody, + index: 'does_not_exist', + }) + .expect(200); + + const chunks: string[] = resp.body.toString().split('\n'); + + expect(chunks.length).to.be(expected.noIndexChunksLength); + + const lastChunk = chunks.pop(); + expect(lastChunk).to.be(''); + + let data: any[] = []; + + expect(() => { + data = chunks.map((c) => JSON.parse(c)); + }).not.to.throwError(); + + expect(data.length).to.be(expected.noIndexActionsLength); + data.forEach((d) => { + expect(typeof d.type).to.be('string'); + }); + + const errorActions = data.filter((d) => d.type === expected.errorFilter); + expect(errorActions.length).to.be(1); + + expect(errorActions[0].payload).to.be( + 'ResponseError: index_not_found_exception: [index_not_found_exception] Reason: no such index [does_not_exist]' + ); + }); }); }; diff --git a/x-pack/test/api_integration/apis/aiops/index.ts b/x-pack/test/api_integration/apis/aiops/index.ts index 24a0391cf4877..1c80923e0c4f2 100644 --- a/x-pack/test/api_integration/apis/aiops/index.ts +++ b/x-pack/test/api_integration/apis/aiops/index.ts @@ -11,7 +11,7 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('AIOps', function () { - this.tags(['aiops']); + this.tags(['aiops', 'walterra']); if (AIOPS_ENABLED) { loadTestFile(require.resolve('./explain_log_rate_spikes')); From 8843e6eba3c085b8b1ab45af7aee43a2ffa3fb9d Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 12:42:49 +0200 Subject: [PATCH 5/8] [ML] Fix response_stream plugin error handling. --- .../common/api/reducer_stream/index.ts | 16 +++++++++++++++- .../common/api/reducer_stream/reducer.ts | 7 +++++++ .../app/pages/page_reducer_stream/index.tsx | 8 ++++++++ .../server/routes/reducer_stream.ts | 13 +++++++------ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/examples/response_stream/common/api/reducer_stream/index.ts b/examples/response_stream/common/api/reducer_stream/index.ts index 02e90dd6bd9eb..606834be2d0ba 100644 --- a/examples/response_stream/common/api/reducer_stream/index.ts +++ b/examples/response_stream/common/api/reducer_stream/index.ts @@ -14,6 +14,7 @@ export const API_ACTION_NAME = { UPDATE_PROGRESS: 'update_progress', ADD_TO_ENTITY: 'add_to_entity', DELETE_ENTITY: 'delete_entity', + ERROR: 'error', } as const; export type ApiActionName = typeof API_ACTION_NAME[keyof typeof API_ACTION_NAME]; @@ -59,7 +60,20 @@ export function deleteEntityAction(payload: string): ApiActionDeleteEntity { }; } +interface ApiActionError { + type: typeof API_ACTION_NAME.ERROR; + payload: string; +} + +export function errorAction(payload: string): ApiActionError { + return { + type: API_ACTION_NAME.ERROR, + payload, + }; +} + export type ReducerStreamApiAction = | ApiActionUpdateProgress | ApiActionAddToEntity - | ApiActionDeleteEntity; + | ApiActionDeleteEntity + | ApiActionError; diff --git a/examples/response_stream/common/api/reducer_stream/reducer.ts b/examples/response_stream/common/api/reducer_stream/reducer.ts index f50fce4cd63cb..9896e760cd75b 100644 --- a/examples/response_stream/common/api/reducer_stream/reducer.ts +++ b/examples/response_stream/common/api/reducer_stream/reducer.ts @@ -14,10 +14,12 @@ export const UI_ACTION_NAME = { export type UiActionName = typeof UI_ACTION_NAME[keyof typeof UI_ACTION_NAME]; export interface StreamState { + errors: string[]; progress: number; entities: Record; } export const initialState: StreamState = { + errors: [], progress: 0, entities: {}, }; @@ -64,6 +66,11 @@ export function reducerStreamReducer( ...state, entities: addToEntities, }; + case API_ACTION_NAME.ERROR: + return { + ...state, + errors: [...state.errors, action.payload], + }; case UI_ACTION_NAME.RESET: return initialState; default: diff --git a/examples/response_stream/public/containers/app/pages/page_reducer_stream/index.tsx b/examples/response_stream/public/containers/app/pages/page_reducer_stream/index.tsx index 42d170ee6699d..8f5687db1749d 100644 --- a/examples/response_stream/public/containers/app/pages/page_reducer_stream/index.tsx +++ b/examples/response_stream/public/containers/app/pages/page_reducer_stream/index.tsx @@ -65,12 +65,20 @@ export const PageReducerStream: FC = () => { } }; + // This is for low level errors on the stream/HTTP level. useEffect(() => { if (error) { notifications.toasts.addDanger(error); } }, [error, notifications.toasts]); + // This is for errors on the application level + useEffect(() => { + if (data.errors.length > 0) { + notifications.toasts.addDanger(data.errors[data.errors.length - 1]); + } + }, [data.errors, notifications.toasts]); + const buttonLabel = isRunning ? 'Stop development' : 'Start development'; return ( diff --git a/examples/response_stream/server/routes/reducer_stream.ts b/examples/response_stream/server/routes/reducer_stream.ts index cb6f6f5e1b552..7cc02d9b1a80f 100644 --- a/examples/response_stream/server/routes/reducer_stream.ts +++ b/examples/response_stream/server/routes/reducer_stream.ts @@ -10,6 +10,7 @@ import type { IRouter, Logger } from '@kbn/core/server'; import { streamFactory } from '@kbn/aiops-utils'; import { + errorAction, reducerStreamRequestBodySchema, updateProgressAction, addToEntityAction, @@ -38,8 +39,9 @@ export const defineReducerStreamRoute = (router: IRouter, logger: Logger) => { shouldStop = true; }); - const { end, error, push, responseWithHeaders } = streamFactory( - request.headers + const { end, push, responseWithHeaders } = streamFactory( + request.headers, + logger ); const entities = [ @@ -84,18 +86,17 @@ export const defineReducerStreamRoute = (router: IRouter, logger: Logger) => { push(deleteEntityAction(randomEntity)); } else if (randomAction === 'throw-error') { // Throw an error. It should not crash Kibana! - // It should be caught, logged and passed on as a stream error. + // It should be caught and logged to the Kibana server console. throw new Error('There was a (simulated) server side error!'); } else if (randomAction === 'emit-error') { - // Directly emit an error to the stream, this will not be logged. - error('Error pushed to the stream'); + // Emit an error as a stream action. + push(errorAction('(Simulated) error pushed to the stream')); return; } pushStreamUpdate(); } catch (e) { logger.error(e); - error(e); } }, Math.floor(Math.random() * maxTimeoutMs)); } From 61c78f762836c76ded408db78a0190994f9d0482 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 13:07:53 +0200 Subject: [PATCH 6/8] [ML] Fix types. --- .../response_stream/server/routes/single_string_stream.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/response_stream/server/routes/single_string_stream.ts b/examples/response_stream/server/routes/single_string_stream.ts index 97550b5630cbf..dd3a784314c96 100644 --- a/examples/response_stream/server/routes/single_string_stream.ts +++ b/examples/response_stream/server/routes/single_string_stream.ts @@ -35,7 +35,7 @@ export const defineSimpleStringStreamRoute = (router: IRouter, logger: Logger) = shouldStop = true; }); - const { end, error, push, responseWithHeaders } = streamFactory(request.headers); + const { end, push, responseWithHeaders } = streamFactory(request.headers, logger); const text = 'Elasticsearch is a search engine based on the Lucene library. It provides a distributed, multitenant-capable full-text search engine with an HTTP web interface and schema-free JSON documents. Elasticsearch is developed in Java and is dual-licensed under the source-available Server Side Public License and the Elastic license, while other parts fall under the proprietary (source-available) Elastic License. Official clients are available in Java, .NET (C#), PHP, Python, Apache Groovy, Ruby and many other languages. According to the DB-Engines ranking, Elasticsearch is the most popular enterprise search engine.'; @@ -62,7 +62,7 @@ export const defineSimpleStringStreamRoute = (router: IRouter, logger: Logger) = end(); } } catch (e) { - error(`There was an error: ${e.toString()}`); + logger.error(`There was an error: ${e.toString()}`); } } From 52c134308784e818d5808e78c3dbd1c0964e2e0f Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 13:41:09 +0200 Subject: [PATCH 7/8] [ML] Fix test tags. --- x-pack/test/api_integration/apis/aiops/index.ts | 2 +- x-pack/test/api_integration_basic/apis/aiops/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/api_integration/apis/aiops/index.ts b/x-pack/test/api_integration/apis/aiops/index.ts index 1c80923e0c4f2..24a0391cf4877 100644 --- a/x-pack/test/api_integration/apis/aiops/index.ts +++ b/x-pack/test/api_integration/apis/aiops/index.ts @@ -11,7 +11,7 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('AIOps', function () { - this.tags(['aiops', 'walterra']); + this.tags(['aiops']); if (AIOPS_ENABLED) { loadTestFile(require.resolve('./explain_log_rate_spikes')); diff --git a/x-pack/test/api_integration_basic/apis/aiops/index.ts b/x-pack/test/api_integration_basic/apis/aiops/index.ts index 579276afb2a41..d3748f0afe299 100644 --- a/x-pack/test/api_integration_basic/apis/aiops/index.ts +++ b/x-pack/test/api_integration_basic/apis/aiops/index.ts @@ -9,7 +9,7 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('aiops basic license', function () { - this.tags(['aiops', 'walterra']); + this.tags(['aiops']); // The aiops API should return forbidden when called without a trial/platinum license. loadTestFile(require.resolve('./permissions')); From 1cc28d4b0068b954de0f3a052e5483aa61c0ad65 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 30 Jun 2022 14:29:43 +0200 Subject: [PATCH 8/8] [ML] Fix test. --- .../server/routes/queries/fetch_field_candidates.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.test.ts b/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.test.ts index 467a24c8993e4..24bbda6a5d22e 100644 --- a/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.test.ts +++ b/x-pack/plugins/aiops/server/routes/queries/fetch_field_candidates.test.ts @@ -95,9 +95,7 @@ describe('query_field_candidates', () => { const resp = await fetchFieldCandidates(esClientMock, params); - expect(resp).toEqual({ - fieldCandidates: ['myIpFieldName', 'myKeywordFieldName'], - }); + expect(resp).toEqual(['myIpFieldName', 'myKeywordFieldName']); expect(esClientFieldCapsMock).toHaveBeenCalledTimes(1); expect(esClientSearchMock).toHaveBeenCalledTimes(1); });