[8.7] [ResponseOps] Update flapping logic order to determine whether …

…an alert is flapping after it's returned for notification (#151148) (#152507) # Backport This will backport the following commits from `main` to `8.7`: - [[ResponseOps] Update flapping logic order to determine whether an alert is flapping after it's returned for notification (#151148)](#151148)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Alexi Doak <109488926+doakalexi@users.noreply.github.com>
elastic · Mar 1, 2023 · 99c6baf · 99c6baf
1 parent e4f24cb
commit 99c6baf
Show file tree

Hide file tree

Showing 7 changed files with 276 additions and 74 deletions.
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
@@ -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<AlertInstanceContext, AlertInstanceContext>('1', testAlert1),
-        '2': new Alert<AlertInstanceContext, AlertInstanceContext>('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
@@ -143,12 +143,6 @@ export class LegacyAlertsClient<
       flappingSettings,
     });
 
-    setFlapping<State, Context, ActionGroupIds, RecoveryActionGroupId>(
-      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<State, Context, ActionGroupIds, RecoveryActionGroupId>(
+      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
@@ -466,6 +466,8 @@ export class TaskRunner<
       }
     });
 
+    this.legacyAlertsClient.setFlapping(flappingSettings);
+
     let alertsToReturn: Record<string, RawAlertInstance> = {};
     let recoveredAlertsToReturn: Record<string, RawAlertInstance> = {};
     // 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
@@ -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,17 +1265,55 @@ 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([
             // alert document
             { 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
@@ -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 },