diff --git a/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.test.ts b/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.test.ts index 5da807ddba65dd..3fb1877756d156 100644 --- a/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.test.ts +++ b/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.test.ts @@ -12,7 +12,7 @@ import { createAlertFactory, getPublicAlertFactory } from '../alert/create_alert import { Alert } from '../alert/alert'; import { alertingEventLoggerMock } from '../lib/alerting_event_logger/alerting_event_logger.mock'; import { ruleRunMetricsStoreMock } from '../lib/rule_run_metrics_store.mock'; -import { getAlertsForNotification, processAlerts, setFlapping } from '../lib'; +import { getAlertsForNotification, processAlerts } from '../lib'; import { logAlerts } from '../task_runner/log_alerts'; import { DEFAULT_FLAPPING_SETTINGS } from '../../common/rules_settings'; @@ -254,19 +254,6 @@ describe('Legacy Alerts Client', () => { flappingSettings: DEFAULT_FLAPPING_SETTINGS, }); - expect(setFlapping).toHaveBeenCalledWith( - { - enabled: true, - lookBackWindow: 20, - statusChangeThreshold: 4, - }, - { - '1': new Alert('1', testAlert1), - '2': new Alert('2', testAlert2), - }, - {} - ); - expect(getAlertsForNotification).toHaveBeenCalledWith( { enabled: true, diff --git a/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.ts b/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.ts index 9affe3a67d7ebc..8fce6782cd2f17 100644 --- a/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.ts +++ b/x-pack/plugins/alerting/server/alerts_client/legacy_alerts_client.ts @@ -143,12 +143,6 @@ export class LegacyAlertsClient< flappingSettings, }); - setFlapping( - flappingSettings, - processedAlertsActive, - processedAlertsRecovered - ); - const { trimmedAlertsRecovered, earlyRecoveredAlerts } = trimRecoveredAlerts( this.options.logger, processedAlertsRecovered, @@ -213,4 +207,12 @@ export class LegacyAlertsClient< public getExecutorServices() { return getPublicAlertFactory(this.alertFactory!); } + + public setFlapping(flappingSettings: RulesSettingsFlappingProperties) { + setFlapping( + flappingSettings, + this.processedAlerts.active, + this.processedAlerts.recovered + ); + } } 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 a94512e1be4839..82e01a93125e58 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -466,6 +466,8 @@ export class TaskRunner< } }); + this.legacyAlertsClient.setFlapping(flappingSettings); + let alertsToReturn: Record = {}; let recoveredAlertsToReturn: Record = {}; // Only serialize alerts into task state if we're auto-recovering, otherwise 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 2902533c145f14..8019e5bf3254a4 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 @@ -1015,7 +1015,7 @@ describe('createLifecycleExecutor', () => { return { state }; }); - await executor( + const serializedAlerts = await executor( createDefaultAlertExecutorOptions({ alertId: 'TEST_ALERT_0', params: {}, @@ -1061,6 +1061,43 @@ describe('createLifecycleExecutor', () => { }) ); + expect(serializedAlerts.state.trackedAlerts).toEqual({ + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + flapping: true, + flappingHistory: flapping.slice(1).concat([false]), + pendingRecoveredCount: 0, + started: '2020-01-01T12:00:00.000Z', + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + flapping: false, + flappingHistory: [false, false, false], + pendingRecoveredCount: 0, + started: '2020-01-02T12:00:00.000Z', + }, + TEST_ALERT_2: { + alertId: 'TEST_ALERT_2', + alertUuid: 'TEST_ALERT_2_UUID', + flapping: true, + flappingHistory: flapping.slice(1).concat([false]), + pendingRecoveredCount: 0, + started: '2020-01-01T12:00:00.000Z', + }, + TEST_ALERT_3: { + alertId: 'TEST_ALERT_3', + alertUuid: 'TEST_ALERT_3_UUID', + flapping: true, + flappingHistory: [false, false, false], + pendingRecoveredCount: 0, + started: '2020-01-02T12:00:00.000Z', + }, + }); + + expect(serializedAlerts.state.trackedAlertsRecovered).toEqual({}); + expect((await ruleDataClientMock.getWriter()).bulk).toHaveBeenCalledWith( expect.objectContaining({ body: [ @@ -1070,7 +1107,7 @@ describe('createLifecycleExecutor', () => { [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', [ALERT_WORKFLOW_STATUS]: 'closed', [ALERT_STATUS]: ALERT_STATUS_ACTIVE, - [ALERT_FLAPPING]: true, + [ALERT_FLAPPING]: false, [EVENT_ACTION]: 'active', [EVENT_KIND]: 'signal', }), @@ -1182,7 +1219,7 @@ describe('createLifecycleExecutor', () => { return { state }; }); - await executor( + const serializedAlerts = await executor( createDefaultAlertExecutorOptions({ alertId: 'TEST_ALERT_0', params: {}, @@ -1228,6 +1265,44 @@ describe('createLifecycleExecutor', () => { }) ); + expect(serializedAlerts.state.trackedAlerts).toEqual({ + TEST_ALERT_2: { + alertId: 'TEST_ALERT_2', + alertUuid: 'TEST_ALERT_2_UUID', + flapping: true, + flappingHistory: [true, true, true], + pendingRecoveredCount: 1, + started: '2020-01-02T12:00:00.000Z', + }, + }); + + expect(serializedAlerts.state.trackedAlertsRecovered).toEqual({ + TEST_ALERT_0: { + alertId: 'TEST_ALERT_0', + alertUuid: 'TEST_ALERT_0_UUID', + flapping: true, + flappingHistory: [true, true, true, true, true], + pendingRecoveredCount: 0, + started: '2020-01-01T12:00:00.000Z', + }, + TEST_ALERT_1: { + alertId: 'TEST_ALERT_1', + alertUuid: 'TEST_ALERT_1_UUID', + flapping: false, + flappingHistory: notFlapping.slice(0, notFlapping.length - 1).concat([true]), + pendingRecoveredCount: 0, + started: '2020-01-02T12:00:00.000Z', + }, + TEST_ALERT_3: { + alertId: 'TEST_ALERT_3', + alertUuid: 'TEST_ALERT_3_UUID', + flapping: false, + flappingHistory: notFlapping.slice(0, notFlapping.length - 1).concat([true]), + pendingRecoveredCount: 0, + started: '2020-01-02T12:00:00.000Z', + }, + }); + expect((await ruleDataClientMock.getWriter()).bulk).toHaveBeenCalledWith( expect.objectContaining({ body: expect.arrayContaining([ @@ -1235,10 +1310,10 @@ describe('createLifecycleExecutor', () => { { index: { _id: 'TEST_ALERT_0_UUID' } }, expect.objectContaining({ [ALERT_INSTANCE_ID]: 'TEST_ALERT_0', - [ALERT_STATUS]: ALERT_STATUS_ACTIVE, - [EVENT_ACTION]: 'active', + [ALERT_STATUS]: ALERT_STATUS_RECOVERED, + [EVENT_ACTION]: 'close', [EVENT_KIND]: 'signal', - [ALERT_FLAPPING]: true, + [ALERT_FLAPPING]: false, }), { index: { _id: 'TEST_ALERT_1_UUID' } }, expect.objectContaining({ 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 f0eef646a1f28a..5d0a22d749060e 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 @@ -278,12 +278,7 @@ export const createLifecycleExecutor = trackedAlertRecoveredIds ); - const { - alertUuid, - started, - flapping: isCurrentlyFlapping, - pendingRecoveredCount, - } = !isNew + const { alertUuid, started, flapping, pendingRecoveredCount } = !isNew ? state.trackedAlerts[alertId] : { alertUuid: lifecycleAlertServices.getAlertUuid(alertId), @@ -294,8 +289,6 @@ export const createLifecycleExecutor = pendingRecoveredCount: 0, }; - const flapping = isFlapping(flappingSettings, flappingHistory, isCurrentlyFlapping); - const event: ParsedTechnicalFields & ParsedExperimentalFields = { ...alertData?.fields, ...commonRuleFields, @@ -366,10 +359,11 @@ export const createLifecycleExecutor = const nextTrackedAlerts = Object.fromEntries( allEventsToIndex .filter(({ event }) => event[ALERT_STATUS] !== ALERT_STATUS_RECOVERED) - .map(({ event, flappingHistory, flapping, pendingRecoveredCount }) => { + .map(({ event, flappingHistory, flapping: isCurrentlyFlapping, pendingRecoveredCount }) => { const alertId = event[ALERT_INSTANCE_ID]!; const alertUuid = event[ALERT_UUID]!; const started = new Date(event[ALERT_START]!).toISOString(); + const flapping = isFlapping(flappingSettings, flappingHistory, isCurrentlyFlapping); return [ alertId, { alertId, alertUuid, started, flappingHistory, flapping, pendingRecoveredCount }, @@ -387,10 +381,11 @@ export const createLifecycleExecutor = event[ALERT_STATUS] === ALERT_STATUS_RECOVERED && (flapping || flappingHistory.filter((f: boolean) => f).length > 0) ) - .map(({ event, flappingHistory, flapping, pendingRecoveredCount }) => { + .map(({ event, flappingHistory, flapping: isCurrentlyFlapping, pendingRecoveredCount }) => { const alertId = event[ALERT_INSTANCE_ID]!; const alertUuid = event[ALERT_UUID]!; const started = new Date(event[ALERT_START]!).toISOString(); + const flapping = isFlapping(flappingSettings, flappingHistory, isCurrentlyFlapping); return [ alertId, { alertId, alertUuid, started, flappingHistory, flapping, pendingRecoveredCount }, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts index 6b01ece5465f59..d787a5cb411fc6 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts @@ -540,15 +540,15 @@ export default function eventLogTests({ getService }: FtrProviderContext) { }); }); - it('should generate expected events for flapping alerts that are mainly active', async () => { + it('should generate expected events for flapping alerts that settle on active', async () => { await supertest .post(`${getUrlPrefix(space.id)}/internal/alerting/rules/settings/_flapping`) .set('kbn-xsrf', 'foo') .auth('superuser', 'superuser') .send({ enabled: true, - look_back_window: 3, - status_change_threshold: 2, + look_back_window: 6, + status_change_threshold: 4, }) .expect(200); const { body: createdAction } = await supertest @@ -563,7 +563,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) { .expect(200); // pattern of when the alert should fire - const instance = [true, false, true, true, true, true, true]; + const instance = [true, false, false, true, false, true, false, true, false].concat( + ...new Array(8).fill(true), + false + ); const pattern = { instance, }; @@ -585,6 +588,11 @@ export default function eventLogTests({ getService }: FtrProviderContext) { group: 'default', params: {}, }, + { + id: createdAction.id, + group: 'recovered', + params: {}, + }, ], notify_when: RuleNotifyWhen.CHANGE, }) @@ -606,10 +614,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) { // make sure the counts of the # of events per type are as expected ['execute-start', { gte: 6 }], ['execute', { gte: 6 }], - ['execute-action', { equal: 1 }], - ['new-instance', { equal: 1 }], + ['execute-action', { equal: 6 }], + ['new-instance', { equal: 3 }], ['active-instance', { gte: 6 }], - ['recovered-instance', { equal: 1 }], + ['recovered-instance', { equal: 3 }], ]), }); }); @@ -621,19 +629,24 @@ export default function eventLogTests({ getService }: FtrProviderContext) { event?.event?.action === 'recovered-instance' ) .map((event) => event?.kibana?.alert?.flapping); - const result = [false, true, true, true, false, false, false, false]; + const result = [false, false, false, false, false].concat( + new Array(9).fill(true), + false, + false, + false + ); expect(flapping).to.eql(result); }); - it('should generate expected events for flapping alerts that are mainly recovered', async () => { + it('should generate expected events for flapping alerts settle on recovered', async () => { await supertest .post(`${getUrlPrefix(space.id)}/internal/alerting/rules/settings/_flapping`) .set('kbn-xsrf', 'foo') .auth('superuser', 'superuser') .send({ enabled: true, - look_back_window: 3, - status_change_threshold: 2, + look_back_window: 6, + status_change_threshold: 4, }) .expect(200); const { body: createdAction } = await supertest @@ -648,7 +661,9 @@ export default function eventLogTests({ getService }: FtrProviderContext) { .expect(200); // pattern of when the alert should fire - const instance = [true, false, true, false, false, false, true]; + const instance = [true, false, false, true, false, true, false, true, false, true].concat( + new Array(11).fill(false) + ); const pattern = { instance, }; @@ -670,6 +685,11 @@ export default function eventLogTests({ getService }: FtrProviderContext) { group: 'default', params: {}, }, + { + id: createdAction.id, + group: 'recovered', + params: {}, + }, ], notify_when: RuleNotifyWhen.CHANGE, }) @@ -691,10 +711,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) { // make sure the counts of the # of events per type are as expected ['execute-start', { gte: 6 }], ['execute', { gte: 6 }], - ['execute-action', { equal: 2 }], - ['new-instance', { equal: 2 }], - ['active-instance', { gte: 6 }], - ['recovered-instance', { equal: 2 }], + ['execute-action', { equal: 6 }], + ['new-instance', { equal: 3 }], + ['active-instance', { gte: 3 }], + ['recovered-instance', { equal: 3 }], ]), }); }); @@ -706,18 +726,20 @@ export default function eventLogTests({ getService }: FtrProviderContext) { event?.event?.action === 'recovered-instance' ) .map((event) => event?.kibana?.alert?.flapping); - expect(flapping).to.eql([false, true, true, true, true, true, true, true]); + expect(flapping).to.eql( + [false, false, false, false, false].concat(new Array(8).fill(true)) + ); }); - it('should generate expected events for flapping alerts that are mainly active with notifyWhen not set to "on status change"', async () => { + it('should generate expected events for flapping alerts over a period of time longer than the look back', async () => { await supertest .post(`${getUrlPrefix(space.id)}/internal/alerting/rules/settings/_flapping`) .set('kbn-xsrf', 'foo') .auth('superuser', 'superuser') .send({ enabled: true, - look_back_window: 3, - status_change_threshold: 2, + look_back_window: 5, + status_change_threshold: 5, }) .expect(200); const { body: createdAction } = await supertest @@ -732,7 +754,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) { .expect(200); // pattern of when the alert should fire - const instance = [true, false, true, true, true, true, true]; + const instance = [true, false, false, true, false, true, false, true, false].concat( + ...new Array(8).fill(true), + false + ); const pattern = { instance, }; @@ -754,7 +779,13 @@ export default function eventLogTests({ getService }: FtrProviderContext) { group: 'default', params: {}, }, + { + id: createdAction.id, + group: 'recovered', + params: {}, + }, ], + notify_when: RuleNotifyWhen.CHANGE, }) ); @@ -772,12 +803,110 @@ export default function eventLogTests({ getService }: FtrProviderContext) { provider: 'alerting', actions: new Map([ // make sure the counts of the # of events per type are as expected - ['execute-start', { gte: 6 }], - ['execute', { gte: 6 }], - ['execute-action', { equal: 6 }], - ['new-instance', { equal: 1 }], + ['execute-start', { gte: 8 }], + ['execute', { gte: 8 }], + ['execute-action', { equal: 8 }], + ['new-instance', { equal: 4 }], + ['active-instance', { gte: 4 }], + ['recovered-instance', { equal: 4 }], + ]), + }); + }); + + const flapping = events + .filter( + (event) => + event?.event?.action === 'active-instance' || + event?.event?.action === 'recovered-instance' + ) + .map((event) => event?.kibana?.alert?.flapping); + const result = [false, false, false, false, false, false, false].concat( + new Array(6).fill(true), + false, + false, + false, + false + ); + expect(flapping).to.eql(result); + }); + + it('should generate expected events for flapping alerts that settle on active where notifyWhen is not set to "on status change"', async () => { + await supertest + .post(`${getUrlPrefix(space.id)}/internal/alerting/rules/settings/_flapping`) + .set('kbn-xsrf', 'foo') + .auth('superuser', 'superuser') + .send({ + enabled: true, + look_back_window: 6, + status_change_threshold: 4, + }) + .expect(200); + const { body: createdAction } = await supertest + .post(`${getUrlPrefix(space.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + connector_type_id: 'test.noop', + config: {}, + secrets: {}, + }) + .expect(200); + + // pattern of when the alert should fire + const instance = [true, false, false, true, false, true, false, true, false].concat( + ...new Array(8).fill(true), + false + ); + const pattern = { + instance, + }; + + const response = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + rule_type_id: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + params: { + pattern, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + { + id: createdAction.id, + group: 'recovered', + params: {}, + }, + ], + }) + ); + + expect(response.status).to.eql(200); + const alertId = response.body.id; + objectRemover.add(space.id, alertId, 'rule', 'alerting'); + + // get the events we're expecting + const events = await retry.try(async () => { + return await getEventLog({ + getService, + spaceId: space.id, + type: 'alert', + id: alertId, + provider: 'alerting', + actions: new Map([ + // make sure the counts of the # of events per type are as expected + ['execute-start', { gte: 15 }], + ['execute', { gte: 15 }], + ['execute-action', { equal: 15 }], + ['new-instance', { equal: 3 }], ['active-instance', { gte: 6 }], - ['recovered-instance', { equal: 1 }], + ['recovered-instance', { equal: 3 }], ]), }); }); @@ -789,19 +918,24 @@ export default function eventLogTests({ getService }: FtrProviderContext) { event?.event?.action === 'recovered-instance' ) .map((event) => event?.kibana?.alert?.flapping); - const result = [false, true, true, false, false, false, false]; + const result = [false, false, false, false, false].concat( + new Array(7).fill(true), + false, + false, + false + ); expect(flapping).to.eql(result); }); - it('should generate expected events for flapping alerts that are mainly recovered with notifyWhen not set to "on status change"', async () => { + it('should generate expected events for flapping alerts that settle on recovered where notifyWhen is not set to "on status change"', async () => { await supertest .post(`${getUrlPrefix(space.id)}/internal/alerting/rules/settings/_flapping`) .set('kbn-xsrf', 'foo') .auth('superuser', 'superuser') .send({ enabled: true, - look_back_window: 3, - status_change_threshold: 2, + look_back_window: 6, + status_change_threshold: 4, }) .expect(200); const { body: createdAction } = await supertest @@ -816,7 +950,9 @@ export default function eventLogTests({ getService }: FtrProviderContext) { .expect(200); // pattern of when the alert should fire - const instance = [true, false, true, false, false, false, true]; + const instance = [true, false, false, true, false, true, false, true, false, true].concat( + new Array(11).fill(false) + ); const pattern = { instance, }; @@ -838,6 +974,11 @@ export default function eventLogTests({ getService }: FtrProviderContext) { group: 'default', params: {}, }, + { + id: createdAction.id, + group: 'recovered', + params: {}, + }, ], }) ); @@ -856,12 +997,12 @@ export default function eventLogTests({ getService }: FtrProviderContext) { provider: 'alerting', actions: new Map([ // make sure the counts of the # of events per type are as expected - ['execute-start', { gte: 5 }], - ['execute', { gte: 5 }], - ['execute-action', { equal: 3 }], - ['new-instance', { equal: 2 }], + ['execute-start', { gte: 8 }], + ['execute', { gte: 8 }], + ['execute-action', { equal: 8 }], + ['new-instance', { equal: 3 }], ['active-instance', { gte: 3 }], - ['recovered-instance', { equal: 2 }], + ['recovered-instance', { equal: 3 }], ]), }); }); @@ -873,7 +1014,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { event?.event?.action === 'recovered-instance' ) .map((event) => event?.kibana?.alert?.flapping); - expect(flapping).to.eql([false, true, true, true, true]); + expect(flapping).to.eql([false, false, false, false, false, true, true, true]); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/event_log_alerts.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/event_log_alerts.ts index 97ef276fef930f..cc5f3108ddd544 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/event_log_alerts.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/event_log_alerts.ts @@ -132,7 +132,7 @@ export default function eventLogAlertTests({ getService }: FtrProviderContext) { break; } } - expect(flapping).to.eql(new Array(instanceEvents.length - 1).fill(false).concat([true])); + expect(flapping).to.eql(new Array(instanceEvents.length).fill(false)); }); }); }