From 3cdf41e09e0aaff43ed2bc1f4717da8be9acda21 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Mon, 5 Dec 2022 19:31:53 +0100 Subject: [PATCH 01/34] Expand RuleMonitoringService --- x-pack/plugins/alerting/common/rule.ts | 2 +- .../plugins/alerting/server/lib/monitoring.ts | 14 ++++++- .../monitoring/rule_monitoring_service.ts | 41 ++++++++++++++++--- x-pack/plugins/alerting/server/types.ts | 1 + .../client_for_executors/client.ts | 7 +++- .../logic/rule_execution_log/service.ts | 5 ++- .../rule_execution_log/service_interface.ts | 2 + .../create_security_rule_type_wrapper.ts | 3 +- 8 files changed, 64 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index 3bfddd79af10d..d59b8316997bb 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -92,7 +92,7 @@ export interface RuleAggregations { } export interface RuleLastRun { - outcome: RuleLastRunOutcomes; + outcome: RuleLastRunOutcomes | null; warning?: RuleExecutionStatusErrorReasons | RuleExecutionStatusWarningReasons | null; outcomeMsg?: string[] | null; alertsCount: { diff --git a/x-pack/plugins/alerting/server/lib/monitoring.ts b/x-pack/plugins/alerting/server/lib/monitoring.ts index 93da6e2283152..fd27f786d1672 100644 --- a/x-pack/plugins/alerting/server/lib/monitoring.ts +++ b/x-pack/plugins/alerting/server/lib/monitoring.ts @@ -6,7 +6,7 @@ */ import { Logger } from '@kbn/core/server'; import stats from 'stats-lite'; -import { RuleMonitoring, RawRuleMonitoring, RuleMonitoringHistory } from '../types'; +import { RuleMonitoring, RawRuleMonitoring, RuleMonitoringHistory, RuleLastRun } from '../types'; export const INITIAL_METRICS = { total_search_duration_ms: null, @@ -31,6 +31,18 @@ export const getDefaultMonitoring = (timestamp: string): RawRuleMonitoring => { }; }; +export const getDefaultLastRun = (): RuleLastRun => ({ + warning: null, + outcome: null, + outcomeMsg: null, + alertsCount: { + active: null, + new: null, + recovered: null, + ignored: null, + } +}) + export const getExecutionDurationPercentiles = (history: RuleMonitoringHistory[]) => { const durationSamples = history.reduce((duration, historyItem) => { if (typeof historyItem.duration === 'number') { diff --git a/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts b/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts index 0043f47c51633..93554178ee36d 100644 --- a/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts +++ b/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts @@ -5,12 +5,12 @@ * 2.0. */ -import { getDefaultMonitoring, getExecutionDurationPercentiles } from '../lib/monitoring'; -import { RuleMonitoring, RuleMonitoringHistory, PublicRuleMonitoringService } from '../types'; - +import { getDefaultLastRun, getDefaultMonitoring, getExecutionDurationPercentiles } from '../lib/monitoring'; +import { RuleMonitoring, RuleMonitoringHistory, PublicRuleMonitoringService, RuleLastRun, RuleLastRunOutcomes, PublicMetricsSetters, PublicLastRunSetters } from '../types'; export class RuleMonitoringService { private monitoring: RuleMonitoring = getDefaultMonitoring(new Date().toISOString()); - + private lastRun: RuleLastRun = getDefaultLastRun(); + public setLastRunMetricsDuration(duration: number) { this.monitoring.run.last_run.metrics.duration = duration; } @@ -25,6 +25,10 @@ export class RuleMonitoringService { return this.monitoring; } + public getLastRun(): RuleLastRun { + return this.lastRun; + } + public addHistory({ duration, hasError = true, @@ -54,7 +58,7 @@ export class RuleMonitoringService { }; } - public getLastRunMetricsSetters(): PublicRuleMonitoringService { + public getLastRunMetricsSetters(): PublicMetricsSetters { return { setLastRunMetricsTotalSearchDurationMs: this.setLastRunMetricsTotalSearchDurationMs.bind(this), @@ -86,6 +90,33 @@ export class RuleMonitoringService { this.monitoring.run.last_run.metrics.gap_duration_s = gapDurationS; } + public getLastRunSetters(): PublicLastRunSetters { + return { + setLastRunOutcome: this.setLastRunOutcome.bind(this), + setLastRunOutcomeMsg: this.setLastRunOutcomeMsg.bind(this), + setLastRunWarning: this.setLastRunWarning.bind(this), + }; + } + + private setLastRunOutcome(outcome: RuleLastRunOutcomes) { + this.lastRun.outcome = outcome; + } + + private setLastRunOutcomeMsg(outcomeMsg: string) { + this.lastRun.outcomeMsg = outcomeMsg; + } + + private setLastRunWarning(warning: RuleLastRun['warning']) { + this.lastRun.warning = warning; + } + + public getRuleMonitoringSetters(): PublicRuleMonitoringService { + return { + ...this.getLastRunMetricsSetters(), + ...this.getLastRunSetters(), + } + } + private buildExecutionSuccessRatio() { const { history } = this.monitoring.run; return history.filter(({ success }) => success).length / history.length; diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index 4092b9209432a..bbf7bf52b9e52 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -48,6 +48,7 @@ import { RuleSnooze, IntervalSchedule, RuleLastRun, + RuleLastRunOutcomes, } from '../common'; import { PublicAlertFactory } from './alert/create_alert_factory'; import { FieldMap } from '../common/alert_schema/field_maps/types'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 3c712847851fd..d94f1f1b9ee06 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -35,13 +35,15 @@ import type { RuleExecutionContext, StatusChangeArgs, } from './client_interface'; +import { PublicRuleMonitoringService } from '@kbn/alerting-plugin/server/types'; export const createClientForExecutors = ( settings: RuleExecutionSettings, soClient: IRuleExecutionSavedObjectsClient, eventLog: IEventLogWriter, logger: Logger, - context: RuleExecutionContext + context: RuleExecutionContext, + ruleMonitoringService: PublicRuleMonitoringService ): IRuleExecutionLogForExecutors => { const baseCorrelationIds = getCorrelationIds(context); const baseLogSuffix = baseCorrelationIds.getLogSuffix(); @@ -163,6 +165,9 @@ export const createClientForExecutors = ( ): Promise => { const { newStatus, message, metrics } = args; + ruleMonitoringService.setLastRunOutcomeMsg("cualquiera gato"); + debugger; + await soClient.createOrUpdate(ruleId, { last_execution: { date: nowISO(), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts index db5aefdaf7370..3f9860d9f44df 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts @@ -53,7 +53,7 @@ export const createRuleExecutionLogService = ( params: ClientForExecutorsParams ): Promise => { return withSecuritySpan('IRuleExecutionLogService.createClientForExecutors', async () => { - const { savedObjectsClient, context } = params; + const { savedObjectsClient, context, ruleMonitoringService } = params; const childLogger = logger.get('ruleExecution'); @@ -72,7 +72,8 @@ export const createRuleExecutionLogService = ( soClient, eventLogWriter, childLogger, - context + context, + ruleMonitoringService ); }); }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts index 27207ea2afde0..dae51b451967f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts @@ -13,6 +13,7 @@ import type { IRuleExecutionLogForExecutors, RuleExecutionContext, } from './client_for_executors/client_interface'; +import { PublicRuleMonitoringService } from '@kbn/alerting-plugin/server/types'; export interface IRuleExecutionLogService { registerEventLogProvider(): void; @@ -31,5 +32,6 @@ export interface ClientForRoutesParams { export interface ClientForExecutorsParams { savedObjectsClient: SavedObjectsClientContract; + ruleMonitoringService: PublicRuleMonitoringService; context: RuleExecutionContext; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts index 9c6b80f80fd79..14e2b9495996a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts @@ -88,6 +88,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = savedObjectsClient, scopedClusterClient, uiSettingsClient, + ruleMonitoringService, } = services; const searchAfterSize = Math.min(maxSignals, DEFAULT_SEARCH_AFTER_PAGE_SIZE); @@ -95,6 +96,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = const ruleExecutionLogger = await ruleExecutionLoggerFactory({ savedObjectsClient, + ruleMonitoringService, context: { executionId, ruleId: rule.id, @@ -342,7 +344,6 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = secondaryTimestamp, ruleExecutionLogger, aggregatableTimestampField, - alertTimestampOverride, }, }); From f9c144a17bf19c8211985d11045c7b3177d4e2a0 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Mon, 5 Dec 2022 20:20:05 +0100 Subject: [PATCH 02/34] Make PublicRuleMonitoringService; not optional --- x-pack/plugins/alerting/server/types.ts | 1 - .../rule_types/create_security_rule_type_wrapper.ts | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index bbf7bf52b9e52..4092b9209432a 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -48,7 +48,6 @@ import { RuleSnooze, IntervalSchedule, RuleLastRun, - RuleLastRunOutcomes, } from '../common'; import { PublicAlertFactory } from './alert/create_alert_factory'; import { FieldMap } from '../common/alert_schema/field_maps/types'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts index 14e2b9495996a..6f6cd67b86011 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts @@ -344,6 +344,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = secondaryTimestamp, ruleExecutionLogger, aggregatableTimestampField, + alertTimestampOverride, }, }); From 6d18a9184bdc64bcb7f92be25fde4abfd5667fec Mon Sep 17 00:00:00 2001 From: jpdjere Date: Mon, 5 Dec 2022 20:25:42 +0100 Subject: [PATCH 03/34] Removed debuggers --- .../logic/rule_execution_log/client_for_executors/client.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index d94f1f1b9ee06..a0228b6ce5454 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -165,8 +165,7 @@ export const createClientForExecutors = ( ): Promise => { const { newStatus, message, metrics } = args; - ruleMonitoringService.setLastRunOutcomeMsg("cualquiera gato"); - debugger; + ruleMonitoringService.setLastRunOutcomeMsg("outcome message"); await soClient.createOrUpdate(ruleId, { last_execution: { From 1adb6e75f066c5f134184361718185561acd5b57 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Tue, 6 Dec 2022 12:20:59 +0100 Subject: [PATCH 04/34] Updated ruleexecutionlogger.logStatusChange to use rule monitoring service Reverted changes to event logging Delete unused methods from service Added 'unknown' as possible rule last run outcome refactored merge_rule_execution_summary.ts --- x-pack/plugins/alerting/common/rule.ts | 4 +- .../plugins/alerting/server/lib/monitoring.ts | 6 +- .../monitoring/rule_monitoring_service.ts | 20 ++++-- .../rule_monitoring/model/execution_status.ts | 19 +++++ .../rule_monitoring/model/log_level.ts | 19 ++++- .../client_for_executors/client.ts | 69 +++++++++++++------ .../merge_rule_execution_summary.ts | 34 +++++---- .../logic/rule_execution_log/service.ts | 2 - 8 files changed, 128 insertions(+), 45 deletions(-) diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index d59b8316997bb..43688b51b82f5 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -32,7 +32,7 @@ export const RuleExecutionStatusValues = [ ] as const; export type RuleExecutionStatuses = typeof RuleExecutionStatusValues[number]; -export const RuleLastRunOutcomeValues = ['succeeded', 'warning', 'failed'] as const; +export const RuleLastRunOutcomeValues = ['succeeded', 'warning', 'failed', 'unknown'] as const; export type RuleLastRunOutcomes = typeof RuleLastRunOutcomeValues[number]; export enum RuleExecutionStatusErrorReasons { @@ -92,7 +92,7 @@ export interface RuleAggregations { } export interface RuleLastRun { - outcome: RuleLastRunOutcomes | null; + outcome: RuleLastRunOutcomes; warning?: RuleExecutionStatusErrorReasons | RuleExecutionStatusWarningReasons | null; outcomeMsg?: string[] | null; alertsCount: { diff --git a/x-pack/plugins/alerting/server/lib/monitoring.ts b/x-pack/plugins/alerting/server/lib/monitoring.ts index fd27f786d1672..fc607952534f6 100644 --- a/x-pack/plugins/alerting/server/lib/monitoring.ts +++ b/x-pack/plugins/alerting/server/lib/monitoring.ts @@ -33,15 +33,15 @@ export const getDefaultMonitoring = (timestamp: string): RawRuleMonitoring => { export const getDefaultLastRun = (): RuleLastRun => ({ warning: null, - outcome: null, + outcome: 'unknown', outcomeMsg: null, alertsCount: { active: null, new: null, recovered: null, ignored: null, - } -}) + }, +}); export const getExecutionDurationPercentiles = (history: RuleMonitoringHistory[]) => { const durationSamples = history.reduce((duration, historyItem) => { diff --git a/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts b/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts index 93554178ee36d..e5e67309cf250 100644 --- a/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts +++ b/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts @@ -5,12 +5,24 @@ * 2.0. */ -import { getDefaultLastRun, getDefaultMonitoring, getExecutionDurationPercentiles } from '../lib/monitoring'; -import { RuleMonitoring, RuleMonitoringHistory, PublicRuleMonitoringService, RuleLastRun, RuleLastRunOutcomes, PublicMetricsSetters, PublicLastRunSetters } from '../types'; +import { + getDefaultLastRun, + getDefaultMonitoring, + getExecutionDurationPercentiles, +} from '../lib/monitoring'; +import { + RuleMonitoring, + RuleMonitoringHistory, + PublicRuleMonitoringService, + RuleLastRun, + RuleLastRunOutcomes, + PublicMetricsSetters, + PublicLastRunSetters, +} from '../types'; export class RuleMonitoringService { private monitoring: RuleMonitoring = getDefaultMonitoring(new Date().toISOString()); private lastRun: RuleLastRun = getDefaultLastRun(); - + public setLastRunMetricsDuration(duration: number) { this.monitoring.run.last_run.metrics.duration = duration; } @@ -114,7 +126,7 @@ export class RuleMonitoringService { return { ...this.getLastRunMetricsSetters(), ...this.getLastRunSetters(), - } + }; } private buildExecutionSuccessRatio() { diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts index 22e599b18c541..8fa47be3ad6bb 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts @@ -7,6 +7,7 @@ import type * as t from 'io-ts'; import { enumeration, PositiveInteger } from '@kbn/securitysolution-io-ts-types'; +import type { RuleLastRunOutcomes } from '@kbn/alerting-plugin/common'; import { assertUnreachable } from '../../../utility_types'; /** @@ -78,3 +79,21 @@ export const ruleExecutionStatusToNumber = ( return 0; } }; + +export const ruleLastRunOutcomeToNumber = ( + outcome: RuleLastRunOutcomes +): RuleExecutionStatusOrder => { + switch (outcome) { + case 'succeeded': + return 0; + case 'warning': + return 20; + case 'failed': + return 30; + case 'unknown': + return 40; // TBC + default: + assertUnreachable(outcome); + return 0; + } +}; diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts index b37ce62ad4891..fb5356092b65c 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts @@ -6,11 +6,12 @@ */ import { enumeration } from '@kbn/securitysolution-io-ts-types'; +import type { RuleLastRunOutcomes } from '@kbn/alerting-plugin/common'; import { enumFromString } from '../../../utils/enum_from_string'; import { assertUnreachable } from '../../../utility_types'; import { RuleExecutionStatus } from './execution_status'; -export enum LogLevel { +export enum LogLevel { 'trace' = 'trace', 'debug' = 'debug', 'info' = 'info', @@ -80,3 +81,19 @@ export const logLevelFromExecutionStatus = (status: RuleExecutionStatus): LogLev return LogLevel.trace; } }; + +export const logLevelFromLastRunOutcome = (status: RuleLastRunOutcomes): LogLevel => { + switch (status) { + case 'succeeded': + return LogLevel.info; + case 'warning': + return LogLevel.warn; + case 'failed': + return LogLevel.error; + case 'unknown': + return LogLevel.trace; + default: + assertUnreachable(status); + return LogLevel.trace; + } +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index a0228b6ce5454..bf3c35c52844d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -9,17 +9,20 @@ import { sum } from 'lodash'; import type { Duration } from 'moment'; import type { Logger } from '@kbn/core/server'; +import type { + PublicRuleMonitoringService, + RuleLastRunOutcomes, +} from '@kbn/alerting-plugin/server/types'; import type { RuleExecutionMetrics, RuleExecutionSettings, - RuleExecutionStatus, } from '../../../../../../../common/detection_engine/rule_monitoring'; import { LogLevel, logLevelFromExecutionStatus, LogLevelSetting, logLevelToNumber, - ruleExecutionStatusToNumber, + RuleExecutionStatus, } from '../../../../../../../common/detection_engine/rule_monitoring'; import { assertUnreachable } from '../../../../../../../common/utility_types'; @@ -29,17 +32,14 @@ import type { ExtMeta } from '../utils/console_logging'; import { getCorrelationIds } from './correlation_ids'; import type { IEventLogWriter } from '../event_log/event_log_writer'; -import type { IRuleExecutionSavedObjectsClient } from '../execution_saved_object/saved_objects_client'; import type { IRuleExecutionLogForExecutors, RuleExecutionContext, StatusChangeArgs, } from './client_interface'; -import { PublicRuleMonitoringService } from '@kbn/alerting-plugin/server/types'; export const createClientForExecutors = ( settings: RuleExecutionSettings, - soClient: IRuleExecutionSavedObjectsClient, eventLog: IEventLogWriter, logger: Logger, context: RuleExecutionContext, @@ -86,7 +86,7 @@ export const createClientForExecutors = ( await Promise.all([ writeStatusChangeToConsole(normalizedArgs, logMeta), - writeStatusChangeToSavedObjects(normalizedArgs), + writeStatusChangeToMonitoringService(normalizedArgs), writeStatusChangeToEventLog(normalizedArgs), ]); } catch (e) { @@ -159,23 +159,37 @@ export const createClientForExecutors = ( writeMessageToConsole(logMessage, logLevel, logMeta); }; - // TODO: Add executionId to new status SO? - const writeStatusChangeToSavedObjects = async ( + const writeStatusChangeToMonitoringService = async ( args: NormalizedStatusChangeArgs ): Promise => { const { newStatus, message, metrics } = args; + const { + total_search_duration_ms: totalSearchDurationMs, + total_indexing_duration_ms: totalIndexingDurationMs, + execution_gap_duration_s: executionGapDurationS, + } = metrics ?? {}; + + if ( + newStatus !== RuleExecutionStatus['going to run'] && + newStatus !== RuleExecutionStatus.running + ) { + const outcome = mapRuleExecutionStatusToRuleLastRunOutcome(newStatus); + ruleMonitoringService.setLastRunOutcome(outcome); + } - ruleMonitoringService.setLastRunOutcomeMsg("outcome message"); + ruleMonitoringService.setLastRunOutcomeMsg(message); - await soClient.createOrUpdate(ruleId, { - last_execution: { - date: nowISO(), - status: newStatus, - status_order: ruleExecutionStatusToNumber(newStatus), - message, - metrics: metrics ?? {}, - }, - }); + if (totalSearchDurationMs) { + ruleMonitoringService.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); + } + + if (totalIndexingDurationMs) { + ruleMonitoringService.setLastRunMetricsTotalIndexingDurationMs(totalIndexingDurationMs); + } + + if (executionGapDurationS) { + ruleMonitoringService.setLastRunMetricsGapDurationS(executionGapDurationS); + } }; const writeStatusChangeToEventLog = (args: NormalizedStatusChangeArgs): void => { @@ -208,8 +222,6 @@ export const createClientForExecutors = ( return client; }; -const nowISO = () => new Date().toISOString(); - interface NormalizedStatusChangeArgs { newStatus: RuleExecutionStatus; message: string; @@ -245,3 +257,20 @@ const normalizeDurations = (durations?: string[]): number | undefined => { const normalizeGap = (duration?: Duration): number | undefined => { return duration ? Math.round(duration.asSeconds()) : undefined; }; + +const mapRuleExecutionStatusToRuleLastRunOutcome = ( + newStatus: RuleExecutionStatus +): RuleLastRunOutcomes => { + switch (newStatus) { + case RuleExecutionStatus.succeeded: + return 'succeeded'; + case RuleExecutionStatus.failed: + return 'failed'; + case RuleExecutionStatus['partial failure']: + return 'warning'; + case RuleExecutionStatus['going to run']: + case RuleExecutionStatus.running: + default: + throw new Error(`Unexpected rule execution status: ${newStatus}`); + } +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts index cf403f3d6b66c..acc9db2253f57 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts @@ -5,37 +5,45 @@ * 2.0. */ -import type { RuleExecutionStatus as RuleExecutionStatusByFramework } from '@kbn/alerting-plugin/common'; +import type { + ResolvedSanitizedRule, + RuleExecutionStatus as RuleExecutionStatusByFramework, + SanitizedRule, +} from '@kbn/alerting-plugin/common'; import type { RuleExecutionSummary } from '../../../../../../common/detection_engine/rule_monitoring'; import { RuleExecutionStatus, ruleExecutionStatusToNumber, } from '../../../../../../common/detection_engine/rule_monitoring'; +import type { RuleParams } from '../../../rule_schema'; export const mergeRuleExecutionSummary = ( ruleExecutionStatus: RuleExecutionStatusByFramework, - ruleExecutionSummary: RuleExecutionSummary | null + rule: SanitizedRule | ResolvedSanitizedRule ): RuleExecutionSummary | null => { - if (ruleExecutionSummary == null) { - return null; - } - const frameworkStatus = ruleExecutionStatus; - const customStatus = ruleExecutionSummary.last_execution; + const isFrameworkStatusMoreRecent = + !rule.monitoring || + new Date(frameworkStatus.lastExecutionDate) > new Date(rule.monitoring?.run.last_run.timestamp); - if ( - frameworkStatus.status === 'error' && - new Date(frameworkStatus.lastExecutionDate) > new Date(customStatus.date) - ) { + if (frameworkStatus.status === 'error' && isFrameworkStatusMoreRecent) { return { - ...ruleExecutionSummary, last_execution: { date: frameworkStatus.lastExecutionDate.toISOString(), status: RuleExecutionStatus.failed, status_order: ruleExecutionStatusToNumber(RuleExecutionStatus.failed), message: `Reason: ${frameworkStatus.error?.reason} Message: ${frameworkStatus.error?.message}`, - metrics: customStatus.metrics, + metrics: { + total_indexing_duration_ms: + rule.monitoring?.run.last_run.metrics.total_indexing_duration_ms ?? undefined, + total_search_duration_ms: + rule.monitoring?.run.last_run.metrics.total_search_duration_ms ?? undefined, + execution_gap_duration_s: + rule.monitoring?.run.last_run.metrics.gap_duration_s ?? undefined, + total_enrichment_duration_ms: + rule.monitoring?.run.last_run.metrics.total_enrichment_duration_ms ?? undefined, + }, }, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts index 3f9860d9f44df..c67582b4747b7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts @@ -64,12 +64,10 @@ export const createRuleExecutionLogService = ( savedObjectsClient ); - const soClient = createRuleExecutionSavedObjectsClient(savedObjectsClient, childLogger); const eventLogWriter = createEventLogWriter(plugins.eventLog); return createClientForExecutors( ruleExecutionSettings, - soClient, eventLogWriter, childLogger, context, From dfe8090893d4a8d2979af12dfb5148227f2a9606 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 8 Dec 2022 18:35:47 +0100 Subject: [PATCH 05/34] Add RuleLastRunService Use new RuleLastRunService Revert changes Refactoring [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' Refactor lastRunFromState method to use new RuleLastRunService Rewrite createRuleExecutionSummary Solve type issues Modify outcomeMessage Remove setting of warning Apply changes --- x-pack/plugins/alerting/common/rule.ts | 2 +- .../alerting/server/lib/last_run_status.ts | 13 ++++ .../plugins/alerting/server/lib/monitoring.ts | 14 +--- .../monitoring/rule_last_run_service.test.ts | 72 +++++++++++++++++++ .../monitoring/rule_last_run_service.ts | 60 ++++++++++++++++ .../monitoring/rule_monitoring_service.ts | 51 ++----------- .../rule_monitoring/model/execution_status.ts | 14 ++-- .../rule_monitoring/model/log_level.ts | 4 +- .../normalization/rule_converters.ts | 11 +-- .../client_for_executors/client.ts | 38 ++++------ .../create_rule_execution_summary.ts | 40 +++++++++++ .../logic/rule_execution_log/index.ts | 2 +- .../merge_rule_execution_summary.ts | 52 -------------- .../logic/rule_execution_log/service.ts | 5 +- .../rule_execution_log/service_interface.ts | 6 +- .../create_security_rule_type_wrapper.ts | 2 + 16 files changed, 224 insertions(+), 162 deletions(-) create mode 100644 x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts create mode 100644 x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index 43688b51b82f5..3bfddd79af10d 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -32,7 +32,7 @@ export const RuleExecutionStatusValues = [ ] as const; export type RuleExecutionStatuses = typeof RuleExecutionStatusValues[number]; -export const RuleLastRunOutcomeValues = ['succeeded', 'warning', 'failed', 'unknown'] as const; +export const RuleLastRunOutcomeValues = ['succeeded', 'warning', 'failed'] as const; export type RuleLastRunOutcomes = typeof RuleLastRunOutcomeValues[number]; export enum RuleExecutionStatusErrorReasons { diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.ts b/x-pack/plugins/alerting/server/lib/last_run_status.ts index d2bc22cadb285..b4b156bccacb6 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.ts @@ -62,6 +62,19 @@ export const lastRunFromState = ( outcomeMsg.push(outcomeMessage); } + // Overwrite outcome to be error if last run reported any errors + if (errors.length > 0) { + outcome = RuleLastRunOutcomeValues[2]; + } + + // Optionally merge outcome messages reported by + // rule execution to the Framework's outcome message + if (outcomeMsg.length > 0 && outcomeMessage.length > 0) { + outcomeMsg = `${outcomeMsg} - ${outcomeMessage}`; + } else if (outcomeMessage.length > 0) { + outcomeMsg = outcomeMessage; + } + return { lastRun: { outcome, diff --git a/x-pack/plugins/alerting/server/lib/monitoring.ts b/x-pack/plugins/alerting/server/lib/monitoring.ts index fc607952534f6..93da6e2283152 100644 --- a/x-pack/plugins/alerting/server/lib/monitoring.ts +++ b/x-pack/plugins/alerting/server/lib/monitoring.ts @@ -6,7 +6,7 @@ */ import { Logger } from '@kbn/core/server'; import stats from 'stats-lite'; -import { RuleMonitoring, RawRuleMonitoring, RuleMonitoringHistory, RuleLastRun } from '../types'; +import { RuleMonitoring, RawRuleMonitoring, RuleMonitoringHistory } from '../types'; export const INITIAL_METRICS = { total_search_duration_ms: null, @@ -31,18 +31,6 @@ export const getDefaultMonitoring = (timestamp: string): RawRuleMonitoring => { }; }; -export const getDefaultLastRun = (): RuleLastRun => ({ - warning: null, - outcome: 'unknown', - outcomeMsg: null, - alertsCount: { - active: null, - new: null, - recovered: null, - ignored: null, - }, -}); - export const getExecutionDurationPercentiles = (history: RuleMonitoringHistory[]) => { const durationSamples = history.reduce((duration, historyItem) => { if (typeof historyItem.duration === 'number') { diff --git a/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts b/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts new file mode 100644 index 0000000000000..e9761b14d9f75 --- /dev/null +++ b/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts @@ -0,0 +1,72 @@ +/* + * 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 { PublicLastRunSetters } from '../types'; +import { RuleLastRunResults, RuleLastRunService } from './rule_last_run_service'; + +describe('RuleLastRunService', () => { + let ruleLastRunService: RuleLastRunService; + let lastRunSetters: PublicLastRunSetters; + + beforeEach(() => { + ruleLastRunService = new RuleLastRunService(); + lastRunSetters = ruleLastRunService.getLastRunSetters(); + }); + + test('should return empty errors array if no errors were added', () => { + expect(ruleLastRunService.getLastRunErrors()).toEqual([]); + }); + + test('should return empty warnings array if no warnings were added', () => { + expect(ruleLastRunService.getLastRunWarnings()).toEqual([]); + }); + + test('should return empty outcome messages array if none were added', () => { + expect(ruleLastRunService.getLastRunOutcomeMessage()).toEqual(''); + }); + + test('should return errors array with added error', () => { + lastRunSetters.addLastRunError('First error'); + expect(ruleLastRunService.getLastRunErrors()).toEqual(['First error']); + }); + + test('should return warnings array with added warning', () => { + lastRunSetters.addLastRunWarning('Second warning'); + expect(ruleLastRunService.getLastRunWarnings()).toEqual(['Second warning']); + }); + + test('should return outcome messages array with added outcome message', () => { + lastRunSetters.setLastRunOutcomeMessage('Third outcome message'); + expect(ruleLastRunService.getLastRunOutcomeMessage()).toEqual('Third outcome message'); + }); + + test('should return last run object with added errors, warnings and outcome messages', () => { + lastRunSetters.addLastRunError('error'); + lastRunSetters.addLastRunWarning('warning'); + lastRunSetters.setLastRunOutcomeMessage('outcome message'); + const expectedLastRun: RuleLastRunResults = { + errors: ['error'], + warnings: ['warning'], + outcomeMessage: 'outcome message', + }; + expect(ruleLastRunService.getLastRunResults()).toEqual(expectedLastRun); + }); + + test('should return last run object with multiple added errors, warnings and the last outcome messag reported', () => { + lastRunSetters.addLastRunError('first error'); + lastRunSetters.addLastRunError('second error'); + lastRunSetters.setLastRunOutcomeMessage('first outcome message'); + lastRunSetters.setLastRunOutcomeMessage('second outcome message'); + lastRunSetters.setLastRunOutcomeMessage('last outcome message'); + const expectedLastRun = { + errors: ['first error', 'second error'], + warnings: [], + outcomeMessage: 'last outcome message', + }; + expect(ruleLastRunService.getLastRunResults()).toEqual(expectedLastRun); + }); +}); diff --git a/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts b/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts new file mode 100644 index 0000000000000..0e1a7a530169a --- /dev/null +++ b/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts @@ -0,0 +1,60 @@ +/* + * 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 { PublicLastRunSetters } from '../types'; + +export interface RuleLastRunResults { + errors: string[]; + warnings: string[]; + outcomeMessage: string; +} + +export class RuleLastRunService { + private errors: string[] = []; + private warnings: string[] = []; + private outcomeMessage: string = ''; + + public getLastRunErrors(): string[] { + return this.errors; + } + + public getLastRunWarnings(): string[] { + return this.warnings; + } + + public getLastRunOutcomeMessage(): string { + return this.outcomeMessage; + } + + public getLastRunResults(): RuleLastRunResults { + return { + errors: this.errors, + warnings: this.warnings, + outcomeMessage: this.outcomeMessage, + }; + } + + public getLastRunSetters(): PublicLastRunSetters { + return { + addLastRunError: this.addLastRunError.bind(this), + addLastRunWarning: this.addLastRunWarning.bind(this), + setLastRunOutcomeMessage: this.setLastRunOutcomeMessage.bind(this), + }; + } + + private addLastRunError(error: string) { + this.errors.push(error); + } + + private addLastRunWarning(warning: string) { + this.warnings.push(warning); + } + + private setLastRunOutcomeMessage(outcomeMessage: string) { + this.outcomeMessage = outcomeMessage; + } +} diff --git a/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts b/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts index e5e67309cf250..0043f47c51633 100644 --- a/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts +++ b/x-pack/plugins/alerting/server/monitoring/rule_monitoring_service.ts @@ -5,23 +5,11 @@ * 2.0. */ -import { - getDefaultLastRun, - getDefaultMonitoring, - getExecutionDurationPercentiles, -} from '../lib/monitoring'; -import { - RuleMonitoring, - RuleMonitoringHistory, - PublicRuleMonitoringService, - RuleLastRun, - RuleLastRunOutcomes, - PublicMetricsSetters, - PublicLastRunSetters, -} from '../types'; +import { getDefaultMonitoring, getExecutionDurationPercentiles } from '../lib/monitoring'; +import { RuleMonitoring, RuleMonitoringHistory, PublicRuleMonitoringService } from '../types'; + export class RuleMonitoringService { private monitoring: RuleMonitoring = getDefaultMonitoring(new Date().toISOString()); - private lastRun: RuleLastRun = getDefaultLastRun(); public setLastRunMetricsDuration(duration: number) { this.monitoring.run.last_run.metrics.duration = duration; @@ -37,10 +25,6 @@ export class RuleMonitoringService { return this.monitoring; } - public getLastRun(): RuleLastRun { - return this.lastRun; - } - public addHistory({ duration, hasError = true, @@ -70,7 +54,7 @@ export class RuleMonitoringService { }; } - public getLastRunMetricsSetters(): PublicMetricsSetters { + public getLastRunMetricsSetters(): PublicRuleMonitoringService { return { setLastRunMetricsTotalSearchDurationMs: this.setLastRunMetricsTotalSearchDurationMs.bind(this), @@ -102,33 +86,6 @@ export class RuleMonitoringService { this.monitoring.run.last_run.metrics.gap_duration_s = gapDurationS; } - public getLastRunSetters(): PublicLastRunSetters { - return { - setLastRunOutcome: this.setLastRunOutcome.bind(this), - setLastRunOutcomeMsg: this.setLastRunOutcomeMsg.bind(this), - setLastRunWarning: this.setLastRunWarning.bind(this), - }; - } - - private setLastRunOutcome(outcome: RuleLastRunOutcomes) { - this.lastRun.outcome = outcome; - } - - private setLastRunOutcomeMsg(outcomeMsg: string) { - this.lastRun.outcomeMsg = outcomeMsg; - } - - private setLastRunWarning(warning: RuleLastRun['warning']) { - this.lastRun.warning = warning; - } - - public getRuleMonitoringSetters(): PublicRuleMonitoringService { - return { - ...this.getLastRunMetricsSetters(), - ...this.getLastRunSetters(), - }; - } - private buildExecutionSuccessRatio() { const { history } = this.monitoring.run; return history.filter(({ success }) => success).length / history.length; diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts index 8fa47be3ad6bb..af5049c8f9bf0 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/execution_status.ts @@ -80,20 +80,18 @@ export const ruleExecutionStatusToNumber = ( } }; -export const ruleLastRunOutcomeToNumber = ( +export const ruleLastRunOutcomeToExecutionStatus = ( outcome: RuleLastRunOutcomes -): RuleExecutionStatusOrder => { +): RuleExecutionStatus => { switch (outcome) { case 'succeeded': - return 0; + return RuleExecutionStatus.succeeded; case 'warning': - return 20; + return RuleExecutionStatus['partial failure']; case 'failed': - return 30; - case 'unknown': - return 40; // TBC + return RuleExecutionStatus.failed; default: assertUnreachable(outcome); - return 0; + return RuleExecutionStatus.failed; } }; diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts index fb5356092b65c..d166de577ddca 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts @@ -11,7 +11,7 @@ import { enumFromString } from '../../../utils/enum_from_string'; import { assertUnreachable } from '../../../utility_types'; import { RuleExecutionStatus } from './execution_status'; -export enum LogLevel { +export enum LogLevel { 'trace' = 'trace', 'debug' = 'debug', 'info' = 'info', @@ -90,8 +90,6 @@ export const logLevelFromLastRunOutcome = (status: RuleLastRunOutcomes): LogLeve return LogLevel.warn; case 'failed': return LogLevel.error; - case 'unknown': - return LogLevel.trace; default: assertUnreachable(status); return LogLevel.trace; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 4e2d17ec91803..9e06b584a06e9 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -19,7 +19,6 @@ import { } from '../../../../../common/constants'; import type { PatchRuleRequestBody } from '../../../../../common/detection_engine/rule_management'; -import type { RuleExecutionSummary } from '../../../../../common/detection_engine/rule_monitoring'; import type { RelatedIntegrationArray, RequiredFieldArray, @@ -54,7 +53,6 @@ import { assertUnreachable } from '../../../../../common/utility_types'; // eslint-disable-next-line no-restricted-imports import type { LegacyRuleActions } from '../../rule_actions_legacy'; -import { mergeRuleExecutionSummary } from '../../rule_monitoring'; import type { InternalRuleCreate, RuleParams, @@ -83,6 +81,7 @@ import { transformToNotifyWhen, } from './rule_actions'; import { convertAlertSuppressionToCamel, convertAlertSuppressionToSnake } from '../utils/utils'; +import { createRuleExecutionSummary } from '../../rule_monitoring'; // These functions provide conversions from the request API schema to the internal rule schema and from the internal rule schema // to the response API schema. This provides static type-check assurances that the internal schema is in sync with the API schema for @@ -669,13 +668,9 @@ export const commonParamsCamelToSnake = (params: BaseRuleParams) => { export const internalRuleToAPIResponse = ( rule: SanitizedRule | ResolvedSanitizedRule, - ruleExecutionSummary?: RuleExecutionSummary | null, legacyRuleActions?: LegacyRuleActions | null ): RuleResponse => { - const mergedExecutionSummary = mergeRuleExecutionSummary( - rule.executionStatus, - ruleExecutionSummary ?? null - ); + const executionSummary = createRuleExecutionSummary(rule); const isResolvedRule = (obj: unknown): obj is ResolvedSanitizedRule => (obj as ResolvedSanitizedRule).outcome != null; @@ -703,6 +698,6 @@ export const internalRuleToAPIResponse = ( throttle: transformFromAlertThrottle(rule, legacyRuleActions), actions: transformActions(rule.actions, legacyRuleActions), // Execution summary - execution_summary: mergedExecutionSummary ?? undefined, + execution_summary: executionSummary ?? undefined, }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index bf3c35c52844d..139d7f5ca6dc3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -10,8 +10,8 @@ import type { Duration } from 'moment'; import type { Logger } from '@kbn/core/server'; import type { + PublicRuleLastRunService, PublicRuleMonitoringService, - RuleLastRunOutcomes, } from '@kbn/alerting-plugin/server/types'; import type { RuleExecutionMetrics, @@ -43,7 +43,8 @@ export const createClientForExecutors = ( eventLog: IEventLogWriter, logger: Logger, context: RuleExecutionContext, - ruleMonitoringService: PublicRuleMonitoringService + ruleMonitoringService: PublicRuleMonitoringService, + ruleLastRunService: PublicRuleLastRunService ): IRuleExecutionLogForExecutors => { const baseCorrelationIds = getCorrelationIds(context); const baseLogSuffix = baseCorrelationIds.getLogSuffix(); @@ -169,15 +170,17 @@ export const createClientForExecutors = ( execution_gap_duration_s: executionGapDurationS, } = metrics ?? {}; - if ( - newStatus !== RuleExecutionStatus['going to run'] && - newStatus !== RuleExecutionStatus.running - ) { - const outcome = mapRuleExecutionStatusToRuleLastRunOutcome(newStatus); - ruleMonitoringService.setLastRunOutcome(outcome); + if (newStatus === RuleExecutionStatus.failed) { + ruleLastRunService.addLastRunError(message); } - ruleMonitoringService.setLastRunOutcomeMsg(message); + if (newStatus === RuleExecutionStatus['partial failure']) { + ruleLastRunService.addLastRunWarning(message); + } + + if (newStatus === RuleExecutionStatus.succeeded) { + ruleLastRunService.addLastRunOutcomeMessage(message); + } if (totalSearchDurationMs) { ruleMonitoringService.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); @@ -257,20 +260,3 @@ const normalizeDurations = (durations?: string[]): number | undefined => { const normalizeGap = (duration?: Duration): number | undefined => { return duration ? Math.round(duration.asSeconds()) : undefined; }; - -const mapRuleExecutionStatusToRuleLastRunOutcome = ( - newStatus: RuleExecutionStatus -): RuleLastRunOutcomes => { - switch (newStatus) { - case RuleExecutionStatus.succeeded: - return 'succeeded'; - case RuleExecutionStatus.failed: - return 'failed'; - case RuleExecutionStatus['partial failure']: - return 'warning'; - case RuleExecutionStatus['going to run']: - case RuleExecutionStatus.running: - default: - throw new Error(`Unexpected rule execution status: ${newStatus}`); - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts new file mode 100644 index 0000000000000..4df8ce0720338 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -0,0 +1,40 @@ +/* + * 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 { ResolvedSanitizedRule, SanitizedRule } from '@kbn/alerting-plugin/common'; + +import type { RuleExecutionSummary } from '../../../../../../common/detection_engine/rule_monitoring'; +import { + ruleLastRunOutcomeToExecutionStatus, + RuleExecutionStatus, + ruleExecutionStatusToNumber, +} from '../../../../../../common/detection_engine/rule_monitoring'; +import type { RuleParams } from '../../../rule_schema'; + +export const createRuleExecutionSummary = ( + rule: SanitizedRule | ResolvedSanitizedRule +): RuleExecutionSummary | null => { + const ruleExecutionStatus = ruleLastRunOutcomeToExecutionStatus( + rule.lastRun?.outcome ?? RuleExecutionStatus.failed + ); + + return { + last_execution: { + date: rule.monitoring?.run.last_run?.timestamp ?? '', + status: ruleExecutionStatus, + status_order: ruleExecutionStatusToNumber(ruleExecutionStatus), + message: rule.lastRun?.outcomeMsg ?? '', + metrics: { + total_indexing_duration_ms: + rule.monitoring?.run.last_run.metrics.total_indexing_duration_ms ?? undefined, + total_search_duration_ms: + rule.monitoring?.run.last_run.metrics.total_search_duration_ms ?? undefined, + execution_gap_duration_s: rule.monitoring?.run.last_run.metrics.gap_duration_s ?? undefined, + }, + }, + }; +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts index 1cc6be1e0c6c4..faea572babfc8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts @@ -13,5 +13,5 @@ export * from './service'; export { ruleExecutionType } from './execution_saved_object/saved_objects_type'; export { RULE_EXECUTION_LOG_PROVIDER } from './event_log/constants'; -export { mergeRuleExecutionSummary } from './merge_rule_execution_summary'; +export { createRuleExecutionSummary } from './create_rule_execution_summary'; export * from './utils/normalization'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts deleted file mode 100644 index acc9db2253f57..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/merge_rule_execution_summary.ts +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 { - ResolvedSanitizedRule, - RuleExecutionStatus as RuleExecutionStatusByFramework, - SanitizedRule, -} from '@kbn/alerting-plugin/common'; - -import type { RuleExecutionSummary } from '../../../../../../common/detection_engine/rule_monitoring'; -import { - RuleExecutionStatus, - ruleExecutionStatusToNumber, -} from '../../../../../../common/detection_engine/rule_monitoring'; -import type { RuleParams } from '../../../rule_schema'; - -export const mergeRuleExecutionSummary = ( - ruleExecutionStatus: RuleExecutionStatusByFramework, - rule: SanitizedRule | ResolvedSanitizedRule -): RuleExecutionSummary | null => { - const frameworkStatus = ruleExecutionStatus; - const isFrameworkStatusMoreRecent = - !rule.monitoring || - new Date(frameworkStatus.lastExecutionDate) > new Date(rule.monitoring?.run.last_run.timestamp); - - if (frameworkStatus.status === 'error' && isFrameworkStatusMoreRecent) { - return { - last_execution: { - date: frameworkStatus.lastExecutionDate.toISOString(), - status: RuleExecutionStatus.failed, - status_order: ruleExecutionStatusToNumber(RuleExecutionStatus.failed), - message: `Reason: ${frameworkStatus.error?.reason} Message: ${frameworkStatus.error?.message}`, - metrics: { - total_indexing_duration_ms: - rule.monitoring?.run.last_run.metrics.total_indexing_duration_ms ?? undefined, - total_search_duration_ms: - rule.monitoring?.run.last_run.metrics.total_search_duration_ms ?? undefined, - execution_gap_duration_s: - rule.monitoring?.run.last_run.metrics.gap_duration_s ?? undefined, - total_enrichment_duration_ms: - rule.monitoring?.run.last_run.metrics.total_enrichment_duration_ms ?? undefined, - }, - }, - }; - } - - return ruleExecutionSummary; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts index c67582b4747b7..a3db746db42e5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts @@ -53,7 +53,7 @@ export const createRuleExecutionLogService = ( params: ClientForExecutorsParams ): Promise => { return withSecuritySpan('IRuleExecutionLogService.createClientForExecutors', async () => { - const { savedObjectsClient, context, ruleMonitoringService } = params; + const { savedObjectsClient, context, ruleMonitoringService, ruleLastRunService } = params; const childLogger = logger.get('ruleExecution'); @@ -71,7 +71,8 @@ export const createRuleExecutionLogService = ( eventLogWriter, childLogger, context, - ruleMonitoringService + ruleMonitoringService, + ruleLastRunService ); }); }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts index dae51b451967f..83123ecc3a6ed 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts @@ -8,12 +8,15 @@ import type { SavedObjectsClientContract } from '@kbn/core/server'; import type { IEventLogClient } from '@kbn/event-log-plugin/server'; +import type { + PublicRuleLastRunService, + PublicRuleMonitoringService, +} from '@kbn/alerting-plugin/server/types'; import type { IRuleExecutionLogForRoutes } from './client_for_routes/client_interface'; import type { IRuleExecutionLogForExecutors, RuleExecutionContext, } from './client_for_executors/client_interface'; -import { PublicRuleMonitoringService } from '@kbn/alerting-plugin/server/types'; export interface IRuleExecutionLogService { registerEventLogProvider(): void; @@ -33,5 +36,6 @@ export interface ClientForRoutesParams { export interface ClientForExecutorsParams { savedObjectsClient: SavedObjectsClientContract; ruleMonitoringService: PublicRuleMonitoringService; + ruleLastRunService: PublicRuleLastRunService; context: RuleExecutionContext; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts index 6f6cd67b86011..1a287b721aec5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts @@ -89,6 +89,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = scopedClusterClient, uiSettingsClient, ruleMonitoringService, + ruleLastRunService, } = services; const searchAfterSize = Math.min(maxSignals, DEFAULT_SEARCH_AFTER_PAGE_SIZE); @@ -97,6 +98,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = const ruleExecutionLogger = await ruleExecutionLoggerFactory({ savedObjectsClient, ruleMonitoringService, + ruleLastRunService, context: { executionId, ruleId: rule.id, From 9627f8687e40d27fb5daebefecd9095ce207bc0b Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 15 Dec 2022 13:32:34 +0100 Subject: [PATCH 06/34] Add null return for createRuleExecutionSummary Test fix Cleanup unused methods from cleint_for_executor Remove leftovers Remove param Fix types Fix types Cherrypick alerting framework changes --- .../alerting/server/lib/last_run_status.ts | 10 +- ...test.ts => rule_execution_service.test.ts} | 28 +++--- ...n_service.ts => rule_execution_service.ts} | 6 +- .../logic/update_prebuilt_rules.ts | 1 - .../api/rules/bulk_actions/route.ts | 2 - .../api/rules/bulk_create_rules/route.ts | 2 +- .../api/rules/bulk_delete_rules/route.ts | 10 +- .../api/rules/bulk_patch_rules/route.ts | 4 +- .../api/rules/bulk_update_rules/route.ts | 4 +- .../api/rules/create_rule/route.ts | 5 +- .../api/rules/delete_rule/route.ts | 6 +- .../api/rules/find_rules/route.ts | 12 +-- .../api/rules/patch_rule/route.ts | 5 +- .../api/rules/read_rule/route.ts | 5 +- .../api/rules/update_rule/route.ts | 4 +- .../logic/crud/delete_rules.test.ts | 5 - .../logic/crud/delete_rules.ts | 5 +- .../logic/export/get_export_by_object_ids.ts | 2 +- .../rule_management/utils/utils.test.ts | 5 +- .../rule_management/utils/utils.ts | 11 +-- .../rule_management/utils/validate.test.ts | 18 +--- .../rule_management/utils/validate.ts | 9 +- .../rule_execution_log/__mocks__/index.ts | 19 ---- .../client_for_executors/client.ts | 16 ++-- .../client_for_routes/client.ts | 93 ------------------- .../client_for_routes/client_interface.ts | 23 ----- .../create_rule_execution_summary.ts | 9 +- .../rule_execution_log/service_interface.ts | 4 +- 28 files changed, 63 insertions(+), 260 deletions(-) rename x-pack/plugins/alerting/server/monitoring/{rule_last_run_service.test.ts => rule_execution_service.test.ts} (67%) rename x-pack/plugins/alerting/server/monitoring/{rule_last_run_service.ts => rule_execution_service.ts} (91%) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.ts b/x-pack/plugins/alerting/server/lib/last_run_status.ts index b4b156bccacb6..2eaccd9f0aabb 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.ts @@ -67,12 +67,10 @@ export const lastRunFromState = ( outcome = RuleLastRunOutcomeValues[2]; } - // Optionally merge outcome messages reported by - // rule execution to the Framework's outcome message - if (outcomeMsg.length > 0 && outcomeMessage.length > 0) { - outcomeMsg = `${outcomeMsg} - ${outcomeMessage}`; - } else if (outcomeMessage.length > 0) { - outcomeMsg = outcomeMessage; + // Optionally push outcome message reported by + // rule execution to the Framework's outcome message array + if (outcomeMessage) { + outcomeMsg.push(outcomeMessage); } return { diff --git a/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts b/x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts similarity index 67% rename from x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts rename to x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts index e9761b14d9f75..a8572382b6e82 100644 --- a/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.test.ts +++ b/x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts @@ -6,54 +6,54 @@ */ import { PublicLastRunSetters } from '../types'; -import { RuleLastRunResults, RuleLastRunService } from './rule_last_run_service'; +import { RuleExecutionResults, RuleExecutionService } from './rule_execution_service'; -describe('RuleLastRunService', () => { - let ruleLastRunService: RuleLastRunService; +describe('RuleExecutionService', () => { + let ruleExecutionService: RuleExecutionService; let lastRunSetters: PublicLastRunSetters; beforeEach(() => { - ruleLastRunService = new RuleLastRunService(); - lastRunSetters = ruleLastRunService.getLastRunSetters(); + ruleExecutionService = new RuleExecutionService(); + lastRunSetters = ruleExecutionService.getLastRunSetters(); }); test('should return empty errors array if no errors were added', () => { - expect(ruleLastRunService.getLastRunErrors()).toEqual([]); + expect(ruleExecutionService.getLastRunErrors()).toEqual([]); }); test('should return empty warnings array if no warnings were added', () => { - expect(ruleLastRunService.getLastRunWarnings()).toEqual([]); + expect(ruleExecutionService.getLastRunWarnings()).toEqual([]); }); test('should return empty outcome messages array if none were added', () => { - expect(ruleLastRunService.getLastRunOutcomeMessage()).toEqual(''); + expect(ruleExecutionService.getLastRunOutcomeMessage()).toEqual(''); }); test('should return errors array with added error', () => { lastRunSetters.addLastRunError('First error'); - expect(ruleLastRunService.getLastRunErrors()).toEqual(['First error']); + expect(ruleExecutionService.getLastRunErrors()).toEqual(['First error']); }); test('should return warnings array with added warning', () => { lastRunSetters.addLastRunWarning('Second warning'); - expect(ruleLastRunService.getLastRunWarnings()).toEqual(['Second warning']); + expect(ruleExecutionService.getLastRunWarnings()).toEqual(['Second warning']); }); test('should return outcome messages array with added outcome message', () => { lastRunSetters.setLastRunOutcomeMessage('Third outcome message'); - expect(ruleLastRunService.getLastRunOutcomeMessage()).toEqual('Third outcome message'); + expect(ruleExecutionService.getLastRunOutcomeMessage()).toEqual('Third outcome message'); }); test('should return last run object with added errors, warnings and outcome messages', () => { lastRunSetters.addLastRunError('error'); lastRunSetters.addLastRunWarning('warning'); lastRunSetters.setLastRunOutcomeMessage('outcome message'); - const expectedLastRun: RuleLastRunResults = { + const expectedLastRun: RuleExecutionResults = { errors: ['error'], warnings: ['warning'], outcomeMessage: 'outcome message', }; - expect(ruleLastRunService.getLastRunResults()).toEqual(expectedLastRun); + expect(ruleExecutionService.getLastRunResults()).toEqual(expectedLastRun); }); test('should return last run object with multiple added errors, warnings and the last outcome messag reported', () => { @@ -67,6 +67,6 @@ describe('RuleLastRunService', () => { warnings: [], outcomeMessage: 'last outcome message', }; - expect(ruleLastRunService.getLastRunResults()).toEqual(expectedLastRun); + expect(ruleExecutionService.getLastRunResults()).toEqual(expectedLastRun); }); }); diff --git a/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts b/x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts similarity index 91% rename from x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts rename to x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts index 0e1a7a530169a..f0a2c4ca04792 100644 --- a/x-pack/plugins/alerting/server/monitoring/rule_last_run_service.ts +++ b/x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts @@ -7,13 +7,13 @@ import { PublicLastRunSetters } from '../types'; -export interface RuleLastRunResults { +export interface RuleExecutionResults { errors: string[]; warnings: string[]; outcomeMessage: string; } -export class RuleLastRunService { +export class RuleExecutionService { private errors: string[] = []; private warnings: string[] = []; private outcomeMessage: string = ''; @@ -30,7 +30,7 @@ export class RuleLastRunService { return this.outcomeMessage; } - public getLastRunResults(): RuleLastRunResults { + public getLastRunResults(): RuleExecutionResults { return { errors: this.errors, warnings: this.warnings, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/update_prebuilt_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/update_prebuilt_rules.ts index 341038097f448..4956b9377c947 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/update_prebuilt_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/update_prebuilt_rules.ts @@ -87,7 +87,6 @@ const createPromises = ( await deleteRules({ ruleId: migratedRule.id, rulesClient, - ruleExecutionLog, }); return createRules({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts index 82660cba2a870..2234f09cb1888 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts @@ -329,7 +329,6 @@ export const performBulkActionRoute = ( ]); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const exceptionsClient = ctx.lists?.getExceptionListClient(); const savedObjectsClient = ctx.core.savedObjects.client; @@ -481,7 +480,6 @@ export const performBulkActionRoute = ( await deleteRules({ ruleId: migratedRule.id, rulesClient, - ruleExecutionLog, }); return null; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_create_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_create_rules/route.ts index f4497830fd85b..958a1e01ce2f5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_create_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_create_rules/route.ts @@ -121,7 +121,7 @@ export const bulkCreateRulesRoute = ( params: payloadRule, }); - return transformValidateBulkError(createdRule.params.ruleId, createdRule, null); + return transformValidateBulkError(createdRule.params.ruleId, createdRule); } catch (err) { return transformBulkError( payloadRule.rule_id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_delete_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_delete_rules/route.ts index 399b12ded105e..7864fb0bbb3d3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_delete_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_delete_rules/route.ts @@ -64,7 +64,6 @@ export const bulkDeleteRulesRoute = (router: SecuritySolutionPluginRouter, logge const ctx = await context.resolve(['core', 'securitySolution', 'alerting']); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const savedObjectsClient = ctx.core.savedObjects.client; const rules = await Promise.all( @@ -91,19 +90,12 @@ export const bulkDeleteRulesRoute = (router: SecuritySolutionPluginRouter, logge return getIdBulkError({ id, ruleId }); } - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(migratedRule.id); - await deleteRules({ ruleId: migratedRule.id, rulesClient, - ruleExecutionLog, }); - return transformValidateBulkError( - idOrRuleIdOrUnknown, - migratedRule, - ruleExecutionSummary - ); + return transformValidateBulkError(idOrRuleIdOrUnknown, migratedRule); } catch (err) { return transformBulkError(idOrRuleIdOrUnknown, err); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_patch_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_patch_rules/route.ts index b2b56fb3b0d8f..2aa1f46c9b9d6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_patch_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_patch_rules/route.ts @@ -56,7 +56,6 @@ export const bulkPatchRulesRoute = ( const ctx = await context.resolve(['core', 'securitySolution', 'alerting', 'licensing']); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const savedObjectsClient = ctx.core.savedObjects.client; const mlAuthz = buildMlAuthz({ @@ -111,8 +110,7 @@ export const bulkPatchRulesRoute = ( nextParams: payloadRule, }); if (rule != null && rule.enabled != null && rule.name != null) { - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id); - return transformValidateBulkError(rule.id, rule, ruleExecutionSummary); + return transformValidateBulkError(rule.id, rule); } else { return getIdBulkError({ id: payloadRule.id, ruleId: payloadRule.rule_id }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_update_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_update_rules/route.ts index 86738289fd79e..d186abecf7b38 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_update_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_update_rules/route.ts @@ -61,7 +61,6 @@ export const bulkUpdateRulesRoute = ( const ctx = await context.resolve(['core', 'securitySolution', 'alerting', 'licensing']); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const savedObjectsClient = ctx.core.savedObjects.client; const mlAuthz = buildMlAuthz({ @@ -116,8 +115,7 @@ export const bulkUpdateRulesRoute = ( ruleUpdate: payloadRule, }); if (rule != null) { - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id); - return transformValidateBulkError(rule.id, rule, ruleExecutionSummary); + return transformValidateBulkError(rule.id, rule); } else { return getIdBulkError({ id: payloadRule.id, ruleId: payloadRule.rule_id }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts index 8b081b6f7e20b..912d4227554d6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts @@ -55,7 +55,6 @@ export const createRuleRoute = ( ]); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const savedObjectsClient = ctx.core.savedObjects.client; const exceptionsClient = ctx.lists?.getExceptionListClient(); @@ -99,9 +98,7 @@ export const createRuleRoute = ( params: request.body, }); - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(createdRule.id); - - const [validated, errors] = transformValidate(createdRule, ruleExecutionSummary); + const [validated, errors] = transformValidate(createdRule); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/delete_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/delete_rule/route.ts index f5dfb61cd32ef..b4044e3dd9914 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/delete_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/delete_rule/route.ts @@ -46,7 +46,6 @@ export const deleteRuleRoute = (router: SecuritySolutionPluginRouter) => { const ctx = await context.resolve(['core', 'securitySolution', 'alerting']); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const savedObjectsClient = ctx.core.savedObjects.client; const rule = await readRules({ rulesClient, id, ruleId }); @@ -64,15 +63,12 @@ export const deleteRuleRoute = (router: SecuritySolutionPluginRouter) => { }); } - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(migratedRule.id); - await deleteRules({ ruleId: migratedRule.id, rulesClient, - ruleExecutionLog, }); - const transformed = transform(migratedRule, ruleExecutionSummary); + const transformed = transform(migratedRule); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'failed to transform alert' }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/find_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/find_rules/route.ts index 34117ca07f817..8654a9dafd245 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/find_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/find_rules/route.ts @@ -49,7 +49,6 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter, logger: Log const { query } = request; const ctx = await context.resolve(['core', 'securitySolution', 'alerting']); const rulesClient = ctx.alerting.getRulesClient(); - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); const savedObjectsClient = ctx.core.savedObjects.client; const rules = await findRules({ @@ -64,12 +63,13 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter, logger: Log const ruleIds = rules.data.map((rule) => rule.id); - const [ruleExecutionSummaries, ruleActions] = await Promise.all([ - ruleExecutionLog.getExecutionSummariesBulk(ruleIds), - legacyGetBulkRuleActionsSavedObject({ alertIds: ruleIds, savedObjectsClient, logger }), - ]); + const ruleActions = await legacyGetBulkRuleActionsSavedObject({ + alertIds: ruleIds, + savedObjectsClient, + logger, + }); - const transformed = transformFindAlerts(rules, ruleExecutionSummaries, ruleActions); + const transformed = transformFindAlerts(rules, ruleActions); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/patch_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/patch_rule/route.ts index 298891be82d33..a06c36a1502ea 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/patch_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/patch_rule/route.ts @@ -52,7 +52,6 @@ export const patchRuleRoute = (router: SecuritySolutionPluginRouter, ml: SetupPl try { const params = request.body; const rulesClient = (await context.alerting).getRulesClient(); - const ruleExecutionLog = (await context.securitySolution).getRuleExecutionLog(); const savedObjectsClient = (await context.core).savedObjects.client; const mlAuthz = buildMlAuthz({ @@ -96,9 +95,7 @@ export const patchRuleRoute = (router: SecuritySolutionPluginRouter, ml: SetupPl nextParams: params, }); if (rule != null && rule.enabled != null && rule.name != null) { - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id); - - const [validated, errors] = transformValidate(rule, ruleExecutionSummary); + const [validated, errors] = transformValidate(rule); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/read_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/read_rule/route.ts index eaa58bf00a1be..eca9636842074 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/read_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/read_rule/route.ts @@ -45,7 +45,6 @@ export const readRuleRoute = (router: SecuritySolutionPluginRouter, logger: Logg try { const rulesClient = (await context.alerting).getRulesClient(); - const ruleExecutionLog = (await context.securitySolution).getRuleExecutionLog(); const savedObjectsClient = (await context.core).savedObjects.client; const rule = await readRules({ @@ -60,9 +59,7 @@ export const readRuleRoute = (router: SecuritySolutionPluginRouter, logger: Logg logger, }); - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id); - - const transformed = transform(rule, ruleExecutionSummary, legacyRuleActions); + const transformed = transform(rule, legacyRuleActions); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/update_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/update_rule/route.ts index 6e37b325dfd95..1f171b15df6e7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/update_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/update_rule/route.ts @@ -84,9 +84,7 @@ export const updateRuleRoute = (router: SecuritySolutionPluginRouter, ml: SetupP }); if (rule != null) { - const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog(); - const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id); - const [validated, errors] = transformValidate(rule, ruleExecutionSummary); + const [validated, errors] = transformValidate(rule); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.test.ts index e7176b8d51af0..c34adec573f15 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.test.ts @@ -6,29 +6,24 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; -import { ruleExecutionLogMock } from '../../../rule_monitoring/mocks'; import type { DeleteRuleOptions } from './delete_rules'; import { deleteRules } from './delete_rules'; describe('deleteRules', () => { let rulesClient: ReturnType; - let ruleExecutionLog: ReturnType; beforeEach(() => { rulesClient = rulesClientMock.create(); - ruleExecutionLog = ruleExecutionLogMock.forRoutes.create(); }); it('should delete the rule along with its actions, and statuses', async () => { const options: DeleteRuleOptions = { ruleId: 'ruleId', rulesClient, - ruleExecutionLog, }; await deleteRules(options); expect(rulesClient.delete).toHaveBeenCalledWith({ id: options.ruleId }); - expect(ruleExecutionLog.clearExecutionSummary).toHaveBeenCalledWith(options.ruleId); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.ts index a1190d8827c0d..e650c89fd96eb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/delete_rules.ts @@ -7,15 +7,12 @@ import type { RulesClient } from '@kbn/alerting-plugin/server'; import type { RuleObjectId } from '../../../../../../common/detection_engine/rule_schema'; -import type { IRuleExecutionLogForRoutes } from '../../../rule_monitoring'; export interface DeleteRuleOptions { ruleId: RuleObjectId; rulesClient: RulesClient; - ruleExecutionLog: IRuleExecutionLogForRoutes; } -export const deleteRules = async ({ ruleId, rulesClient, ruleExecutionLog }: DeleteRuleOptions) => { +export const deleteRules = async ({ ruleId, rulesClient }: DeleteRuleOptions) => { await rulesClient.delete({ id: ruleId }); - await ruleExecutionLog.clearExecutionSummary(ruleId); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts index 67da643d503e4..60a7ec4c8185c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts @@ -120,7 +120,7 @@ export const getRulesFromObjects = async ( ) { return { statusCode: 200, - rule: internalRuleToAPIResponse(matchingRule, null, legacyActions[matchingRule.id]), + rule: internalRuleToAPIResponse(matchingRule, legacyActions[matchingRule.id]), }; } else { return { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts index 4737fad57a7e9..91cdfc0e8a37b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts @@ -277,7 +277,7 @@ describe('utils', () => { describe('transformFindAlerts', () => { test('outputs empty data set when data set is empty correct', () => { - const output = transformFindAlerts({ data: [], page: 1, perPage: 0, total: 0 }, {}, {}); + const output = transformFindAlerts({ data: [], page: 1, perPage: 0, total: 0 }, {}); expect(output).toEqual({ data: [], page: 1, perPage: 0, total: 0 }); }); @@ -289,7 +289,6 @@ describe('utils', () => { total: 0, data: [getRuleMock(getQueryRuleParams())], }, - {}, {} ); const expected = getOutputRuleAlertForRest(); @@ -309,7 +308,6 @@ describe('utils', () => { total: 0, data: [getRuleMock(getQueryRuleParams())], }, - {}, { '123': undefined, } @@ -348,7 +346,6 @@ describe('utils', () => { total: 0, data: [getRuleMock(getQueryRuleParams())], }, - {}, legacyRuleActions ); const expected = { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts index 3e3275321d7ab..5b09fe1b3a8cd 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts @@ -15,7 +15,6 @@ import type { PartialRule, FindResult } from '@kbn/alerting-plugin/server'; import type { ActionsClient, FindActionResult } from '@kbn/actions-plugin/server'; import type { RuleToImport } from '../../../../../common/detection_engine/rule_management'; -import type { RuleExecutionSummary } from '../../../../../common/detection_engine/rule_monitoring'; import type { AlertSuppression, RuleResponse, @@ -23,7 +22,6 @@ import type { // eslint-disable-next-line no-restricted-imports import type { LegacyRulesActionsSavedObject } from '../../rule_actions_legacy'; -import type { RuleExecutionSummariesByRuleId } from '../../rule_monitoring'; import type { AlertSuppressionCamel, RuleAlertType, RuleParams } from '../../rule_schema'; import { isAlertType } from '../../rule_schema'; import type { BulkError, OutputError } from '../../routes/utils'; @@ -96,12 +94,11 @@ export const transformAlertsToRules = ( rules: RuleAlertType[], legacyRuleActions: Record ): RuleResponse[] => { - return rules.map((rule) => internalRuleToAPIResponse(rule, null, legacyRuleActions[rule.id])); + return rules.map((rule) => internalRuleToAPIResponse(rule, legacyRuleActions[rule.id])); }; export const transformFindAlerts = ( ruleFindResults: FindResult, - ruleExecutionSummariesByRuleId: RuleExecutionSummariesByRuleId, legacyRuleActions: Record ): { page: number; @@ -114,19 +111,17 @@ export const transformFindAlerts = ( perPage: ruleFindResults.perPage, total: ruleFindResults.total, data: ruleFindResults.data.map((rule) => { - const executionSummary = ruleExecutionSummariesByRuleId[rule.id]; - return internalRuleToAPIResponse(rule, executionSummary, legacyRuleActions[rule.id]); + return internalRuleToAPIResponse(rule, legacyRuleActions[rule.id]); }), }; }; export const transform = ( rule: PartialRule, - ruleExecutionSummary?: RuleExecutionSummary | null, legacyRuleActions?: LegacyRulesActionsSavedObject | null ): RuleResponse | null => { if (isAlertType(rule)) { - return internalRuleToAPIResponse(rule, ruleExecutionSummary, legacyRuleActions); + return internalRuleToAPIResponse(rule, legacyRuleActions); } return null; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts index f90ce4a33b573..40ced9baaf28f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts @@ -8,7 +8,6 @@ import { transformValidate, transformValidateBulkError } from './validate'; import type { BulkError } from '../../routes/utils'; import { getRuleMock } from '../../routes/__mocks__/request_responses'; -import { ruleExecutionSummaryMock } from '../../../../../common/detection_engine/rule_monitoring/mocks'; import { getListArrayMock } from '../../../../../common/detection_engine/schemas/types/lists.mock'; import { getThreatMock } from '../../../../../common/detection_engine/schemas/types/threat.mock'; import { getQueryRuleParams } from '../../rule_schema/mocks'; @@ -102,7 +101,7 @@ describe('validate', () => { describe('transformValidateBulkError', () => { test('it should do a validation correctly of a rule id', () => { const ruleAlert = getRuleMock(getQueryRuleParams()); - const validatedOrError = transformValidateBulkError('rule-1', ruleAlert, null); + const validatedOrError = transformValidateBulkError('rule-1', ruleAlert); expect(validatedOrError).toEqual(ruleOutput()); }); @@ -110,7 +109,7 @@ describe('validate', () => { const ruleAlert = getRuleMock(getQueryRuleParams()); // @ts-expect-error delete ruleAlert.name; - const validatedOrError = transformValidateBulkError('rule-1', ruleAlert, null); + const validatedOrError = transformValidateBulkError('rule-1', ruleAlert); const expected: BulkError = { error: { message: 'Invalid value "undefined" supplied to "name"', @@ -121,22 +120,11 @@ describe('validate', () => { expect(validatedOrError).toEqual(expected); }); - test('it should do a validation correctly of a rule id with rule execution summary passed in', () => { - const rule = getRuleMock(getQueryRuleParams()); - const ruleExecutionSumary = ruleExecutionSummaryMock.getSummarySucceeded(); - const validatedOrError = transformValidateBulkError('rule-1', rule, ruleExecutionSumary); - const expected: RuleResponse = { - ...ruleOutput(), - execution_summary: ruleExecutionSumary, - }; - expect(validatedOrError).toEqual(expected); - }); - test('it should return error object if "alert" is not expected alert type', () => { const ruleAlert = getRuleMock(getQueryRuleParams()); // @ts-expect-error delete ruleAlert.alertTypeId; - const validatedOrError = transformValidateBulkError('rule-1', ruleAlert, null); + const validatedOrError = transformValidateBulkError('rule-1', ruleAlert); const expected: BulkError = { error: { message: 'Internal error transforming', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.ts index e784068e0248e..5ae9efb4501d5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.ts @@ -8,7 +8,6 @@ import { validateNonExact } from '@kbn/securitysolution-io-ts-utils'; import type { PartialRule } from '@kbn/alerting-plugin/server'; -import type { RuleExecutionSummary } from '../../../../../common/detection_engine/rule_monitoring'; import { RuleResponse } from '../../../../../common/detection_engine/rule_schema'; import type { RuleParams } from '../../rule_schema'; import { isAlertType } from '../../rule_schema'; @@ -21,10 +20,9 @@ import { internalRuleToAPIResponse } from '../normalization/rule_converters'; export const transformValidate = ( rule: PartialRule, - ruleExecutionSummary: RuleExecutionSummary | null, legacyRuleActions?: LegacyRulesActionsSavedObject | null ): [RuleResponse | null, string | null] => { - const transformed = transform(rule, ruleExecutionSummary, legacyRuleActions); + const transformed = transform(rule, legacyRuleActions); if (transformed == null) { return [null, 'Internal error transforming']; } else { @@ -34,11 +32,10 @@ export const transformValidate = ( export const transformValidateBulkError = ( ruleId: string, - rule: PartialRule, - ruleExecutionSummary: RuleExecutionSummary | null + rule: PartialRule ): RuleResponse | BulkError => { if (isAlertType(rule)) { - const transformed = internalRuleToAPIResponse(rule, ruleExecutionSummary); + const transformed = internalRuleToAPIResponse(rule); const [validated, errors] = validateNonExact(transformed, RuleResponse); if (errors != null || validated == null) { return createBulkErrorObject({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/__mocks__/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/__mocks__/index.ts index e43a03c46a906..c0cb0cefa234e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/__mocks__/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/__mocks__/index.ts @@ -8,7 +8,6 @@ import { getRuleExecutionEventsResponseMock, getRuleExecutionResultsResponseMock, - ruleExecutionSummaryMock, } from '../../../../../../../common/detection_engine/rule_monitoring/mocks'; import type { IRuleExecutionLogForRoutes } from '../client_for_routes/client_interface'; @@ -17,29 +16,11 @@ import type { RuleExecutionContext, } from '../client_for_executors/client_interface'; -type GetExecutionSummariesBulk = IRuleExecutionLogForRoutes['getExecutionSummariesBulk']; -type GetExecutionSummary = IRuleExecutionLogForRoutes['getExecutionSummary']; -type ClearExecutionSummary = IRuleExecutionLogForRoutes['clearExecutionSummary']; type GetExecutionEvents = IRuleExecutionLogForRoutes['getExecutionEvents']; type GetExecutionResults = IRuleExecutionLogForRoutes['getExecutionResults']; const ruleExecutionLogForRoutesMock = { create: (): jest.Mocked => ({ - getExecutionSummariesBulk: jest - .fn, Parameters>() - .mockResolvedValue({ - '04128c15-0d1b-4716-a4c5-46997ac7f3bd': ruleExecutionSummaryMock.getSummarySucceeded(), - '1ea5a820-4da1-4e82-92a1-2b43a7bece08': ruleExecutionSummaryMock.getSummaryFailed(), - }), - - getExecutionSummary: jest - .fn, Parameters>() - .mockResolvedValue(ruleExecutionSummaryMock.getSummarySucceeded()), - - clearExecutionSummary: jest - .fn, Parameters>() - .mockResolvedValue(), - getExecutionEvents: jest .fn, Parameters>() .mockResolvedValue(getRuleExecutionEventsResponseMock.getSomeResponse()), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 139d7f5ca6dc3..95d74aa9358ca 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -43,8 +43,8 @@ export const createClientForExecutors = ( eventLog: IEventLogWriter, logger: Logger, context: RuleExecutionContext, - ruleMonitoringService: PublicRuleMonitoringService, - ruleLastRunService: PublicRuleLastRunService + ruleMonitoringService?: PublicRuleMonitoringService, + ruleLastRunService?: PublicRuleLastRunService ): IRuleExecutionLogForExecutors => { const baseCorrelationIds = getCorrelationIds(context); const baseLogSuffix = baseCorrelationIds.getLogSuffix(); @@ -171,27 +171,27 @@ export const createClientForExecutors = ( } = metrics ?? {}; if (newStatus === RuleExecutionStatus.failed) { - ruleLastRunService.addLastRunError(message); + ruleLastRunService?.addLastRunError(message); } if (newStatus === RuleExecutionStatus['partial failure']) { - ruleLastRunService.addLastRunWarning(message); + ruleLastRunService?.addLastRunWarning(message); } if (newStatus === RuleExecutionStatus.succeeded) { - ruleLastRunService.addLastRunOutcomeMessage(message); + ruleLastRunService?.setLastRunOutcomeMessage(message); } if (totalSearchDurationMs) { - ruleMonitoringService.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); + ruleMonitoringService?.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); } if (totalIndexingDurationMs) { - ruleMonitoringService.setLastRunMetricsTotalIndexingDurationMs(totalIndexingDurationMs); + ruleMonitoringService?.setLastRunMetricsTotalIndexingDurationMs(totalIndexingDurationMs); } if (executionGapDurationS) { - ruleMonitoringService.setLastRunMetricsGapDurationS(executionGapDurationS); + ruleMonitoringService?.setLastRunMetricsGapDurationS(executionGapDurationS); } }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts index 095c77d86a4ce..4c16216acaf1e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts @@ -5,17 +5,13 @@ * 2.0. */ -import { chunk, mapValues } from 'lodash'; import type { Logger } from '@kbn/core/server'; -import { initPromisePool } from '../../../../../../utils/promise_pool'; import { withSecuritySpan } from '../../../../../../utils/with_security_span'; import type { ExtMeta } from '../utils/console_logging'; -import { truncateList } from '../utils/normalization'; import type { GetRuleExecutionEventsResponse, GetRuleExecutionResultsResponse, - RuleExecutionSummary, } from '../../../../../../../common/detection_engine/rule_monitoring'; import type { IEventLogReader } from '../event_log/event_log_reader'; @@ -24,103 +20,14 @@ import type { GetExecutionEventsArgs, GetExecutionResultsArgs, IRuleExecutionLogForRoutes, - RuleExecutionSummariesByRuleId, } from './client_interface'; -const RULES_PER_CHUNK = 1000; - export const createClientForRoutes = ( soClient: IRuleExecutionSavedObjectsClient, eventLog: IEventLogReader, logger: Logger ): IRuleExecutionLogForRoutes => { return { - getExecutionSummariesBulk: (ruleIds: string[]): Promise => { - return withSecuritySpan('IRuleExecutionLogForRoutes.getExecutionSummariesBulk', async () => { - try { - // This method splits work into chunks so not to overwhelm Elasticsearch - // when fetching statuses for a big number of rules. - const ruleIdsChunks = chunk(ruleIds, RULES_PER_CHUNK); - - const { results, errors } = await initPromisePool({ - concurrency: 1, - items: ruleIdsChunks, - executor: async (ruleIdsChunk) => { - try { - const savedObjectsByRuleId = await soClient.getManyByRuleIds(ruleIdsChunk); - return mapValues(savedObjectsByRuleId, (so) => so?.attributes ?? null); - } catch (e) { - const ruleIdsString = `[${truncateList(ruleIdsChunk).join(', ')}]`; - - const logMessage = 'Error fetching a chunk of rule execution saved objects'; - const logReason = e instanceof Error ? e.stack ?? e.message : String(e); - const logSuffix = `[${ruleIdsChunk.length} rules][rule ids: ${ruleIdsString}]`; - - logger.error(`${logMessage}: ${logReason} ${logSuffix}`); - throw e; - } - }, - }); - - if (errors.length) { - const numAllChunks = ruleIdsChunks.length; - const numFailedChunks = errors.length; - const message = `Error fetching rule execution saved objects in chunks: ${numFailedChunks}/${numAllChunks} chunks failed`; - throw new AggregateError(errors, message); - } - - // Merge all rule statuses into a single dict - return Object.assign({}, ...results.map(({ result }) => result)); - } catch (e) { - const ruleIdsString = `[${truncateList(ruleIds).join(', ')}]`; - - const logMessage = 'Error bulk getting rule execution summaries'; - const logReason = e instanceof Error ? e.message : String(e); - const logSuffix = `[${ruleIds.length} rules][rule ids: ${ruleIdsString}]`; - - logger.error(`${logMessage}: ${logReason} ${logSuffix}`); - throw e; - } - }); - }, - - getExecutionSummary: (ruleId: string): Promise => { - return withSecuritySpan('IRuleExecutionLogForRoutes.getExecutionSummary', async () => { - try { - const savedObject = await soClient.getOneByRuleId(ruleId); - return savedObject ? savedObject.attributes : null; - } catch (e) { - const logMessage = 'Error getting rule execution summary'; - const logReason = e instanceof Error ? e.message : String(e); - const logSuffix = `[rule id ${ruleId}]`; - const logMeta: ExtMeta = { - rule: { id: ruleId }, - }; - - logger.error(`${logMessage}: ${logReason} ${logSuffix}`, logMeta); - throw e; - } - }); - }, - - clearExecutionSummary: (ruleId: string): Promise => { - return withSecuritySpan('IRuleExecutionLogForRoutes.clearExecutionSummary', async () => { - try { - await soClient.delete(ruleId); - } catch (e) { - const logMessage = 'Error clearing rule execution summary'; - const logReason = e instanceof Error ? e.message : String(e); - const logSuffix = `[rule id ${ruleId}]`; - const logMeta: ExtMeta = { - rule: { id: ruleId }, - }; - - logger.error(`${logMessage}: ${logReason} ${logSuffix}`, logMeta); - throw e; - } - }); - }, - getExecutionEvents: (args: GetExecutionEventsArgs): Promise => { return withSecuritySpan('IRuleExecutionLogForRoutes.getExecutionEvents', async () => { const { ruleId } = args; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client_interface.ts index dec3990654b80..a3fb5365181eb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client_interface.ts @@ -12,7 +12,6 @@ import type { LogLevel, RuleExecutionEventType, RuleExecutionStatus, - RuleExecutionSummary, SortFieldOfRuleExecutionResult, } from '../../../../../../../common/detection_engine/rule_monitoring'; @@ -22,26 +21,6 @@ import type { * - execution events such as recent failures and status changes */ export interface IRuleExecutionLogForRoutes { - /** - * Fetches a list of current execution summaries of multiple rules. - * @param ruleIds A list of saved object ids of multiple rules (`rule.id`). - * @returns A dict with rule IDs as keys and execution summaries as values. - * @throws AggregateError if any of the rule status requests fail. - */ - getExecutionSummariesBulk(ruleIds: string[]): Promise; - - /** - * Fetches current execution summary of a given rule. - * @param ruleId Saved object id of the rule (`rule.id`). - */ - getExecutionSummary(ruleId: string): Promise; - - /** - * Deletes the current execution summary if it exists. - * @param ruleId Saved object id of the rule (`rule.id`). - */ - clearExecutionSummary(ruleId: string): Promise; - /** * Fetches plain execution events of a given rule from Event Log. This includes debug, info, and * error messages that executor functions write during a rule execution to the log. @@ -103,5 +82,3 @@ export interface GetExecutionResultsArgs { /** Number of results to fetch per page. */ perPage: number; } - -export type RuleExecutionSummariesByRuleId = Record; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts index 4df8ce0720338..137d0d9edde28 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -10,7 +10,6 @@ import type { ResolvedSanitizedRule, SanitizedRule } from '@kbn/alerting-plugin/ import type { RuleExecutionSummary } from '../../../../../../common/detection_engine/rule_monitoring'; import { ruleLastRunOutcomeToExecutionStatus, - RuleExecutionStatus, ruleExecutionStatusToNumber, } from '../../../../../../common/detection_engine/rule_monitoring'; import type { RuleParams } from '../../../rule_schema'; @@ -18,9 +17,11 @@ import type { RuleParams } from '../../../rule_schema'; export const createRuleExecutionSummary = ( rule: SanitizedRule | ResolvedSanitizedRule ): RuleExecutionSummary | null => { - const ruleExecutionStatus = ruleLastRunOutcomeToExecutionStatus( - rule.lastRun?.outcome ?? RuleExecutionStatus.failed - ); + if (!rule.lastRun) { + return null; + } + + const ruleExecutionStatus = ruleLastRunOutcomeToExecutionStatus(rule.lastRun?.outcome); return { last_execution: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts index 83123ecc3a6ed..c44b56729ab86 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts @@ -35,7 +35,7 @@ export interface ClientForRoutesParams { export interface ClientForExecutorsParams { savedObjectsClient: SavedObjectsClientContract; - ruleMonitoringService: PublicRuleMonitoringService; - ruleLastRunService: PublicRuleLastRunService; + ruleMonitoringService?: PublicRuleMonitoringService; + ruleLastRunService?: PublicRuleLastRunService; context: RuleExecutionContext; } From 2c380e4ad4483c869cf21f07a6dcb417eee2cd77 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Wed, 21 Dec 2022 17:59:57 +0100 Subject: [PATCH 07/34] Rebase cleanup --- .../alerting/server/lib/last_run_status.ts | 11 --- .../monitoring/rule_execution_service.test.ts | 72 ------------------- .../monitoring/rule_execution_service.ts | 60 ---------------- 3 files changed, 143 deletions(-) delete mode 100644 x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts delete mode 100644 x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.ts b/x-pack/plugins/alerting/server/lib/last_run_status.ts index 2eaccd9f0aabb..d2bc22cadb285 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.ts @@ -62,17 +62,6 @@ export const lastRunFromState = ( outcomeMsg.push(outcomeMessage); } - // Overwrite outcome to be error if last run reported any errors - if (errors.length > 0) { - outcome = RuleLastRunOutcomeValues[2]; - } - - // Optionally push outcome message reported by - // rule execution to the Framework's outcome message array - if (outcomeMessage) { - outcomeMsg.push(outcomeMessage); - } - return { lastRun: { outcome, diff --git a/x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts b/x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts deleted file mode 100644 index a8572382b6e82..0000000000000 --- a/x-pack/plugins/alerting/server/monitoring/rule_execution_service.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -/* - * 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 { PublicLastRunSetters } from '../types'; -import { RuleExecutionResults, RuleExecutionService } from './rule_execution_service'; - -describe('RuleExecutionService', () => { - let ruleExecutionService: RuleExecutionService; - let lastRunSetters: PublicLastRunSetters; - - beforeEach(() => { - ruleExecutionService = new RuleExecutionService(); - lastRunSetters = ruleExecutionService.getLastRunSetters(); - }); - - test('should return empty errors array if no errors were added', () => { - expect(ruleExecutionService.getLastRunErrors()).toEqual([]); - }); - - test('should return empty warnings array if no warnings were added', () => { - expect(ruleExecutionService.getLastRunWarnings()).toEqual([]); - }); - - test('should return empty outcome messages array if none were added', () => { - expect(ruleExecutionService.getLastRunOutcomeMessage()).toEqual(''); - }); - - test('should return errors array with added error', () => { - lastRunSetters.addLastRunError('First error'); - expect(ruleExecutionService.getLastRunErrors()).toEqual(['First error']); - }); - - test('should return warnings array with added warning', () => { - lastRunSetters.addLastRunWarning('Second warning'); - expect(ruleExecutionService.getLastRunWarnings()).toEqual(['Second warning']); - }); - - test('should return outcome messages array with added outcome message', () => { - lastRunSetters.setLastRunOutcomeMessage('Third outcome message'); - expect(ruleExecutionService.getLastRunOutcomeMessage()).toEqual('Third outcome message'); - }); - - test('should return last run object with added errors, warnings and outcome messages', () => { - lastRunSetters.addLastRunError('error'); - lastRunSetters.addLastRunWarning('warning'); - lastRunSetters.setLastRunOutcomeMessage('outcome message'); - const expectedLastRun: RuleExecutionResults = { - errors: ['error'], - warnings: ['warning'], - outcomeMessage: 'outcome message', - }; - expect(ruleExecutionService.getLastRunResults()).toEqual(expectedLastRun); - }); - - test('should return last run object with multiple added errors, warnings and the last outcome messag reported', () => { - lastRunSetters.addLastRunError('first error'); - lastRunSetters.addLastRunError('second error'); - lastRunSetters.setLastRunOutcomeMessage('first outcome message'); - lastRunSetters.setLastRunOutcomeMessage('second outcome message'); - lastRunSetters.setLastRunOutcomeMessage('last outcome message'); - const expectedLastRun = { - errors: ['first error', 'second error'], - warnings: [], - outcomeMessage: 'last outcome message', - }; - expect(ruleExecutionService.getLastRunResults()).toEqual(expectedLastRun); - }); -}); diff --git a/x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts b/x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts deleted file mode 100644 index f0a2c4ca04792..0000000000000 --- a/x-pack/plugins/alerting/server/monitoring/rule_execution_service.ts +++ /dev/null @@ -1,60 +0,0 @@ -/* - * 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 { PublicLastRunSetters } from '../types'; - -export interface RuleExecutionResults { - errors: string[]; - warnings: string[]; - outcomeMessage: string; -} - -export class RuleExecutionService { - private errors: string[] = []; - private warnings: string[] = []; - private outcomeMessage: string = ''; - - public getLastRunErrors(): string[] { - return this.errors; - } - - public getLastRunWarnings(): string[] { - return this.warnings; - } - - public getLastRunOutcomeMessage(): string { - return this.outcomeMessage; - } - - public getLastRunResults(): RuleExecutionResults { - return { - errors: this.errors, - warnings: this.warnings, - outcomeMessage: this.outcomeMessage, - }; - } - - public getLastRunSetters(): PublicLastRunSetters { - return { - addLastRunError: this.addLastRunError.bind(this), - addLastRunWarning: this.addLastRunWarning.bind(this), - setLastRunOutcomeMessage: this.setLastRunOutcomeMessage.bind(this), - }; - } - - private addLastRunError(error: string) { - this.errors.push(error); - } - - private addLastRunWarning(warning: string) { - this.warnings.push(warning); - } - - private setLastRunOutcomeMessage(outcomeMessage: string) { - this.outcomeMessage = outcomeMessage; - } -} From 4a1aebb91e5a11d742876f36b77625857e9b474f Mon Sep 17 00:00:00 2001 From: jpdjere Date: Wed, 21 Dec 2022 18:40:58 +0100 Subject: [PATCH 08/34] Type fixes --- .../rule_execution_log/client_for_executors/client.ts | 10 +++++----- .../create_rule_execution_summary.ts | 2 +- .../logic/rule_execution_log/service.ts | 4 ++-- .../logic/rule_execution_log/service_interface.ts | 4 ++-- .../rule_types/create_security_rule_type_wrapper.ts | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 95d74aa9358ca..353f8de944cc4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -10,7 +10,7 @@ import type { Duration } from 'moment'; import type { Logger } from '@kbn/core/server'; import type { - PublicRuleLastRunService, + PublicRuleResultService, PublicRuleMonitoringService, } from '@kbn/alerting-plugin/server/types'; import type { @@ -44,7 +44,7 @@ export const createClientForExecutors = ( logger: Logger, context: RuleExecutionContext, ruleMonitoringService?: PublicRuleMonitoringService, - ruleLastRunService?: PublicRuleLastRunService + ruleResultService?: PublicRuleResultService ): IRuleExecutionLogForExecutors => { const baseCorrelationIds = getCorrelationIds(context); const baseLogSuffix = baseCorrelationIds.getLogSuffix(); @@ -171,15 +171,15 @@ export const createClientForExecutors = ( } = metrics ?? {}; if (newStatus === RuleExecutionStatus.failed) { - ruleLastRunService?.addLastRunError(message); + ruleResultService?.addLastRunError(message); } if (newStatus === RuleExecutionStatus['partial failure']) { - ruleLastRunService?.addLastRunWarning(message); + ruleResultService?.addLastRunWarning(message); } if (newStatus === RuleExecutionStatus.succeeded) { - ruleLastRunService?.setLastRunOutcomeMessage(message); + ruleResultService?.setLastRunOutcomeMessage(message); } if (totalSearchDurationMs) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts index 137d0d9edde28..ef827354511f2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -28,7 +28,7 @@ export const createRuleExecutionSummary = ( date: rule.monitoring?.run.last_run?.timestamp ?? '', status: ruleExecutionStatus, status_order: ruleExecutionStatusToNumber(ruleExecutionStatus), - message: rule.lastRun?.outcomeMsg ?? '', + message: rule.lastRun?.outcomeMsg?.join(' ') ?? '', metrics: { total_indexing_duration_ms: rule.monitoring?.run.last_run.metrics.total_indexing_duration_ms ?? undefined, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts index a3db746db42e5..f965422199976 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts @@ -53,7 +53,7 @@ export const createRuleExecutionLogService = ( params: ClientForExecutorsParams ): Promise => { return withSecuritySpan('IRuleExecutionLogService.createClientForExecutors', async () => { - const { savedObjectsClient, context, ruleMonitoringService, ruleLastRunService } = params; + const { savedObjectsClient, context, ruleMonitoringService, ruleResultService } = params; const childLogger = logger.get('ruleExecution'); @@ -72,7 +72,7 @@ export const createRuleExecutionLogService = ( childLogger, context, ruleMonitoringService, - ruleLastRunService + ruleResultService ); }); }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts index c44b56729ab86..aa8d5e0bfee36 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service_interface.ts @@ -9,7 +9,7 @@ import type { SavedObjectsClientContract } from '@kbn/core/server'; import type { IEventLogClient } from '@kbn/event-log-plugin/server'; import type { - PublicRuleLastRunService, + PublicRuleResultService, PublicRuleMonitoringService, } from '@kbn/alerting-plugin/server/types'; import type { IRuleExecutionLogForRoutes } from './client_for_routes/client_interface'; @@ -36,6 +36,6 @@ export interface ClientForRoutesParams { export interface ClientForExecutorsParams { savedObjectsClient: SavedObjectsClientContract; ruleMonitoringService?: PublicRuleMonitoringService; - ruleLastRunService?: PublicRuleLastRunService; + ruleResultService?: PublicRuleResultService; context: RuleExecutionContext; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts index 1a287b721aec5..270a521c78e13 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts @@ -89,7 +89,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = scopedClusterClient, uiSettingsClient, ruleMonitoringService, - ruleLastRunService, + ruleResultService, } = services; const searchAfterSize = Math.min(maxSignals, DEFAULT_SEARCH_AFTER_PAGE_SIZE); @@ -98,7 +98,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = const ruleExecutionLogger = await ruleExecutionLoggerFactory({ savedObjectsClient, ruleMonitoringService, - ruleLastRunService, + ruleResultService, context: { executionId, ruleId: rule.id, From 7becc334aa2fa9f18507ec5936f50e9fa66c671b Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 22 Dec 2022 12:56:45 +0100 Subject: [PATCH 09/34] Log default new date --- .../logic/rule_execution_log/create_rule_execution_summary.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts index ef827354511f2..cae87e5034651 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -25,7 +25,7 @@ export const createRuleExecutionSummary = ( return { last_execution: { - date: rule.monitoring?.run.last_run?.timestamp ?? '', + date: rule.monitoring?.run.last_run?.timestamp ?? new Date().toISOString(), status: ruleExecutionStatus, status_order: ruleExecutionStatusToNumber(ruleExecutionStatus), message: rule.lastRun?.outcomeMsg?.join(' ') ?? '', From 70f66e4190cae48f73e0a0aafade8adbd809e317 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 22 Dec 2022 13:24:53 +0100 Subject: [PATCH 10/34] Fix writeStatusChangeToMonitoringService --- .../logic/rule_execution_log/client_for_executors/client.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 353f8de944cc4..d4dca6bde91a3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -178,9 +178,7 @@ export const createClientForExecutors = ( ruleResultService?.addLastRunWarning(message); } - if (newStatus === RuleExecutionStatus.succeeded) { - ruleResultService?.setLastRunOutcomeMessage(message); - } + ruleResultService?.setLastRunOutcomeMessage(message); if (totalSearchDurationMs) { ruleMonitoringService?.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); From 116d4d1b4e493938f58b9709e3eb473edd798018 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Tue, 27 Dec 2022 16:05:28 +0100 Subject: [PATCH 11/34] WIP --- .../plugins/alerting/server/lib/last_run_status.ts | 8 +++++++- .../common/experimental_features.ts | 2 +- .../execution_log_table/execution_log_table.tsx | 1 + .../rule_status_failed_callout.tsx | 4 +++- .../client_for_executors/client.ts | 8 +++----- .../create_rule_execution_summary.ts | 2 +- .../server/lib/detection_engine/signals/utils.ts | 3 ++- .../security_and_spaces/group1/check_privileges.ts | 12 +++++++----- 8 files changed, 25 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.ts b/x-pack/plugins/alerting/server/lib/last_run_status.ts index d2bc22cadb285..6345b57a1eaa8 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.ts @@ -38,6 +38,9 @@ export const lastRunFromState = ( if (warnings.length > 0) { outcome = RuleLastRunOutcomeValues[1]; + warnings.forEach((warn) => { + outcomeMsg.push(`WARNING: ${warn}`); + }); } // We only have a single warning field so prioritizing the alert circuit breaker over the actions circuit breaker @@ -54,12 +57,15 @@ export const lastRunFromState = ( // Overwrite outcome to be error if last run reported any errors if (errors.length > 0) { outcome = RuleLastRunOutcomeValues[2]; + errors.forEach((err) => { + outcomeMsg.push(`ERROR: ${err}`); + }); } // Optionally push outcome message reported by // rule execution to the Framework's outcome message array if (outcomeMessage) { - outcomeMsg.push(outcomeMessage); + outcomeMsg.push(`INFO: ${outcomeMessage}`); } return { diff --git a/x-pack/plugins/security_solution/common/experimental_features.ts b/x-pack/plugins/security_solution/common/experimental_features.ts index 1af1cdd9a06a8..cd6a4a6cf11c4 100644 --- a/x-pack/plugins/security_solution/common/experimental_features.ts +++ b/x-pack/plugins/security_solution/common/experimental_features.ts @@ -49,7 +49,7 @@ export const allowedExperimentalValues = Object.freeze({ * - We add a Kibana Advanced Setting that controls this behavior (on/off and log level). * - We show a table with plain execution logs on the Rule Details page. */ - extendedRuleExecutionLoggingEnabled: false, + extendedRuleExecutionLoggingEnabled: true, /** * Enables the SOC trends timerange and stats on D&R page diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx index 072d3b6f58174..30eac13ad3e3f 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx @@ -195,6 +195,7 @@ const ExecutionLogTableComponent: React.FC = ({ sortOrder: sortDirection, }); const items = events?.events ?? []; + debugger; const maxEvents = events?.total ?? 0; // Cache UUID field from data view as it can be expensive to iterate all data view fields diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_failed_callout.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_failed_callout.tsx index 042484634a925..ff5b559eb4b32 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_failed_callout.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_failed_callout.tsx @@ -40,7 +40,9 @@ const RuleStatusFailedCallOutComponent: React.FC = iconType="alert" data-test-subj="ruleStatusFailedCallOut" > -

{message}

+ {message.split('\n').map((line) => ( +

{line}

+ ))} ); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index d4dca6bde91a3..c06bdc1736795 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -172,14 +172,12 @@ export const createClientForExecutors = ( if (newStatus === RuleExecutionStatus.failed) { ruleResultService?.addLastRunError(message); - } - - if (newStatus === RuleExecutionStatus['partial failure']) { + } else if (newStatus === RuleExecutionStatus['partial failure']) { ruleResultService?.addLastRunWarning(message); + } else { + ruleResultService?.setLastRunOutcomeMessage(message); } - ruleResultService?.setLastRunOutcomeMessage(message); - if (totalSearchDurationMs) { ruleMonitoringService?.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts index cae87e5034651..35211c81db0f7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -28,7 +28,7 @@ export const createRuleExecutionSummary = ( date: rule.monitoring?.run.last_run?.timestamp ?? new Date().toISOString(), status: ruleExecutionStatus, status_order: ruleExecutionStatusToNumber(ruleExecutionStatus), - message: rule.lastRun?.outcomeMsg?.join(' ') ?? '', + message: rule.lastRun?.outcomeMsg?.join('. \n') ?? '', metrics: { total_indexing_duration_ms: rule.monitoring?.run.last_run.metrics.total_indexing_duration_ms ?? undefined, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 3c6fec4afdcdf..d7f86141a6d91 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -241,8 +241,9 @@ export const getExceptions = async ({ client: ExceptionListClient; lists: ListArray; }): Promise => { - if (lists.length > 0) { + if (true) { try { + throw new Error('no exception lists found'); const listIds = lists.map(({ list_id: listId }) => listId); const namespaceTypes = lists.map(({ namespace_type: namespaceType }) => namespaceType); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts index 0b7de71969a7b..42bdf53d517bf 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts @@ -52,10 +52,10 @@ export default ({ getService }: FtrProviderContext) => { describe('should set status to partial failure when user has no access', () => { const indexTestCases = [ - ['host_alias'], - ['host_alias', 'auditbeat-8.0.0'], + // ['host_alias'], + // ['host_alias', 'auditbeat-8.0.0'], ['host_alias*'], - ['host_alias*', 'auditbeat-*'], + // ['host_alias*', 'auditbeat-*'], ]; indexTestCases.forEach((index) => { it(`for KQL rule with index param: ${index}`, async () => { @@ -85,7 +85,7 @@ export default ({ getService }: FtrProviderContext) => { await deleteUserAndRole(getService, ROLES.detections_admin); }); - it(`for threshold rule with index param: ${index}`, async () => { + it.only(`for threshold rule with index param: ${index}`, async () => { const rule: ThresholdRuleCreateProps = { ...getThresholdRuleForSignalTesting(index), threshold: { @@ -98,18 +98,20 @@ export default ({ getService }: FtrProviderContext) => { user: ROLES.detections_admin, pass: 'changeme', }); + await waitForRuleSuccessOrStatus( supertest, log, id, RuleExecutionStatus['partial failure'] ); + const { body } = await supertest .get(DETECTION_ENGINE_RULES_URL) .set('kbn-xsrf', 'true') .query({ id }) .expect(200); - + // expect(body).to.eql({}); // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe expect(body?.execution_summary?.last_execution.message).to.eql( `This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` From 6fd0c07bacdb61ae4925ab2b0d9cb51f4fa6a47d Mon Sep 17 00:00:00 2001 From: jpdjere Date: Tue, 27 Dec 2022 23:53:12 +0100 Subject: [PATCH 12/34] WIP --- .../plugins/alerting/server/lib/last_run_status.ts | 10 +++------- .../common/experimental_features.ts | 2 +- .../execution_log_table/execution_log_table.tsx | 1 - .../client_for_executors/client.ts | 6 +++--- .../server/lib/detection_engine/signals/utils.ts | 3 +-- .../security_and_spaces/group1/check_privileges.ts | 12 +++++------- 6 files changed, 13 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.ts b/x-pack/plugins/alerting/server/lib/last_run_status.ts index 6345b57a1eaa8..ced6f87cc198a 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.ts @@ -38,9 +38,7 @@ export const lastRunFromState = ( if (warnings.length > 0) { outcome = RuleLastRunOutcomeValues[1]; - warnings.forEach((warn) => { - outcomeMsg.push(`WARNING: ${warn}`); - }); + outcomeMsg.push(...warnings); } // We only have a single warning field so prioritizing the alert circuit breaker over the actions circuit breaker @@ -57,15 +55,13 @@ export const lastRunFromState = ( // Overwrite outcome to be error if last run reported any errors if (errors.length > 0) { outcome = RuleLastRunOutcomeValues[2]; - errors.forEach((err) => { - outcomeMsg.push(`ERROR: ${err}`); - }); + outcomeMsg.push(...errors); } // Optionally push outcome message reported by // rule execution to the Framework's outcome message array if (outcomeMessage) { - outcomeMsg.push(`INFO: ${outcomeMessage}`); + outcomeMsg.push(outcomeMessage); } return { diff --git a/x-pack/plugins/security_solution/common/experimental_features.ts b/x-pack/plugins/security_solution/common/experimental_features.ts index cd6a4a6cf11c4..1af1cdd9a06a8 100644 --- a/x-pack/plugins/security_solution/common/experimental_features.ts +++ b/x-pack/plugins/security_solution/common/experimental_features.ts @@ -49,7 +49,7 @@ export const allowedExperimentalValues = Object.freeze({ * - We add a Kibana Advanced Setting that controls this behavior (on/off and log level). * - We show a table with plain execution logs on the Rule Details page. */ - extendedRuleExecutionLoggingEnabled: true, + extendedRuleExecutionLoggingEnabled: false, /** * Enables the SOC trends timerange and stats on D&R page diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx index 30eac13ad3e3f..072d3b6f58174 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/execution_log_table/execution_log_table.tsx @@ -195,7 +195,6 @@ const ExecutionLogTableComponent: React.FC = ({ sortOrder: sortDirection, }); const items = events?.events ?? []; - debugger; const maxEvents = events?.total ?? 0; // Cache UUID field from data view as it can be expensive to iterate all data view fields diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index c06bdc1736795..3d09861cbf928 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -171,11 +171,11 @@ export const createClientForExecutors = ( } = metrics ?? {}; if (newStatus === RuleExecutionStatus.failed) { - ruleResultService?.addLastRunError(message); + ruleResultService?.addLastRunError(`ERROR: ${message}`); } else if (newStatus === RuleExecutionStatus['partial failure']) { - ruleResultService?.addLastRunWarning(message); + ruleResultService?.addLastRunWarning(`WARNING: ${message}`); } else { - ruleResultService?.setLastRunOutcomeMessage(message); + ruleResultService?.setLastRunOutcomeMessage(`INFO: ${message}`); } if (totalSearchDurationMs) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index d7f86141a6d91..3c6fec4afdcdf 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -241,9 +241,8 @@ export const getExceptions = async ({ client: ExceptionListClient; lists: ListArray; }): Promise => { - if (true) { + if (lists.length > 0) { try { - throw new Error('no exception lists found'); const listIds = lists.map(({ list_id: listId }) => listId); const namespaceTypes = lists.map(({ namespace_type: namespaceType }) => namespaceType); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts index 42bdf53d517bf..0b7de71969a7b 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts @@ -52,10 +52,10 @@ export default ({ getService }: FtrProviderContext) => { describe('should set status to partial failure when user has no access', () => { const indexTestCases = [ - // ['host_alias'], - // ['host_alias', 'auditbeat-8.0.0'], + ['host_alias'], + ['host_alias', 'auditbeat-8.0.0'], ['host_alias*'], - // ['host_alias*', 'auditbeat-*'], + ['host_alias*', 'auditbeat-*'], ]; indexTestCases.forEach((index) => { it(`for KQL rule with index param: ${index}`, async () => { @@ -85,7 +85,7 @@ export default ({ getService }: FtrProviderContext) => { await deleteUserAndRole(getService, ROLES.detections_admin); }); - it.only(`for threshold rule with index param: ${index}`, async () => { + it(`for threshold rule with index param: ${index}`, async () => { const rule: ThresholdRuleCreateProps = { ...getThresholdRuleForSignalTesting(index), threshold: { @@ -98,20 +98,18 @@ export default ({ getService }: FtrProviderContext) => { user: ROLES.detections_admin, pass: 'changeme', }); - await waitForRuleSuccessOrStatus( supertest, log, id, RuleExecutionStatus['partial failure'] ); - const { body } = await supertest .get(DETECTION_ENGINE_RULES_URL) .set('kbn-xsrf', 'true') .query({ id }) .expect(200); - // expect(body).to.eql({}); + // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe expect(body?.execution_summary?.last_execution.message).to.eql( `This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` From 11acb9a430dea812e5c10f237d55460e95d93136 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Wed, 28 Dec 2022 12:37:45 +0100 Subject: [PATCH 13/34] WIP --- .../rule_status_badge.tsx | 4 +++- .../client_for_executors/client.ts | 20 +++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_badge.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_badge.tsx index b80445403959f..fff3fe48074ce 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_badge.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_execution_status/rule_status_badge.tsx @@ -30,7 +30,9 @@ const RuleStatusBadgeComponent = ({ status, message }: RuleStatusBadgeProps) => const statusColor = getStatusColor(status); return ( ( +

{line}

+ ))} healthColor={statusColor} dataTestSubj="ruleExecutionStatus" > diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 3d09861cbf928..1d684b73b1438 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -170,14 +170,6 @@ export const createClientForExecutors = ( execution_gap_duration_s: executionGapDurationS, } = metrics ?? {}; - if (newStatus === RuleExecutionStatus.failed) { - ruleResultService?.addLastRunError(`ERROR: ${message}`); - } else if (newStatus === RuleExecutionStatus['partial failure']) { - ruleResultService?.addLastRunWarning(`WARNING: ${message}`); - } else { - ruleResultService?.setLastRunOutcomeMessage(`INFO: ${message}`); - } - if (totalSearchDurationMs) { ruleMonitoringService?.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); } @@ -189,6 +181,18 @@ export const createClientForExecutors = ( if (executionGapDurationS) { ruleMonitoringService?.setLastRunMetricsGapDurationS(executionGapDurationS); } + + if (!message) { + return; + } + + if (newStatus === RuleExecutionStatus.failed) { + ruleResultService?.addLastRunError(`ERROR: ${message}`); + } else if (newStatus === RuleExecutionStatus['partial failure']) { + ruleResultService?.addLastRunWarning(`WARNING: ${message}`); + } else { + ruleResultService?.setLastRunOutcomeMessage(`INFO: ${message}`); + } }; const writeStatusChangeToEventLog = (args: NormalizedStatusChangeArgs): void => { From 831bcd6732d85271a79f2e8bd09844423849646e Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 13:58:45 +0100 Subject: [PATCH 14/34] Fix last run tests --- .../alerting/server/lib/last_run_status.test.ts | 12 +++++++++--- .../client_for_executors/client.ts | 15 +++++++++++---- .../client_for_executors/client_interface.ts | 15 ++++++++++----- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.test.ts b/x-pack/plugins/alerting/server/lib/last_run_status.test.ts index d6b6f62b1b60a..2bb7c8e19d87e 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.test.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.test.ts @@ -71,7 +71,10 @@ describe('lastRunFromState', () => { ); expect(result.lastRun.outcome).toEqual('warning'); - expect(result.lastRun.outcomeMsg).toEqual(['Rule execution reported a warning']); + expect(result.lastRun.outcomeMsg).toEqual([ + 'MOCK_WARNING', + 'Rule execution reported a warning', + ]); expect(result.lastRun.warning).toEqual(null); expect(result.lastRun.alertsCount).toEqual({ @@ -136,12 +139,13 @@ describe('lastRunFromState', () => { }, getRuleResultService({ warnings: ['MOCK_WARNING'], - outcomeMessage: 'Rule execution reported a warning', + outcomeMessage: ruleExecutionOutcomeMessage, }) ); expect(result.lastRun.outcome).toEqual('warning'); expect(result.lastRun.outcomeMsg).toEqual([ + 'MOCK_WARNING', frameworkOutcomeMessage, ruleExecutionOutcomeMessage, ]); @@ -171,6 +175,7 @@ describe('lastRunFromState', () => { expect(result.lastRun.outcome).toEqual('warning'); expect(result.lastRun.outcomeMsg).toEqual([ + 'MOCK_WARNING', frameworkOutcomeMessage, ruleExecutionOutcomeMessage, ]); @@ -198,6 +203,7 @@ describe('lastRunFromState', () => { expect(result.lastRun.outcome).toEqual('failed'); expect(result.lastRun.outcomeMsg).toEqual([ 'Rule reported more than the maximum number of alerts in a single run. Alerts may be missed and recovery notifications may be delayed', + 'MOCK_ERROR', 'Rule execution reported an error', ]); expect(result.lastRun.warning).toEqual('maxAlerts'); @@ -209,4 +215,4 @@ describe('lastRunFromState', () => { ignored: 0, }); }); -}); +}); \ No newline at end of file diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 1d684b73b1438..bb8fb535fabbc 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -164,6 +164,11 @@ export const createClientForExecutors = ( args: NormalizedStatusChangeArgs ): Promise => { const { newStatus, message, metrics } = args; + + if (newStatus === RuleExecutionStatus.running) { + return; + } + const { total_search_duration_ms: totalSearchDurationMs, total_indexing_duration_ms: totalIndexingDurationMs, @@ -182,10 +187,6 @@ export const createClientForExecutors = ( ruleMonitoringService?.setLastRunMetricsGapDurationS(executionGapDurationS); } - if (!message) { - return; - } - if (newStatus === RuleExecutionStatus.failed) { ruleResultService?.addLastRunError(`ERROR: ${message}`); } else if (newStatus === RuleExecutionStatus['partial failure']) { @@ -232,6 +233,12 @@ interface NormalizedStatusChangeArgs { } const normalizeStatusChangeArgs = (args: StatusChangeArgs): NormalizedStatusChangeArgs => { + if (args.newStatus === RuleExecutionStatus.running) { + return { + newStatus: args.newStatus, + message: '', + }; + } const { newStatus, message, metrics } = args; return { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts index 5e48a43e949c3..f907c04ed896c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts @@ -106,11 +106,16 @@ export interface RuleExecutionContext { /** * Information about the status change event. */ -export interface StatusChangeArgs { - newStatus: RuleExecutionStatus; - message?: string; - metrics?: MetricsArgs; -} +export type StatusChangeArgs = + | { + newStatus: RuleExecutionStatus.running; + metrics?: MetricsArgs; + } + | { + newStatus: Exclude; + metrics?: MetricsArgs; + message: string; + }; export interface MetricsArgs { searchDurations?: string[]; From 6601458776dc4fd13a33dea9e72116a4e4ad5eb1 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 14:23:55 +0100 Subject: [PATCH 15/34] Delete execution_saved_object files --- .../client_for_routes/client.ts | 2 - .../saved_objects_client.ts | 133 ------------------ .../saved_objects_type.ts | 80 ----------- .../saved_objects_utils.ts | 36 ----- .../logic/rule_execution_log/index.ts | 2 - .../logic/rule_execution_log/service.ts | 6 +- .../security_solution/server/saved_objects.ts | 2 - 7 files changed, 2 insertions(+), 259 deletions(-) delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_client.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_type.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_utils.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts index 4c16216acaf1e..f760eb9cd2455 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_routes/client.ts @@ -15,7 +15,6 @@ import type { } from '../../../../../../../common/detection_engine/rule_monitoring'; import type { IEventLogReader } from '../event_log/event_log_reader'; -import type { IRuleExecutionSavedObjectsClient } from '../execution_saved_object/saved_objects_client'; import type { GetExecutionEventsArgs, GetExecutionResultsArgs, @@ -23,7 +22,6 @@ import type { } from './client_interface'; export const createClientForRoutes = ( - soClient: IRuleExecutionSavedObjectsClient, eventLog: IEventLogReader, logger: Logger ): IRuleExecutionLogForRoutes => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_client.ts deleted file mode 100644 index 08a7c49f739c0..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_client.ts +++ /dev/null @@ -1,133 +0,0 @@ -/* - * 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 { Logger, SavedObjectsClientContract } from '@kbn/core/server'; -import { SavedObjectsErrorHelpers } from '@kbn/core/server'; - -import { withSecuritySpan } from '../../../../../../utils/with_security_span'; - -import type { RuleExecutionSavedObject, RuleExecutionAttributes } from './saved_objects_type'; -import { RULE_EXECUTION_SO_TYPE } from './saved_objects_type'; -import { - getRuleExecutionSoId, - getRuleExecutionSoReferences, - extractRuleIdFromReferences, -} from './saved_objects_utils'; - -export interface IRuleExecutionSavedObjectsClient { - createOrUpdate( - ruleId: string, - attributes: RuleExecutionAttributes - ): Promise; - - delete(ruleId: string): Promise; - - getOneByRuleId(ruleId: string): Promise; - - getManyByRuleIds(ruleIds: string[]): Promise; -} - -export type RuleExecutionSavedObjectsByRuleId = Record; - -export const createRuleExecutionSavedObjectsClient = ( - soClient: SavedObjectsClientContract, - logger: Logger -): IRuleExecutionSavedObjectsClient => { - return { - async createOrUpdate(ruleId, attributes) { - const id = getRuleExecutionSoId(ruleId); - const references = getRuleExecutionSoReferences(ruleId); - - const result = await withSecuritySpan('createOrUpdateRuleExecutionSO', () => { - return soClient.create(RULE_EXECUTION_SO_TYPE, attributes, { - id, - references, - overwrite: true, - }); - }); - - return result; - }, - - async delete(ruleId) { - try { - const id = getRuleExecutionSoId(ruleId); - await withSecuritySpan('deleteRuleExecutionSO', () => { - return soClient.delete(RULE_EXECUTION_SO_TYPE, id); - }); - } catch (e) { - if (SavedObjectsErrorHelpers.isNotFoundError(e)) { - return; - } - throw e; - } - }, - - async getOneByRuleId(ruleId) { - try { - const id = getRuleExecutionSoId(ruleId); - return await withSecuritySpan('getRuleExecutionSO', () => { - return soClient.get(RULE_EXECUTION_SO_TYPE, id); - }); - } catch (e) { - if (SavedObjectsErrorHelpers.isNotFoundError(e)) { - return null; - } - throw e; - } - }, - - async getManyByRuleIds(ruleIds) { - const ids = ruleIds - .map((id) => getRuleExecutionSoId(id)) - .map((id) => ({ id, type: RULE_EXECUTION_SO_TYPE })); - - const response = await withSecuritySpan('bulkGetRuleExecutionSOs', () => { - return soClient.bulkGet(ids); - }); - - const result = prepopulateRuleExecutionSavedObjectsByRuleId(ruleIds); - - response.saved_objects.forEach((so) => { - // NOTE: We need to explicitly check that this saved object is not an error result and has references in it. - // "Saved object" may not actually contain most of its properties (despite the fact that they are required - // in its TypeScript interface), for example if it wasn't found. In this case it will look like that: - // { - // id: '64b51590-a87e-5afc-9ede-906c3f3483b7', - // type: 'siem-detection-engine-rule-execution-info', - // error: { - // statusCode: 404, - // error: 'Not Found', - // message: 'Saved object [siem-detection-engine-rule-execution-info/64b51590-a87e-5afc-9ede-906c3f3483b7] not found' - // }, - // namespaces: undefined - // } - const hasReferences = !so.error && so.references && Array.isArray(so.references); - const references = hasReferences ? so.references : []; - - const ruleId = extractRuleIdFromReferences(references); - if (ruleId) { - result[ruleId] = so; - } - - if (so.error && so.error.statusCode !== 404) { - logger.error(so.error.message); - } - }); - - return result; - }, - }; -}; - -const prepopulateRuleExecutionSavedObjectsByRuleId = (ruleIds: string[]) => { - const result: RuleExecutionSavedObjectsByRuleId = {}; - ruleIds.forEach((ruleId) => { - result[ruleId] = null; - }); - return result; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_type.ts deleted file mode 100644 index 1e0a2e74cadcd..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_type.ts +++ /dev/null @@ -1,80 +0,0 @@ -/* - * 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 { SavedObject, SavedObjectsType } from '@kbn/core/server'; -import type { - RuleExecutionMetrics, - RuleExecutionStatus, - RuleExecutionStatusOrder, -} from '../../../../../../../common/detection_engine/rule_monitoring'; - -export const RULE_EXECUTION_SO_TYPE = 'siem-detection-engine-rule-execution-info'; - -/** - * This side-car SO stores information about rule executions (like last status and metrics). - * Eventually we're going to replace it with data stored in the rule itself: - * https://github.com/elastic/kibana/issues/112193 - */ -export type RuleExecutionSavedObject = SavedObject; - -export interface RuleExecutionAttributes { - last_execution: { - date: string; - status: RuleExecutionStatus; - status_order: RuleExecutionStatusOrder; - message: string; - metrics: RuleExecutionMetrics; - }; -} - -const ruleExecutionMappings: SavedObjectsType['mappings'] = { - properties: { - last_execution: { - type: 'object', - properties: { - date: { - type: 'date', - }, - status: { - type: 'keyword', - ignore_above: 1024, - }, - status_order: { - type: 'long', - }, - message: { - type: 'text', - }, - metrics: { - type: 'object', - properties: { - total_search_duration_ms: { - type: 'long', - }, - total_indexing_duration_ms: { - type: 'long', - }, - total_enrichment_duration_ms: { - type: 'long', - }, - execution_gap_duration_s: { - type: 'long', - }, - }, - }, - }, - }, - }, -}; - -export const ruleExecutionType: SavedObjectsType = { - name: RULE_EXECUTION_SO_TYPE, - mappings: ruleExecutionMappings, - hidden: false, - namespaceType: 'multiple-isolated', - convertToMultiNamespaceTypeVersion: '8.0.0', -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_utils.ts deleted file mode 100644 index 9d84a1731b088..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/execution_saved_object/saved_objects_utils.ts +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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 { v5 as uuidv5 } from 'uuid'; -import type { SavedObjectReference } from '@kbn/core/server'; -import { RULE_EXECUTION_SO_TYPE } from './saved_objects_type'; - -export const getRuleExecutionSoId = (ruleId: string): string => { - // The uuidv5 namespace constant (uuidv5.DNS) is arbitrary. - return uuidv5(`${RULE_EXECUTION_SO_TYPE}:${ruleId}`, uuidv5.DNS); -}; - -const RULE_REFERENCE_TYPE = 'alert'; -const RULE_REFERENCE_NAME = 'alert_0'; - -export const getRuleExecutionSoReferences = (ruleId: string): SavedObjectReference[] => { - return [ - { - id: ruleId, - type: RULE_REFERENCE_TYPE, - name: RULE_REFERENCE_NAME, - }, - ]; -}; - -export const extractRuleIdFromReferences = (references: SavedObjectReference[]): string | null => { - const foundReference = references.find( - (r) => r.type === RULE_REFERENCE_TYPE && r.name === RULE_REFERENCE_NAME - ); - - return foundReference?.id || null; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts index faea572babfc8..40f9babb53af5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/index.ts @@ -10,8 +10,6 @@ export * from './client_for_routes/client_interface'; export * from './service_interface'; export * from './service'; -export { ruleExecutionType } from './execution_saved_object/saved_objects_type'; - export { RULE_EXECUTION_LOG_PROVIDER } from './event_log/constants'; export { createRuleExecutionSummary } from './create_rule_execution_summary'; export * from './utils/normalization'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts index f965422199976..c45f4573a139c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts @@ -21,7 +21,6 @@ import { createClientForExecutors } from './client_for_executors/client'; import { registerEventLogProvider } from './event_log/register_event_log_provider'; import { createEventLogReader } from './event_log/event_log_reader'; import { createEventLogWriter } from './event_log/event_log_writer'; -import { createRuleExecutionSavedObjectsClient } from './execution_saved_object/saved_objects_client'; import { fetchRuleExecutionSettings } from './execution_settings/fetch_rule_execution_settings'; import type { ClientForExecutorsParams, @@ -41,12 +40,11 @@ export const createRuleExecutionLogService = ( }, createClientForRoutes: (params: ClientForRoutesParams): IRuleExecutionLogForRoutes => { - const { savedObjectsClient, eventLogClient } = params; + const { eventLogClient } = params; - const soClient = createRuleExecutionSavedObjectsClient(savedObjectsClient, logger); const eventLogReader = createEventLogReader(eventLogClient); - return createClientForRoutes(soClient, eventLogReader, logger); + return createClientForRoutes(eventLogReader, logger); }, createClientForExecutors: ( diff --git a/x-pack/plugins/security_solution/server/saved_objects.ts b/x-pack/plugins/security_solution/server/saved_objects.ts index bc1d86add3767..7ce17ff0c0b51 100644 --- a/x-pack/plugins/security_solution/server/saved_objects.ts +++ b/x-pack/plugins/security_solution/server/saved_objects.ts @@ -10,7 +10,6 @@ import type { CoreSetup } from '@kbn/core/server'; import { noteType, pinnedEventType, timelineType } from './lib/timeline/saved_object_mappings'; // eslint-disable-next-line no-restricted-imports import { legacyType as legacyRuleActionsType } from './lib/detection_engine/rule_actions_legacy'; -import { ruleExecutionType } from './lib/detection_engine/rule_monitoring'; import { ruleAssetType } from './lib/detection_engine/prebuilt_rules/logic/rule_asset/rule_asset_saved_object_mappings'; import { type as signalsMigrationType } from './lib/detection_engine/migrations/saved_objects'; import { @@ -22,7 +21,6 @@ const types = [ noteType, pinnedEventType, legacyRuleActionsType, - ruleExecutionType, ruleAssetType, timelineType, exceptionsArtifactType, From caf9c5bab28640b8905135a675f57c51c25ec79a Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 14:25:02 +0100 Subject: [PATCH 16/34] Add SO to unused_types --- .../src/core/unused_types.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts index 60ace4e28de0e..f30ee906e714d 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts @@ -33,6 +33,8 @@ export const REMOVED_TYPES: string[] = [ 'osquery-usage-metric', // Was removed in 8.1 https://github.com/elastic/kibana/issues/91265 'siem-detection-engine-rule-status', + // Was removed in 8.7 (TBC) https://github.com/elastic/kibana/issues/130966 + 'siem-detection-engine-rule-execution-info', // Was removed in 7.16 'timelion-sheet', // Removed in 8.3 https://github.com/elastic/kibana/issues/127745 From 8c41d21df3459a7e5e6555d7f4e817a9de24e5cb Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 15:00:21 +0100 Subject: [PATCH 17/34] Fix type --- .../client_for_executors/client_interface.ts | 20 +++++++++---------- .../rule_preview/api/preview_rules/route.ts | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts index f907c04ed896c..54d9055f1874d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts @@ -103,19 +103,19 @@ export interface RuleExecutionContext { spaceId: string; } +export interface RunningStatusChangeArgs { + newStatus: RuleExecutionStatus.running; +} + +export interface StatusChangeArgsWithMessage { + newStatus: Exclude; + metrics?: MetricsArgs; + message: string; +} /** * Information about the status change event. */ -export type StatusChangeArgs = - | { - newStatus: RuleExecutionStatus.running; - metrics?: MetricsArgs; - } - | { - newStatus: Exclude; - metrics?: MetricsArgs; - message: string; - }; +export type StatusChangeArgs = RunningStatusChangeArgs | StatusChangeArgsWithMessage; export interface MetricsArgs { searchDurations?: string[]; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts index 24aaf9ac4cd0c..ce91f9cd1ee79 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts @@ -39,7 +39,7 @@ import { throwAuthzError } from '../../../../machine_learning/validation'; import { buildRouteValidation } from '../../../../../utils/build_validation/route_validation'; import type { SecuritySolutionPluginRouter } from '../../../../../types'; -import type { RuleExecutionContext, StatusChangeArgs } from '../../../rule_monitoring'; +import type { RuleExecutionContext, StatusChangeArgsWithMessage } from '../../../rule_monitoring'; import type { ConfigType } from '../../../../../config'; import { alertInstanceFactoryStub } from '../../../signals/preview/alert_instance_factory_stub'; @@ -123,7 +123,7 @@ export const previewRulesRoute = async ( const spaceId = siemClient.getSpaceId(); const previewId = uuidv4(); const username = security?.authc.getCurrentUser(request)?.username; - const loggedStatusChanges: Array = []; + const loggedStatusChanges: Array = []; const previewRuleExecutionLogger = createPreviewRuleExecutionLogger(loggedStatusChanges); const runState: Record = {}; const logs: RulePreviewLogs[] = []; @@ -267,7 +267,7 @@ export const previewRulesRoute = async ( const errors = loggedStatusChanges .filter((item) => item.newStatus === RuleExecutionStatus.failed) - .map((item) => item.message ?? 'Unkown Error'); + .map((item) => item.message ?? 'Unknown Error'); const warnings = loggedStatusChanges .filter((item) => item.newStatus === RuleExecutionStatus['partial failure']) From 61fc0a8a7f2697bad63e5f188794753bc3de1337 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 16:46:18 +0100 Subject: [PATCH 18/34] Fix tests --- ...grations_state_action_machine.test.ts.snap | 30 +++++++++++++++++++ .../src/initial_state.test.ts | 5 ++++ .../migrations/check_registered_types.test.ts | 1 - 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/__snapshots__/migrations_state_action_machine.test.ts.snap b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/__snapshots__/migrations_state_action_machine.test.ts.snap index b0c7fd0f37289..28d12601d23f3 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/__snapshots__/migrations_state_action_machine.test.ts.snap +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/__snapshots__/migrations_state_action_machine.test.ts.snap @@ -96,6 +96,11 @@ Object { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", @@ -282,6 +287,11 @@ Object { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", @@ -472,6 +482,11 @@ Object { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", @@ -666,6 +681,11 @@ Object { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", @@ -902,6 +922,11 @@ Object { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", @@ -1099,6 +1124,11 @@ Object { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/initial_state.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/initial_state.test.ts index ee99e599a998a..8a66bac6ce9fe 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/initial_state.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/initial_state.test.ts @@ -133,6 +133,11 @@ describe('createInitialState', () => { "type": "server", }, }, + Object { + "term": Object { + "type": "siem-detection-engine-rule-execution-info", + }, + }, Object { "term": Object { "type": "siem-detection-engine-rule-status", diff --git a/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts index 54a61456ac769..347401e9b30f9 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts @@ -131,7 +131,6 @@ describe('checking migration metadata changes on all registered SO types', () => "security-rule": "e0dfdba5d66139d0300723b2e6672993cd4a11f3", "security-solution-signals-migration": "e65933e32926e0ca385415bd44fc6da0b6d3d419", "siem-detection-engine-rule-actions": "d4b5934c0c0e4ccdf509a41000eb0bee07be0c28", - "siem-detection-engine-rule-execution-info": "b92d51db7b7d591758d3e85892a91064aff01ff8", "siem-ui-timeline": "95474f10662802e2f9ea068b45bf69212a2f5842", "siem-ui-timeline-note": "08c71dc0b8b8018a67e80beb4659a078404c223d", "siem-ui-timeline-pinned-event": "e2697b38751506c7fce6e8b7207a830483dc4283", From fc35960120c5749a85d97c1482a780f22e3a32f3 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 17:26:44 +0100 Subject: [PATCH 19/34] Fix test --- .../security_and_spaces/group1/check_privileges.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts index 0b7de71969a7b..c9af9f5e65065 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts @@ -79,12 +79,18 @@ export default ({ getService }: FtrProviderContext) => { // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe expect(body?.execution_summary?.last_execution.message).to.eql( - `This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` + `WARNING: This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` ); await deleteUserAndRole(getService, ROLES.detections_admin); }); + }); + const thresholdIndexTestCases = [ + ['host_alias', 'auditbeat-8.0.0'], + ['host_alias*', 'auditbeat-*'], + ]; + thresholdIndexTestCases.forEach((index) => { it(`for threshold rule with index param: ${index}`, async () => { const rule: ThresholdRuleCreateProps = { ...getThresholdRuleForSignalTesting(index), @@ -112,7 +118,7 @@ export default ({ getService }: FtrProviderContext) => { // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe expect(body?.execution_summary?.last_execution.message).to.eql( - `This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` + `WARNING: This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]. \nERROR: An error occurred during rule execution: message: "Aggregations were missing on threshold rule search result"` ); await deleteUserAndRole(getService, ROLES.detections_admin); From 67cbc12a381cb8ae1e65213fbe64219b132d241a Mon Sep 17 00:00:00 2001 From: jpdjere Date: Thu, 29 Dec 2022 17:39:29 +0100 Subject: [PATCH 20/34] Lint fix --- x-pack/plugins/alerting/server/lib/last_run_status.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.test.ts b/x-pack/plugins/alerting/server/lib/last_run_status.test.ts index 2bb7c8e19d87e..d43f495ebe632 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.test.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.test.ts @@ -215,4 +215,4 @@ describe('lastRunFromState', () => { ignored: 0, }); }); -}); \ No newline at end of file +}); From 1e4a35193db0e64d7fdfa3596843f1a3b88155ae Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Thu, 19 Jan 2023 18:28:46 +0100 Subject: [PATCH 21/34] set always outcome message --- .../logic/rule_execution_log/client_for_executors/client.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index bb8fb535fabbc..27dcca825563d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -194,6 +194,8 @@ export const createClientForExecutors = ( } else { ruleResultService?.setLastRunOutcomeMessage(`INFO: ${message}`); } + + ruleResultService?.setLastRunOutcomeMessage(message); }; const writeStatusChangeToEventLog = (args: NormalizedStatusChangeArgs): void => { From ccbec38871a660c38d73b02c2592355abcf97e64 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Thu, 19 Jan 2023 18:30:11 +0100 Subject: [PATCH 22/34] remove message type prefixes --- .../logic/rule_execution_log/client_for_executors/client.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 27dcca825563d..dd096987cf60a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -188,11 +188,9 @@ export const createClientForExecutors = ( } if (newStatus === RuleExecutionStatus.failed) { - ruleResultService?.addLastRunError(`ERROR: ${message}`); + ruleResultService?.addLastRunError(message); } else if (newStatus === RuleExecutionStatus['partial failure']) { - ruleResultService?.addLastRunWarning(`WARNING: ${message}`); - } else { - ruleResultService?.setLastRunOutcomeMessage(`INFO: ${message}`); + ruleResultService?.addLastRunWarning(message); } ruleResultService?.setLastRunOutcomeMessage(message); From bf65057f5ee2edc01020fafe5e3a9a15f4091789 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Thu, 19 Jan 2023 18:30:54 +0100 Subject: [PATCH 23/34] revert last_run_status changes --- x-pack/plugins/alerting/server/lib/last_run_status.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.ts b/x-pack/plugins/alerting/server/lib/last_run_status.ts index ced6f87cc198a..d2bc22cadb285 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.ts @@ -38,7 +38,6 @@ export const lastRunFromState = ( if (warnings.length > 0) { outcome = RuleLastRunOutcomeValues[1]; - outcomeMsg.push(...warnings); } // We only have a single warning field so prioritizing the alert circuit breaker over the actions circuit breaker @@ -55,7 +54,6 @@ export const lastRunFromState = ( // Overwrite outcome to be error if last run reported any errors if (errors.length > 0) { outcome = RuleLastRunOutcomeValues[2]; - outcomeMsg.push(...errors); } // Optionally push outcome message reported by From 29a90732aa89ccc501733ff0f0f5e9d4e257487a Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Thu, 19 Jan 2023 18:31:49 +0100 Subject: [PATCH 24/34] do not add an end line period --- .../logic/rule_execution_log/create_rule_execution_summary.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts index 35211c81db0f7..a7b7153feea96 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -28,7 +28,7 @@ export const createRuleExecutionSummary = ( date: rule.monitoring?.run.last_run?.timestamp ?? new Date().toISOString(), status: ruleExecutionStatus, status_order: ruleExecutionStatusToNumber(ruleExecutionStatus), - message: rule.lastRun?.outcomeMsg?.join('. \n') ?? '', + message: rule.lastRun?.outcomeMsg?.join(' \n') ?? '', metrics: { total_indexing_duration_ms: rule.monitoring?.run.last_run.metrics.total_indexing_duration_ms ?? undefined, From ba28aaef02b2f4955d0f2a17460896c77b508279 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 20 Jan 2023 15:18:14 +0100 Subject: [PATCH 25/34] take into account rule's 'running' status --- .../create_rule_execution_summary.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts index a7b7153feea96..c12dbf9c36d9c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts @@ -11,17 +11,30 @@ import type { RuleExecutionSummary } from '../../../../../../common/detection_en import { ruleLastRunOutcomeToExecutionStatus, ruleExecutionStatusToNumber, + RuleExecutionStatus, } from '../../../../../../common/detection_engine/rule_monitoring'; import type { RuleParams } from '../../../rule_schema'; export const createRuleExecutionSummary = ( rule: SanitizedRule | ResolvedSanitizedRule ): RuleExecutionSummary | null => { + if (rule.running) { + return { + last_execution: { + date: new Date().toISOString(), + message: '', + metrics: {}, + status: RuleExecutionStatus.running, + status_order: ruleExecutionStatusToNumber(RuleExecutionStatus.running), + }, + }; + } + if (!rule.lastRun) { return null; } - const ruleExecutionStatus = ruleLastRunOutcomeToExecutionStatus(rule.lastRun?.outcome); + const ruleExecutionStatus = ruleLastRunOutcomeToExecutionStatus(rule.lastRun.outcome); return { last_execution: { From eec24ed34e5b672412d01a9314cab9ecb52f6135 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Sun, 22 Jan 2023 13:13:00 +0100 Subject: [PATCH 26/34] fix unit tests --- .../alerting/server/lib/last_run_status.test.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/alerting/server/lib/last_run_status.test.ts b/x-pack/plugins/alerting/server/lib/last_run_status.test.ts index d43f495ebe632..d6b6f62b1b60a 100644 --- a/x-pack/plugins/alerting/server/lib/last_run_status.test.ts +++ b/x-pack/plugins/alerting/server/lib/last_run_status.test.ts @@ -71,10 +71,7 @@ describe('lastRunFromState', () => { ); expect(result.lastRun.outcome).toEqual('warning'); - expect(result.lastRun.outcomeMsg).toEqual([ - 'MOCK_WARNING', - 'Rule execution reported a warning', - ]); + expect(result.lastRun.outcomeMsg).toEqual(['Rule execution reported a warning']); expect(result.lastRun.warning).toEqual(null); expect(result.lastRun.alertsCount).toEqual({ @@ -139,13 +136,12 @@ describe('lastRunFromState', () => { }, getRuleResultService({ warnings: ['MOCK_WARNING'], - outcomeMessage: ruleExecutionOutcomeMessage, + outcomeMessage: 'Rule execution reported a warning', }) ); expect(result.lastRun.outcome).toEqual('warning'); expect(result.lastRun.outcomeMsg).toEqual([ - 'MOCK_WARNING', frameworkOutcomeMessage, ruleExecutionOutcomeMessage, ]); @@ -175,7 +171,6 @@ describe('lastRunFromState', () => { expect(result.lastRun.outcome).toEqual('warning'); expect(result.lastRun.outcomeMsg).toEqual([ - 'MOCK_WARNING', frameworkOutcomeMessage, ruleExecutionOutcomeMessage, ]); @@ -203,7 +198,6 @@ describe('lastRunFromState', () => { expect(result.lastRun.outcome).toEqual('failed'); expect(result.lastRun.outcomeMsg).toEqual([ 'Rule reported more than the maximum number of alerts in a single run. Alerts may be missed and recovery notifications may be delayed', - 'MOCK_ERROR', 'Rule execution reported an error', ]); expect(result.lastRun.warning).toEqual('maxAlerts'); From 79b66a6703913cfcd8f64765bddc52b9b55f0953 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Sun, 22 Jan 2023 13:13:10 +0100 Subject: [PATCH 27/34] fix functional tests --- .../security_and_spaces/group1/check_privileges.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts index c9af9f5e65065..b4ae68bb8d61c 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/check_privileges.ts @@ -79,7 +79,7 @@ export default ({ getService }: FtrProviderContext) => { // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe expect(body?.execution_summary?.last_execution.message).to.eql( - `WARNING: This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` + `This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` ); await deleteUserAndRole(getService, ROLES.detections_admin); @@ -118,7 +118,7 @@ export default ({ getService }: FtrProviderContext) => { // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe expect(body?.execution_summary?.last_execution.message).to.eql( - `WARNING: This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]. \nERROR: An error occurred during rule execution: message: "Aggregations were missing on threshold rule search result"` + `This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]` ); await deleteUserAndRole(getService, ROLES.detections_admin); From ed7965df483e00a17b14702b328843882b75275c Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 11:52:24 +0100 Subject: [PATCH 28/34] remove TBC acronym --- .../src/core/unused_types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts index f30ee906e714d..3f46404066383 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts @@ -33,7 +33,7 @@ export const REMOVED_TYPES: string[] = [ 'osquery-usage-metric', // Was removed in 8.1 https://github.com/elastic/kibana/issues/91265 'siem-detection-engine-rule-status', - // Was removed in 8.7 (TBC) https://github.com/elastic/kibana/issues/130966 + // Was removed in 8.7 https://github.com/elastic/kibana/issues/130966 'siem-detection-engine-rule-execution-info', // Was removed in 7.16 'timelion-sheet', From d319580e2f38a85813dcee71754094854e941118 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 11:53:14 +0100 Subject: [PATCH 29/34] remove unused function --- .../rule_monitoring/model/log_level.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts index d166de577ddca..87f6b81b61dba 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts @@ -81,17 +81,3 @@ export const logLevelFromExecutionStatus = (status: RuleExecutionStatus): LogLev return LogLevel.trace; } }; - -export const logLevelFromLastRunOutcome = (status: RuleLastRunOutcomes): LogLevel => { - switch (status) { - case 'succeeded': - return LogLevel.info; - case 'warning': - return LogLevel.warn; - case 'failed': - return LogLevel.error; - default: - assertUnreachable(status); - return LogLevel.trace; - } -}; From 2b441c9e33f066d01f1c5132784f8d8a2c9eea6f Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 12:13:42 +0100 Subject: [PATCH 30/34] add invariant checks for ruleMonitoringService and ruleResultService --- .../rule_monitoring/logic/rule_execution_log/service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts index c45f4573a139c..cea7f5d23a14f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/service.ts @@ -6,6 +6,7 @@ */ import type { Logger } from '@kbn/core/server'; +import { invariant } from '../../../../../../common/utils/invariant'; import type { ConfigType } from '../../../../../config'; import { withSecuritySpan } from '../../../../../utils/with_security_span'; import type { @@ -53,6 +54,9 @@ export const createRuleExecutionLogService = ( return withSecuritySpan('IRuleExecutionLogService.createClientForExecutors', async () => { const { savedObjectsClient, context, ruleMonitoringService, ruleResultService } = params; + invariant(ruleMonitoringService, 'ruleMonitoringService required for detection rules'); + invariant(ruleResultService, 'ruleResultService required for detection rules'); + const childLogger = logger.get('ruleExecution'); const ruleExecutionSettings = await fetchRuleExecutionSettings( From 960b788a0786ce935ab07b5c6d0683d9ad41a08d Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 12:14:45 +0100 Subject: [PATCH 31/34] improve function's naming --- .../logic/rule_execution_log/client_for_executors/client.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index dd096987cf60a..3f4562913ca34 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -87,7 +87,7 @@ export const createClientForExecutors = ( await Promise.all([ writeStatusChangeToConsole(normalizedArgs, logMeta), - writeStatusChangeToMonitoringService(normalizedArgs), + writeStatusChangeToRuleObject(normalizedArgs), writeStatusChangeToEventLog(normalizedArgs), ]); } catch (e) { @@ -160,9 +160,7 @@ export const createClientForExecutors = ( writeMessageToConsole(logMessage, logLevel, logMeta); }; - const writeStatusChangeToMonitoringService = async ( - args: NormalizedStatusChangeArgs - ): Promise => { + const writeStatusChangeToRuleObject = async (args: NormalizedStatusChangeArgs): Promise => { const { newStatus, message, metrics } = args; if (newStatus === RuleExecutionStatus.running) { From 01b52fe5b0c948e468e26cb7dcb992f7ea0f3967 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 12:30:21 +0100 Subject: [PATCH 32/34] rollback StatusChangeArgsWithMessage to StatusChangeArgs --- .../client_for_executors/client_interface.ts | 11 +++++------ .../rule_preview/api/preview_rules/route.ts | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts index 54d9055f1874d..cb65b42d50b69 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client_interface.ts @@ -107,15 +107,14 @@ export interface RunningStatusChangeArgs { newStatus: RuleExecutionStatus.running; } -export interface StatusChangeArgsWithMessage { - newStatus: Exclude; - metrics?: MetricsArgs; - message: string; -} /** * Information about the status change event. */ -export type StatusChangeArgs = RunningStatusChangeArgs | StatusChangeArgsWithMessage; +export interface StatusChangeArgs { + newStatus: RuleExecutionStatus; + message?: string; + metrics?: MetricsArgs; +} export interface MetricsArgs { searchDurations?: string[]; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts index ce91f9cd1ee79..457412dd01501 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts @@ -39,7 +39,7 @@ import { throwAuthzError } from '../../../../machine_learning/validation'; import { buildRouteValidation } from '../../../../../utils/build_validation/route_validation'; import type { SecuritySolutionPluginRouter } from '../../../../../types'; -import type { RuleExecutionContext, StatusChangeArgsWithMessage } from '../../../rule_monitoring'; +import type { RuleExecutionContext, StatusChangeArgs } from '../../../rule_monitoring'; import type { ConfigType } from '../../../../../config'; import { alertInstanceFactoryStub } from '../../../signals/preview/alert_instance_factory_stub'; @@ -123,7 +123,7 @@ export const previewRulesRoute = async ( const spaceId = siemClient.getSpaceId(); const previewId = uuidv4(); const username = security?.authc.getCurrentUser(request)?.username; - const loggedStatusChanges: Array = []; + const loggedStatusChanges: Array = []; const previewRuleExecutionLogger = createPreviewRuleExecutionLogger(loggedStatusChanges); const runState: Record = {}; const logs: RulePreviewLogs[] = []; From 8f0a58c427e6f5392a2e95e65e421d0aa5d92adc Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Fri, 27 Jan 2023 13:29:00 +0100 Subject: [PATCH 33/34] make ruleMonitoringService and ruleResultService required for detection rules --- .../client_for_executors/client.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts index 3f4562913ca34..a0cbe754d6b3c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts @@ -43,8 +43,8 @@ export const createClientForExecutors = ( eventLog: IEventLogWriter, logger: Logger, context: RuleExecutionContext, - ruleMonitoringService?: PublicRuleMonitoringService, - ruleResultService?: PublicRuleResultService + ruleMonitoringService: PublicRuleMonitoringService, + ruleResultService: PublicRuleResultService ): IRuleExecutionLogForExecutors => { const baseCorrelationIds = getCorrelationIds(context); const baseLogSuffix = baseCorrelationIds.getLogSuffix(); @@ -174,24 +174,24 @@ export const createClientForExecutors = ( } = metrics ?? {}; if (totalSearchDurationMs) { - ruleMonitoringService?.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); + ruleMonitoringService.setLastRunMetricsTotalSearchDurationMs(totalSearchDurationMs); } if (totalIndexingDurationMs) { - ruleMonitoringService?.setLastRunMetricsTotalIndexingDurationMs(totalIndexingDurationMs); + ruleMonitoringService.setLastRunMetricsTotalIndexingDurationMs(totalIndexingDurationMs); } if (executionGapDurationS) { - ruleMonitoringService?.setLastRunMetricsGapDurationS(executionGapDurationS); + ruleMonitoringService.setLastRunMetricsGapDurationS(executionGapDurationS); } if (newStatus === RuleExecutionStatus.failed) { - ruleResultService?.addLastRunError(message); + ruleResultService.addLastRunError(message); } else if (newStatus === RuleExecutionStatus['partial failure']) { - ruleResultService?.addLastRunWarning(message); + ruleResultService.addLastRunWarning(message); } - ruleResultService?.setLastRunOutcomeMessage(message); + ruleResultService.setLastRunOutcomeMessage(message); }; const writeStatusChangeToEventLog = (args: NormalizedStatusChangeArgs): void => { From b54f6454ffc0ad090597eb69c853264fec4f9559 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 27 Jan 2023 13:03:49 +0000 Subject: [PATCH 34/34] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../common/detection_engine/rule_monitoring/model/log_level.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts index 87f6b81b61dba..b37ce62ad4891 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring/model/log_level.ts @@ -6,7 +6,6 @@ */ import { enumeration } from '@kbn/securitysolution-io-ts-types'; -import type { RuleLastRunOutcomes } from '@kbn/alerting-plugin/common'; import { enumFromString } from '../../../utils/enum_from_string'; import { assertUnreachable } from '../../../utility_types'; import { RuleExecutionStatus } from './execution_status';