diff --git a/x-pack/plugins/alerting/server/mocks.ts b/x-pack/plugins/alerting/server/mocks.ts index 639ba166e00a8..43c4d149e0867 100644 --- a/x-pack/plugins/alerting/server/mocks.ts +++ b/x-pack/plugins/alerting/server/mocks.ts @@ -73,6 +73,7 @@ const createAlertServicesMock = < .mockReturnValue(alertInstanceFactoryMock), savedObjectsClient: savedObjectsClientMock.create(), scopedClusterClient: elasticsearchServiceMock.createScopedClusterClient(), + shouldLogAndScheduleActionsForAlerts: () => true, }; }; export type AlertServicesMock = ReturnType; diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.ts index 058c85765c8b4..8d7300adadf04 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -317,6 +317,7 @@ export class TaskRunner< InstanceContext, WithoutReservedActionGroups >(alertInstances), + shouldLogAndScheduleActionsForAlerts: () => this.shouldLogAndScheduleActionsForAlerts(), }, params, state: alertTypeState as State, diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index 66eafd5c21a40..b4be92803000d 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -75,6 +75,7 @@ export interface AlertServices< alertInstanceFactory: ( id: string ) => PublicAlertInstance; + shouldLogAndScheduleActionsForAlerts: () => boolean; } export interface AlertExecutorOptions< diff --git a/x-pack/plugins/apm/server/lib/alerts/test_utils/index.ts b/x-pack/plugins/apm/server/lib/alerts/test_utils/index.ts index 5d5865bdd2289..142800a775d93 100644 --- a/x-pack/plugins/apm/server/lib/alerts/test_utils/index.ts +++ b/x-pack/plugins/apm/server/lib/alerts/test_utils/index.ts @@ -45,6 +45,7 @@ export const createRuleTypeMocks = () => { alertInstanceFactory: jest.fn(() => ({ scheduleActions })), alertWithLifecycle: jest.fn(), logger: loggerMock, + shouldLogAndScheduleActionsForAlerts: () => true, }; return { diff --git a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts index c65fdece6c5f0..307c0ddd28a8a 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.test.ts @@ -349,6 +349,33 @@ describe('createLifecycleExecutor', () => { }) ); }); + + it('does not write alert documents when rule execution is cancelled and feature flags indicate to skip', async () => { + const logger = loggerMock.create(); + const ruleDataClientMock = createRuleDataClientMock(); + const executor = createLifecycleExecutor( + logger, + ruleDataClientMock + )<{}, TestRuleState, never, never, never>(async (options) => { + expect(options.state).toEqual(initialRuleState); + + const nextRuleState: TestRuleState = { + aRuleStateKey: 'NEXT_RULE_STATE_VALUE', + }; + + return nextRuleState; + }); + + await executor( + createDefaultAlertExecutorOptions({ + params: {}, + state: { wrapped: initialRuleState, trackedAlerts: {} }, + shouldLogAndScheduleActionsForAlerts: false, + }) + ); + + expect(ruleDataClientMock.getWriter).not.toHaveBeenCalled(); + }); }); type TestRuleState = Record & { diff --git a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts index 1acbc0c3f43bd..283c786134486 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts @@ -140,7 +140,7 @@ export const createLifecycleExecutor = > ): Promise> => { const { - services: { alertInstanceFactory }, + services: { alertInstanceFactory, shouldLogAndScheduleActionsForAlerts }, state: previousState, } = options; @@ -281,7 +281,16 @@ export const createLifecycleExecutor = const newEventsToIndex = makeEventsDataMapFor(newAlertIds); const allEventsToIndex = [...trackedEventsToIndex, ...newEventsToIndex]; - if (allEventsToIndex.length > 0 && ruleDataClient.isWriteEnabled()) { + // Only write alerts if: + // - writing is enabled + // AND + // - rule execution has not been cancelled due to timeout + // OR + // - if execution has been cancelled due to timeout, if feature flags are configured to write alerts anyway + const shouldWriteAlerts = + ruleDataClient.isWriteEnabled() && shouldLogAndScheduleActionsForAlerts(); + + if (allEventsToIndex.length > 0 && shouldWriteAlerts) { logger.debug(`Preparing to index ${allEventsToIndex.length} alerts.`); await ruleDataClient.getWriter().bulk({ @@ -307,6 +316,6 @@ export const createLifecycleExecutor = return { wrapped: nextWrappedState ?? ({} as State), - trackedAlerts: ruleDataClient.isWriteEnabled() ? nextTrackedAlerts : {}, + trackedAlerts: shouldWriteAlerts ? nextTrackedAlerts : {}, }; }; diff --git a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts index 3fa567b8aca96..7e15c86bfa2f9 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts @@ -21,7 +21,7 @@ import { createLifecycleRuleTypeFactory } from './create_lifecycle_rule_type_fac type RuleTestHelpers = ReturnType; -function createRule() { +function createRule(shouldLogAndScheduleActionsForAlerts: boolean = true) { const ruleDataClientMock = createRuleDataClientMock(); const factory = createLifecycleRuleTypeFactory({ @@ -110,6 +110,7 @@ function createRule() { alertInstanceFactory, savedObjectsClient: {} as any, scopedClusterClient: {} as any, + shouldLogAndScheduleActionsForAlerts: () => shouldLogAndScheduleActionsForAlerts, }, spaceId: 'spaceId', state, @@ -152,6 +153,26 @@ describe('createLifecycleRuleTypeFactory', () => { }); }); + describe('when rule is cancelled due to timeout and config flags indicate to skip actions', () => { + beforeEach(() => { + helpers = createRule(false); + helpers.ruleDataClientMock.isWriteEnabled.mockReturnValue(true); + }); + + it("doesn't persist anything", async () => { + await helpers.alertWithLifecycle([ + { + id: 'opbeans-java', + fields: { + 'service.name': 'opbeans-java', + }, + }, + ]); + + expect(helpers.ruleDataClientMock.getWriter().bulk).toHaveBeenCalledTimes(0); + }); + }); + describe('when alerts are new', () => { beforeEach(async () => { await helpers.alertWithLifecycle([ diff --git a/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_factory.ts b/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_factory.ts index 837d0378703f7..a310499f30c59 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_factory.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_factory.ts @@ -23,7 +23,11 @@ export const createPersistenceRuleTypeFactory: CreatePersistenceRuleTypeFactory const numAlerts = alerts.length; logger.debug(`Found ${numAlerts} alerts.`); - if (ruleDataClient.isWriteEnabled() && numAlerts) { + if ( + ruleDataClient.isWriteEnabled() && + numAlerts && + options.services.shouldLogAndScheduleActionsForAlerts() // This check ensures that rule execution has not been cancelled due to timeout or, if it has whether we still want to proceed with handling alerts after rule timeout + ) { const commonRuleFields = getCommonAlertFields(options); const response = await ruleDataClient.getWriter().bulk({ diff --git a/x-pack/plugins/rule_registry/server/utils/rule_executor_test_utils.ts b/x-pack/plugins/rule_registry/server/utils/rule_executor_test_utils.ts index 95a6761152371..7cb5ec309d4f7 100644 --- a/x-pack/plugins/rule_registry/server/utils/rule_executor_test_utils.ts +++ b/x-pack/plugins/rule_registry/server/utils/rule_executor_test_utils.ts @@ -31,6 +31,7 @@ export const createDefaultAlertExecutorOptions = < createdAt = new Date(), startedAt = new Date(), updatedAt = new Date(), + shouldLogAndScheduleActionsForAlerts = true, }: { alertId?: string; ruleName?: string; @@ -39,6 +40,7 @@ export const createDefaultAlertExecutorOptions = < createdAt?: Date; startedAt?: Date; updatedAt?: Date; + shouldLogAndScheduleActionsForAlerts?: boolean; }): AlertExecutorOptions => ({ alertId, createdBy: 'CREATED_BY', @@ -69,6 +71,7 @@ export const createDefaultAlertExecutorOptions = < .alertInstanceFactory, savedObjectsClient: savedObjectsClientMock.create(), scopedClusterClient: elasticsearchServiceMock.createScopedClusterClient(), + shouldLogAndScheduleActionsForAlerts: () => shouldLogAndScheduleActionsForAlerts, }, state, updatedBy: null, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/rule_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/rule_type.ts index c6f818f04fc5d..f6ad7e2ed7db1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/rule_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/rule_type.ts @@ -78,6 +78,7 @@ export const createRuleTypeMocks = ( findAlerts: jest.fn(), // TODO: does this stay? alertWithPersistence: jest.fn(), logger: loggerMock, + shouldLogAndScheduleActionsForAlerts: () => true, }; return { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/bulk_create_factory.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/bulk_create_factory.ts index af0a8a27f2b25..1d0f3bbe88aac 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/bulk_create_factory.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/bulk_create_factory.ts @@ -61,53 +61,64 @@ export const bulkCreateFactory = `individual bulk process time took: ${makeFloatString(end - start)} milliseconds` ) ); - logger.debug( - buildRuleMessage(`took property says bulk took: ${response.body.took} milliseconds`) - ); - const createdItems = wrappedDocs - .map((doc, index) => { - const responseIndex = response.body.items[index].index; - return { - _id: responseIndex?._id ?? '', - _index: responseIndex?._index ?? '', - [ALERT_INSTANCE_ID]: responseIndex?._id ?? '', - ...doc._source, - }; - }) - .filter((_, index) => response.body.items[index].index?.status === 201); - const createdItemsCount = createdItems.length; + if (response) { + logger.debug( + buildRuleMessage(`took property says bulk took: ${response.body.took} milliseconds`) + ); - const duplicateSignalsCount = countBy(response.body.items, 'create.status')['409']; - const errorCountByMessage = errorAggregator(response.body, [409]); + const createdItems = wrappedDocs + .map((doc, index) => { + const responseIndex = response.body.items[index].index; + return { + _id: responseIndex?._id ?? '', + _index: responseIndex?._index ?? '', + [ALERT_INSTANCE_ID]: responseIndex?._id ?? '', + ...doc._source, + }; + }) + .filter((_, index) => response.body.items[index].index?.status === 201); - logger.debug(buildRuleMessage(`bulk created ${createdItemsCount} signals`)); + const createdItemsCount = createdItems.length; + const duplicateSignalsCount = countBy(response.body.items, 'create.status')['409']; + const errorCountByMessage = errorAggregator(response.body, [409]); - if (duplicateSignalsCount > 0) { - logger.debug(buildRuleMessage(`ignored ${duplicateSignalsCount} duplicate signals`)); - } + logger.debug(buildRuleMessage(`bulk created ${createdItemsCount} signals`)); - if (!isEmpty(errorCountByMessage)) { - logger.error( - buildRuleMessage( - `[-] bulkResponse had errors with responses of: ${JSON.stringify(errorCountByMessage)}` - ) - ); + if (duplicateSignalsCount > 0) { + logger.debug(buildRuleMessage(`ignored ${duplicateSignalsCount} duplicate signals`)); + } - return { - errors: Object.keys(errorCountByMessage), - success: false, - bulkCreateDuration: makeFloatString(end - start), - createdItemsCount: createdItems.length, - createdItems, - }; + if (!isEmpty(errorCountByMessage)) { + logger.error( + buildRuleMessage( + `[-] bulkResponse had errors with responses of: ${JSON.stringify(errorCountByMessage)}` + ) + ); + + return { + errors: Object.keys(errorCountByMessage), + success: false, + bulkCreateDuration: makeFloatString(end - start), + createdItemsCount: createdItems.length, + createdItems, + }; + } else { + return { + errors: [], + success: true, + bulkCreateDuration: makeFloatString(end - start), + createdItemsCount: createdItems.length, + createdItems, + }; + } } else { return { errors: [], success: true, bulkCreateDuration: makeFloatString(end - start), - createdItemsCount: createdItems.length, - createdItems, + createdItemsCount: 0, + createdItems: [], }; } };