From d6ac415fb8266b2257c6b48b18de26e0f3c7261f Mon Sep 17 00:00:00 2001 From: Dmitry Shevchenko Date: Wed, 24 Nov 2021 18:44:27 +0100 Subject: [PATCH] Rule execution log performance optimizations (#118925) --- .../__mocks__/rule_execution_log_client.ts | 1 - .../event_log_adapter/event_log_adapter.ts | 3 +- .../rule_execution_log_client.ts | 5 - .../saved_objects_adapter.ts | 171 +++++++----------- .../rule_execution_log/types.ts | 1 - .../preview_rule_execution_log_client.ts | 5 - 6 files changed, 66 insertions(+), 120 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/__mocks__/rule_execution_log_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/__mocks__/rule_execution_log_client.ts index 518e4aa903d95..c01d6afa95190 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/__mocks__/rule_execution_log_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/__mocks__/rule_execution_log_client.ts @@ -19,7 +19,6 @@ export const ruleExecutionLogClientMock = { deleteCurrentStatus: jest.fn(), logStatusChange: jest.fn(), - logExecutionMetrics: jest.fn(), }), }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_adapter.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_adapter.ts index 8b55339aa9f02..f622ea9248b24 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_adapter.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_adapter.ts @@ -79,9 +79,8 @@ export class EventLogAdapter implements IRuleExecutionLogClient { // EventLog execution events are immutable, nothing to do here } - public async logExecutionMetrics(args: LogExecutionMetricsArgs) { + private async logExecutionMetrics(args: LogExecutionMetricsArgs) { const { ruleId, spaceId, ruleType, ruleName, metrics } = args; - await this.savedObjectsAdapter.logExecutionMetrics(args); this.eventLogClient.logExecutionMetrics({ ruleId, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_execution_log_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_execution_log_client.ts index 005097ac3fd82..bc77c6e44fa56 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_execution_log_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_execution_log_client.ts @@ -11,7 +11,6 @@ import { IRuleStatusSOAttributes } from '../rules/types'; import { EventLogAdapter } from './event_log_adapter/event_log_adapter'; import { SavedObjectsAdapter } from './saved_objects_adapter/saved_objects_adapter'; import { - LogExecutionMetricsArgs, FindBulkExecutionLogArgs, FindExecutionLogArgs, IRuleExecutionLogClient, @@ -75,10 +74,6 @@ export class RuleExecutionLogClient implements IRuleExecutionLogClient { return this.client.deleteCurrentStatus(ruleId); } - public async logExecutionMetrics(args: LogExecutionMetricsArgs) { - return this.client.logExecutionMetrics(args); - } - public async logStatusChange(args: LogStatusChangeArgs) { const message = args.message ? truncateMessage(args.message) : args.message; return this.client.logStatusChange({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/saved_objects_adapter/saved_objects_adapter.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/saved_objects_adapter/saved_objects_adapter.ts index 53b50bb8fe638..230306c453ace 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/saved_objects_adapter/saved_objects_adapter.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/saved_objects_adapter/saved_objects_adapter.ts @@ -11,28 +11,26 @@ import { SavedObjectsClientContract } from '../../../../../../../../src/core/ser import { RuleExecutionStatus } from '../../../../../common/detection_engine/schemas/common/schemas'; // eslint-disable-next-line no-restricted-imports import { legacyGetRuleReference } from '../../rules/legacy_rule_status/legacy_utils'; - import { IRuleStatusSOAttributes } from '../../rules/types'; import { - RuleStatusSavedObjectsClient, - ruleStatusSavedObjectsClientFactory, -} from './rule_status_saved_objects_client'; -import { - LogExecutionMetricsArgs, + ExecutionMetrics, FindBulkExecutionLogArgs, FindExecutionLogArgs, - IRuleExecutionLogClient, - ExecutionMetrics, - LogStatusChangeArgs, - GetLastFailuresArgs, GetCurrentStatusArgs, GetCurrentStatusBulkArgs, GetCurrentStatusBulkResult, + GetLastFailuresArgs, + IRuleExecutionLogClient, + LogStatusChangeArgs, } from '../types'; -import { assertUnreachable } from '../../../../../common'; +import { + RuleStatusSavedObjectsClient, + ruleStatusSavedObjectsClientFactory, +} from './rule_status_saved_objects_client'; +const MAX_ERRORS = 5; // 1st is mutable status, followed by 5 most recent failures -export const MAX_RULE_STATUSES = 6; +const MAX_RULE_STATUSES = 1 + MAX_ERRORS; const convertMetricFields = ( metrics: ExecutionMetrics @@ -100,112 +98,73 @@ export class SavedObjectsAdapter implements IRuleExecutionLogClient { await Promise.all(statusSavedObjects.map((so) => this.ruleStatusClient.delete(so.id))); } - public async logExecutionMetrics({ ruleId, metrics }: LogExecutionMetricsArgs) { - const references: SavedObjectReference[] = [legacyGetRuleReference(ruleId)]; - const [currentStatus] = await this.getOrCreateRuleStatuses(ruleId); - - await this.ruleStatusClient.update( - currentStatus.id, - { - ...currentStatus.attributes, - ...convertMetricFields(metrics), - }, - { references } - ); - } - - private createNewRuleStatus = async ( + private findRuleStatuses = async ( ruleId: string - ): Promise> => { - const references: SavedObjectReference[] = [legacyGetRuleReference(ruleId)]; - const now = new Date().toISOString(); - return this.ruleStatusClient.create( - { - statusDate: now, - status: RuleExecutionStatus['going to run'], - lastFailureAt: null, - lastSuccessAt: null, - lastFailureMessage: null, - lastSuccessMessage: null, - gap: null, - bulkCreateTimeDurations: [], - searchAfterTimeDurations: [], - lastLookBackDate: null, - }, - { references } - ); - }; - - private getOrCreateRuleStatuses = async ( - ruleId: string - ): Promise>> => { - const existingStatuses = await this.findRuleStatusSavedObjects(ruleId, MAX_RULE_STATUSES); - if (existingStatuses.length > 0) { - return existingStatuses; - } - - const newStatus = await this.createNewRuleStatus(ruleId); - return [newStatus]; - }; + ): Promise>> => + this.findRuleStatusSavedObjects(ruleId, MAX_RULE_STATUSES); public async logStatusChange({ newStatus, ruleId, message, metrics }: LogStatusChangeArgs) { const references: SavedObjectReference[] = [legacyGetRuleReference(ruleId)]; + const ruleStatuses = await this.findRuleStatuses(ruleId); + const [currentStatus] = ruleStatuses; + const attributes = buildRuleStatusAttributes({ + status: newStatus, + message, + metrics, + currentAttributes: currentStatus?.attributes, + }); + // Create or update current status + if (currentStatus) { + await this.ruleStatusClient.update(currentStatus.id, attributes, { references }); + } else { + await this.ruleStatusClient.create(attributes, { references }); + } - switch (newStatus) { - case RuleExecutionStatus['going to run']: - case RuleExecutionStatus.succeeded: - case RuleExecutionStatus.warning: - case RuleExecutionStatus['partial failure']: { - const [currentStatus] = await this.getOrCreateRuleStatuses(ruleId); - - await this.ruleStatusClient.update( - currentStatus.id, - { - ...currentStatus.attributes, - ...buildRuleStatusAttributes(newStatus, message, metrics), - }, - { references } - ); - - return; - } - - case RuleExecutionStatus.failed: { - const ruleStatuses = await this.getOrCreateRuleStatuses(ruleId); - const [currentStatus] = ruleStatuses; - - const failureAttributes = { - ...currentStatus.attributes, - ...buildRuleStatusAttributes(RuleExecutionStatus.failed, message, metrics), - }; - - // We always update the newest status, so to 'persist' a failure we push a copy to the head of the list - await this.ruleStatusClient.update(currentStatus.id, failureAttributes, { references }); - const lastStatus = await this.ruleStatusClient.create(failureAttributes, { references }); - - // drop oldest failures - const oldStatuses = [lastStatus, ...ruleStatuses].slice(MAX_RULE_STATUSES); - await Promise.all(oldStatuses.map((status) => this.ruleStatusClient.delete(status.id))); - - return; - } - default: - assertUnreachable(newStatus, 'Unknown rule execution status supplied to logStatusChange'); + if (newStatus === RuleExecutionStatus.failed) { + await Promise.all([ + // Persist the current failure in the last five errors list + this.ruleStatusClient.create(attributes, { references }), + // Drop oldest failures + ...ruleStatuses + .slice(MAX_RULE_STATUSES - 1) + .map((status) => this.ruleStatusClient.delete(status.id)), + ]); } } } -const buildRuleStatusAttributes: ( - status: RuleExecutionStatus, - message?: string, - metrics?: ExecutionMetrics -) => Partial = (status, message, metrics = {}) => { +const defaultStatusAttributes: IRuleStatusSOAttributes = { + statusDate: '', + status: RuleExecutionStatus['going to run'], + lastFailureAt: null, + lastSuccessAt: null, + lastFailureMessage: null, + lastSuccessMessage: null, + gap: null, + bulkCreateTimeDurations: [], + searchAfterTimeDurations: [], + lastLookBackDate: null, +}; + +const buildRuleStatusAttributes = ({ + status, + message, + metrics = {}, + currentAttributes, +}: { + status: RuleExecutionStatus; + message?: string; + metrics?: ExecutionMetrics; + currentAttributes?: IRuleStatusSOAttributes; +}): IRuleStatusSOAttributes => { const now = new Date().toISOString(); - const baseAttributes: Partial = { - ...convertMetricFields(metrics), + const baseAttributes: IRuleStatusSOAttributes = { + ...defaultStatusAttributes, + ...currentAttributes, + statusDate: now, status: status === RuleExecutionStatus.warning ? RuleExecutionStatus['partial failure'] : status, - statusDate: now, + ...convertMetricFields(metrics), }; switch (status) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/types.ts index 88802f9f28822..06a0b90985af7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/types.ts @@ -28,7 +28,6 @@ export interface IRuleExecutionLogClient { deleteCurrentStatus(ruleId: string): Promise; logStatusChange(args: LogStatusChangeArgs): Promise; - logExecutionMetrics(args: LogExecutionMetricsArgs): Promise; } /** @deprecated */ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/preview/preview_rule_execution_log_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/preview/preview_rule_execution_log_client.ts index c2c1b5d7615c2..5ab32f27c349e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/preview/preview_rule_execution_log_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/preview/preview_rule_execution_log_client.ts @@ -9,7 +9,6 @@ import { SavedObjectsFindResult } from 'kibana/server'; import { IRuleExecutionLogClient, LogStatusChangeArgs, - LogExecutionMetricsArgs, FindBulkExecutionLogArgs, FindBulkExecutionLogResponse, FindExecutionLogArgs, @@ -65,10 +64,6 @@ export const createWarningsAndErrors = () => { warningsAndErrorsStore.push(args); return Promise.resolve(); }, - - logExecutionMetrics(args: LogExecutionMetricsArgs): Promise { - return Promise.resolve(); - }, }; return { previewRuleExecutionLogClient, warningsAndErrorsStore };