From 32ac1a5355593be82eb0d31cf7e3cb9d20e56965 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Fri, 25 Mar 2022 11:30:59 -0400 Subject: [PATCH] [responseOps] add snoozing MVP in alerting framework (#127694) resolves https://github.com/elastic/kibana/issues/126512 Changes the calculation of rule-level muting to take into account snoozeEndTime. If muteAll is true, the rule is considered snoozing forever, regardless of the setting of snoozeEndTime. If muteAll is false, snoozeEndTime determines whether the rule is snoozed. If snoozeEndTime is null, the rule is not snoozed. Otherwise, snoozeEndTime is a Date, and if it's >= than Date.now(), the rule is snoozed. Otherwise, the rule is not snoozed. --- .../server/rules_client/rules_client.ts | 4 + .../alerting/server/task_runner/fixtures.ts | 1 + .../server/task_runner/task_runner.test.ts | 66 ++++++++- .../server/task_runner/task_runner.ts | 20 ++- .../spaces_only/tests/alerting/index.ts | 1 + .../spaces_only/tests/alerting/snooze.ts | 132 ++++++++++++++++-- 6 files changed, 207 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 9642db04e504a..4dc1ab6ce1122 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -2116,12 +2116,15 @@ export class RulesClient { executionStatus, schedule, actions, + snoozeEndTime, ...partialRawRule }: Partial, references: SavedObjectReference[] | undefined, includeLegacyId: boolean = false, excludeFromPublicApi: boolean = false ): PartialRule | PartialRuleWithLegacyId { + const snoozeEndTimeDate = snoozeEndTime != null ? new Date(snoozeEndTime) : snoozeEndTime; + const includeSnoozeEndTime = snoozeEndTimeDate !== undefined && !excludeFromPublicApi; const rule = { id, notifyWhen, @@ -2131,6 +2134,7 @@ export class RulesClient { schedule: schedule as IntervalSchedule, actions: actions ? this.injectReferencesIntoActions(id, actions, references || []) : [], params: this.injectReferencesIntoParams(id, ruleType, params, references || []) as Params, + ...(includeSnoozeEndTime ? { snoozeEndTime: snoozeEndTimeDate } : {}), ...(updatedAt ? { updatedAt: new Date(updatedAt) } : {}), ...(createdAt ? { createdAt: new Date(createdAt) } : {}), ...(scheduledTaskId ? { scheduledTaskId } : {}), diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts index 3ba21c0de092c..4e38be4834c86 100644 --- a/x-pack/plugins/alerting/server/task_runner/fixtures.ts +++ b/x-pack/plugins/alerting/server/task_runner/fixtures.ts @@ -22,6 +22,7 @@ export const RULE_TYPE_ID = 'test'; export const DATE_1969 = '1969-12-31T00:00:00.000Z'; export const DATE_1970 = '1970-01-01T00:00:00.000Z'; export const DATE_1970_5_MIN = '1969-12-31T23:55:00.000Z'; +export const DATE_9999 = '9999-12-31T12:34:56.789Z'; export const MOCK_DURATION = 86400000000000; export const SAVED_OBJECT = { diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts index e3e4f3045dd8f..2227a34dfa111 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts @@ -65,6 +65,7 @@ import { DATE_1969, DATE_1970, DATE_1970_5_MIN, + DATE_9999, } from './fixtures'; import { EVENT_LOG_ACTIONS } from '../plugin'; import { IN_MEMORY_METRICS } from '../monitoring'; @@ -414,7 +415,7 @@ describe('Task Runner', () => { ); expect(logger.debug).nthCalledWith( 3, - `no scheduling of actions for rule test:1: '${RULE_NAME}': rule is muted.` + `no scheduling of actions for rule test:1: '${RULE_NAME}': rule is snoozed.` ); expect(logger.debug).nthCalledWith( 4, @@ -468,6 +469,69 @@ describe('Task Runner', () => { expect(mockUsageCounter.incrementCounter).not.toHaveBeenCalled(); }); + type SnoozeTestParams = [ + muteAll: boolean, + snoozeEndTime: string | undefined | null, + shouldBeSnoozed: boolean + ]; + + const snoozeTestParams: SnoozeTestParams[] = [ + [false, null, false], + [false, undefined, false], + [false, DATE_1970, false], + [false, DATE_9999, true], + [true, null, true], + [true, undefined, true], + [true, DATE_1970, true], + [true, DATE_9999, true], + ]; + + test.each(snoozeTestParams)( + 'snoozing works as expected with muteAll: %s; snoozeEndTime: %s', + async (muteAll, snoozeEndTime, shouldBeSnoozed) => { + taskRunnerFactoryInitializerParams.actionsPlugin.isActionTypeEnabled.mockReturnValue(true); + taskRunnerFactoryInitializerParams.actionsPlugin.isActionExecutable.mockReturnValue(true); + ruleType.executor.mockImplementation( + async ({ + services: executorServices, + }: AlertExecutorOptions< + AlertTypeParams, + AlertTypeState, + AlertInstanceState, + AlertInstanceContext, + string + >) => { + executorServices.alertFactory.create('1').scheduleActions('default'); + } + ); + const taskRunner = new TaskRunner( + ruleType, + mockedTaskInstance, + taskRunnerFactoryInitializerParams, + inMemoryMetrics + ); + rulesClient.get.mockResolvedValue({ + ...mockedRuleTypeSavedObject, + muteAll, + snoozeEndTime: snoozeEndTime != null ? new Date(snoozeEndTime) : snoozeEndTime, + }); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); + await taskRunner.run(); + + const expectedExecutions = shouldBeSnoozed ? 0 : 1; + expect(actionsClient.enqueueExecution).toHaveBeenCalledTimes(expectedExecutions); + expect(actionsClient.ephemeralEnqueuedExecution).toHaveBeenCalledTimes(0); + + const logger = taskRunnerFactoryInitializerParams.logger; + const expectedMessage = `no scheduling of actions for rule test:1: '${RULE_NAME}': rule is snoozed.`; + if (expectedExecutions) { + expect(logger.debug).not.toHaveBeenCalledWith(expectedMessage); + } else { + expect(logger.debug).toHaveBeenCalledWith(expectedMessage); + } + } + ); + test.each(ephemeralTestParams)( 'skips firing actions for active alert if alert is muted %s', async (nameExtension, customTaskRunnerFactoryInitializerParams, enqueueFunction) => { 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 b3dacd053b632..b7980f612e7b9 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -252,6 +252,18 @@ export class TaskRunner< } } + private isRuleSnoozed(rule: SanitizedAlert): boolean { + if (rule.muteAll) { + return true; + } + + if (rule.snoozeEndTime == null) { + return false; + } + + return Date.now() < rule.snoozeEndTime.getTime(); + } + private shouldLogAndScheduleActionsForAlerts() { // if execution hasn't been cancelled, return true if (!this.cancelled) { @@ -312,7 +324,6 @@ export class TaskRunner< schedule, throttle, notifyWhen, - muteAll, mutedInstanceIds, name, tags, @@ -484,7 +495,8 @@ export class TaskRunner< triggeredActionsStatus: ActionsCompletion.COMPLETE, }; - if (!muteAll && this.shouldLogAndScheduleActionsForAlerts()) { + const ruleIsSnoozed = this.isRuleSnoozed(rule); + if (!ruleIsSnoozed && this.shouldLogAndScheduleActionsForAlerts()) { const mutedAlertIdsSet = new Set(mutedInstanceIds); const alertsWithExecutableActions = Object.entries(alertsWithScheduledActions).filter( @@ -535,8 +547,8 @@ export class TaskRunner< alertExecutionStore, }); } else { - if (muteAll) { - this.logger.debug(`no scheduling of actions for rule ${ruleLabel}: rule is muted.`); + if (ruleIsSnoozed) { + this.logger.debug(`no scheduling of actions for rule ${ruleLabel}: rule is snoozed.`); } if (!this.shouldLogAndScheduleActionsForAlerts()) { this.logger.debug( diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts index 823de1aa798c1..3007e37395156 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts @@ -44,6 +44,7 @@ export default function alertingTests({ loadTestFile, getService }: FtrProviderC loadTestFile(require.resolve('./notify_when')); loadTestFile(require.resolve('./ephemeral')); loadTestFile(require.resolve('./event_log_alerts')); + loadTestFile(require.resolve('./snooze')); loadTestFile(require.resolve('./scheduled_task_id')); // Do not place test files here, due to https://github.com/elastic/kibana/issues/123059 diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts index bb3e0cea469e4..5be5b59a15248 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts @@ -14,6 +14,7 @@ import { getUrlPrefix, getTestRuleData, ObjectRemover, + getEventLog, } from '../../../common/lib'; const FUTURE_SNOOZE_TIME = '9999-12-31T06:00:00.000Z'; @@ -22,6 +23,8 @@ const FUTURE_SNOOZE_TIME = '9999-12-31T06:00:00.000Z'; export default function createSnoozeRuleTests({ getService }: FtrProviderContext) { const supertest = getService('supertest'); const supertestWithoutAuth = getService('supertestWithoutAuth'); + const log = getService('log'); + const retry = getService('retry'); describe('snooze', () => { const objectRemover = new ObjectRemover(supertest); @@ -32,7 +35,7 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext it('should handle snooze rule request appropriately', async () => { const { body: createdAction } = await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}}/api/actions/connector`) + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) .set('kbn-xsrf', 'foo') .send({ name: 'MY action', @@ -41,8 +44,9 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext secrets: {}, }) .expect(200); + objectRemover.add(Spaces.space1.id, createdAction.id, 'action', 'actions'); - const { body: createdAlert } = await supertest + const { body: createdRule } = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) .set('kbn-xsrf', 'foo') .send( @@ -58,16 +62,16 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext }) ) .expect(200); - objectRemover.add(Spaces.space1.id, createdAlert.id, 'rule', 'alerting'); + objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting'); const response = await alertUtils - .getSnoozeRequest(createdAlert.id) + .getSnoozeRequest(createdRule.id) .send({ snooze_end_time: FUTURE_SNOOZE_TIME }); expect(response.statusCode).to.eql(204); expect(response.body).to.eql(''); const { body: updatedAlert } = await supertestWithoutAuth - .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdAlert.id}`) + .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`) .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.snooze_end_time).to.eql(FUTURE_SNOOZE_TIME); @@ -77,13 +81,13 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext supertest, spaceId: Spaces.space1.id, type: 'alert', - id: createdAlert.id, + id: createdRule.id, }); }); it('should handle snooze rule request appropriately when snoozeEndTime is -1', async () => { const { body: createdAction } = await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}}/api/actions/connector`) + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) .set('kbn-xsrf', 'foo') .send({ name: 'MY action', @@ -92,8 +96,9 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext secrets: {}, }) .expect(200); + objectRemover.add(Spaces.space1.id, createdAction.id, 'action', 'actions'); - const { body: createdAlert } = await supertest + const { body: createdRule } = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) .set('kbn-xsrf', 'foo') .send( @@ -109,16 +114,16 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext }) ) .expect(200); - objectRemover.add(Spaces.space1.id, createdAlert.id, 'rule', 'alerting'); + objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting'); const response = await alertUtils - .getSnoozeRequest(createdAlert.id) + .getSnoozeRequest(createdRule.id) .send({ snooze_end_time: -1 }); expect(response.statusCode).to.eql(204); expect(response.body).to.eql(''); const { body: updatedAlert } = await supertestWithoutAuth - .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdAlert.id}`) + .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`) .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.snooze_end_time).to.eql(null); @@ -128,8 +133,111 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext supertest, spaceId: Spaces.space1.id, type: 'alert', - id: createdAlert.id, + id: createdRule.id, }); }); + + it('should not trigger actions when snoozed', async () => { + const { body: createdAction, status: connStatus } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + connector_type_id: 'test.noop', + config: {}, + secrets: {}, + }); + expect(connStatus).to.be(200); + objectRemover.add(Spaces.space1.id, createdAction.id, 'action', 'actions'); + + log.info('creating rule'); + const { body: createdRule, status: ruleStatus } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + name: 'should not trigger actions when snoozed', + rule_type_id: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + notify_when: 'onActiveAlert', + params: { + pattern: { instance: arrayOfTrues(100) }, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + ], + }) + ); + expect(ruleStatus).to.be(200); + objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting'); + + // wait for an action to be triggered + log.info('wait for rule to trigger an action'); + await getRuleEvents(createdRule.id); + + log.info('start snoozing'); + const snoozeSeconds = 10; + const snoozeEndDate = new Date(Date.now() + 1000 * snoozeSeconds); + await alertUtils + .getSnoozeRequest(createdRule.id) + .send({ snooze_end_time: snoozeEndDate.toISOString() }); + + // could be an action execution while calling snooze, so set snooze start + // to a value that we know it will be in effect (after this call) + const snoozeStartDate = new Date(); + + // wait for 4 triggered actions - in case some fired before snooze went into effect + log.info('wait for snoozing to end'); + const ruleEvents = await getRuleEvents(createdRule.id, 4); + const snoozeStart = snoozeStartDate.valueOf(); + const snoozeEnd = snoozeStartDate.valueOf(); + let actionsBefore = 0; + let actionsDuring = 0; + let actionsAfter = 0; + + for (const event of ruleEvents) { + const timestamp = event?.['@timestamp']; + if (!timestamp) continue; + + const time = new Date(timestamp).valueOf(); + if (time < snoozeStart) { + actionsBefore++; + } else if (time > snoozeEnd) { + actionsAfter++; + } else { + actionsDuring++; + } + } + + expect(actionsBefore).to.be.greaterThan(0, 'no actions triggered before snooze'); + expect(actionsAfter).to.be.greaterThan(0, 'no actions triggered after snooze'); + expect(actionsDuring).to.be(0); + }); }); + + async function getRuleEvents(id: string, minActions: number = 1) { + return await retry.try(async () => { + return await getEventLog({ + getService, + spaceId: Spaces.space1.id, + type: 'alert', + id, + provider: 'alerting', + actions: new Map([['execute-action', { gte: minActions }]]), + }); + }); + } +} + +function arrayOfTrues(length: number) { + const result = []; + for (let i = 0; i < length; i++) { + result.push(true); + } + return result; }