diff --git a/x-pack/plugins/alerting/server/mocks.ts b/x-pack/plugins/alerting/server/mocks.ts index 7fb748a305037..f23dbf05449ad 100644 --- a/x-pack/plugins/alerting/server/mocks.ts +++ b/x-pack/plugins/alerting/server/mocks.ts @@ -74,6 +74,7 @@ const createAlertServicesMock = < .mockReturnValue(alertInstanceFactoryMock), savedObjectsClient: savedObjectsClientMock.create(), scopedClusterClient: elasticsearchServiceMock.createScopedClusterClient(), + shouldWriteAlerts: () => 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 fb7268ef529da..0cf5202787392 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -323,6 +323,7 @@ export class TaskRunner< InstanceContext, WithoutReservedActionGroups >(alertInstances), + shouldWriteAlerts: () => 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 c1645936c06e9..343b717dcb1aa 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; + shouldWriteAlerts: () => boolean; } export interface AlertExecutorOptions< diff --git a/x-pack/plugins/apm/server/routes/alerts/test_utils/index.ts b/x-pack/plugins/apm/server/routes/alerts/test_utils/index.ts index 22649a7010461..a8610bbcc8d37 100644 --- a/x-pack/plugins/apm/server/routes/alerts/test_utils/index.ts +++ b/x-pack/plugins/apm/server/routes/alerts/test_utils/index.ts @@ -45,6 +45,7 @@ export const createRuleTypeMocks = () => { alertInstanceFactory: jest.fn(() => ({ scheduleActions })), alertWithLifecycle: jest.fn(), logger: loggerMock, + shouldWriteAlerts: () => 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 fcddcab378bc6..2c5fe09d80563 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 @@ -350,6 +350,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: {} }, + shouldWriteAlerts: 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..c30b1654a3587 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, shouldWriteAlerts }, state: previousState, } = options; @@ -281,7 +281,15 @@ 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 writeAlerts = ruleDataClient.isWriteEnabled() && shouldWriteAlerts(); + + if (allEventsToIndex.length > 0 && writeAlerts) { logger.debug(`Preparing to index ${allEventsToIndex.length} alerts.`); await ruleDataClient.getWriter().bulk({ @@ -307,6 +315,6 @@ export const createLifecycleExecutor = return { wrapped: nextWrappedState ?? ({} as State), - trackedAlerts: ruleDataClient.isWriteEnabled() ? nextTrackedAlerts : {}, + trackedAlerts: writeAlerts ? 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 2284ad5e796ee..f0e2412629bb1 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(shouldWriteAlerts: boolean = true) { const ruleDataClientMock = createRuleDataClientMock(); const factory = createLifecycleRuleTypeFactory({ @@ -110,6 +110,7 @@ function createRule() { alertInstanceFactory, savedObjectsClient: {} as any, scopedClusterClient: {} as any, + shouldWriteAlerts: () => shouldWriteAlerts, }, 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_wrapper.ts b/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts index afdcf856a872f..de1193771dd95 100644 --- a/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts +++ b/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts @@ -25,7 +25,16 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper const numAlerts = alerts.length; logger.debug(`Found ${numAlerts} alerts.`); - if (ruleDataClient.isWriteEnabled() && numAlerts) { + // 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 writeAlerts = + ruleDataClient.isWriteEnabled() && options.services.shouldWriteAlerts(); + + if (writeAlerts && numAlerts) { const commonRuleFields = getCommonAlertFields(options); const CHUNK_SIZE = 10000; 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..288830f4b3804 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(), + shouldWriteAlerts = true, }: { alertId?: string; ruleName?: string; @@ -39,6 +40,7 @@ export const createDefaultAlertExecutorOptions = < createdAt?: Date; startedAt?: Date; updatedAt?: Date; + shouldWriteAlerts?: boolean; }): AlertExecutorOptions => ({ alertId, createdBy: 'CREATED_BY', @@ -69,6 +71,7 @@ export const createDefaultAlertExecutorOptions = < .alertInstanceFactory, savedObjectsClient: savedObjectsClientMock.create(), scopedClusterClient: elasticsearchServiceMock.createScopedClusterClient(), + shouldWriteAlerts: () => shouldWriteAlerts, }, state, updatedBy: null, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts index 882732544dcbb..3949e71582020 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts @@ -122,6 +122,7 @@ export const previewRulesRoute = async ( ruleTypeId: string, ruleTypeName: string, params: TParams, + shouldWriteAlerts: () => boolean, alertInstanceFactory: ( id: string ) => Pick< @@ -157,6 +158,7 @@ export const previewRulesRoute = async ( previousStartedAt, rule, services: { + shouldWriteAlerts, alertInstanceFactory, savedObjectsClient: context.core.savedObjects.client, scopedClusterClient: context.core.elasticsearch.client, @@ -191,6 +193,7 @@ export const previewRulesRoute = async ( signalRuleAlertType.id, signalRuleAlertType.name, previewRuleParams, + () => true, alertInstanceFactoryStub ); 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..aaab81a4c66ff 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, + shouldWriteAlerts: () => true, }; return {