Revert the Revert of "[Alerting] renames Resolved action group to Rec…

…overed (#84123)" (#84662) Reapplies the #84123 PR: This PR changes the default term from “Resolved” to “Recovered”, as it fits most use cases and we feel users are most likely to understand its meaning across domains. # Conflicts: # x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts # x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts # x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts # x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx
elastic · Dec 1, 2020 · 4ea20a0 · 4ea20a0
1 parent d4cc408
commit 4ea20a0
Show file tree

Hide file tree

Showing 20 changed files with 153 additions and 90 deletions.
diff --git a/x-pack/plugins/alerts/common/builtin_action_groups.ts b/x-pack/plugins/alerts/common/builtin_action_groups.ts
@@ -6,13 +6,13 @@
 import { i18n } from '@kbn/i18n';
 import { ActionGroup } from './alert_type';
 
-export const ResolvedActionGroup: ActionGroup = {
-  id: 'resolved',
-  name: i18n.translate('xpack.alerts.builtinActionGroups.resolved', {
-    defaultMessage: 'Resolved',
+export const RecoveredActionGroup: ActionGroup = {
+  id: 'recovered',
+  name: i18n.translate('xpack.alerts.builtinActionGroups.recovered', {
+    defaultMessage: 'Recovered',
   }),
 };
 
 export function getBuiltinActionGroups(): ActionGroup[] {
-  return [ResolvedActionGroup];
+  return [RecoveredActionGroup];
 }
diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts
@@ -105,8 +105,8 @@ describe('register()', () => {
           name: 'Default',
         },
         {
-          id: 'resolved',
-          name: 'Resolved',
+          id: 'recovered',
+          name: 'Recovered',
         },
       ],
       defaultActionGroupId: 'default',
@@ -117,7 +117,7 @@ describe('register()', () => {
 
     expect(() => registry.register(alertType)).toThrowError(
       new Error(
-        `Alert type [id="${alertType.id}"] cannot be registered. Action groups [resolved] are reserved by the framework.`
+        `Alert type [id="${alertType.id}"] cannot be registered. Action groups [recovered] are reserved by the framework.`
       )
     );
   });
@@ -229,8 +229,8 @@ describe('get()', () => {
             "name": "Default",
           },
           Object {
-            "id": "resolved",
-            "name": "Resolved",
+            "id": "recovered",
+            "name": "Recovered",
           },
         ],
         "actionVariables": Object {
@@ -287,8 +287,8 @@ describe('list()', () => {
               "name": "Test Action Group",
             },
             Object {
-              "id": "resolved",
-              "name": "Resolved",
+              "id": "recovered",
+              "name": "Recovered",
             },
           ],
           "actionVariables": Object {

diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts
@@ -122,7 +122,7 @@ describe('getAlertInstanceSummary()', () => {
       .addActiveInstance('instance-previously-active', 'action group B')
       .advanceTime(10000)
       .addExecute()
-      .addResolvedInstance('instance-previously-active')
+      .addRecoveredInstance('instance-previously-active')
       .addActiveInstance('instance-currently-active', 'action group A')
       .getEvents();
     const eventsResult = {

diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts
@@ -6,7 +6,7 @@
 
 import { SanitizedAlert, AlertInstanceSummary } from '../types';
 import { IValidatedEvent } from '../../../event_log/server';
-import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER } from '../plugin';
+import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER, LEGACY_EVENT_LOG_ACTIONS } from '../plugin';
 import { alertInstanceSummaryFromEventLog } from './alert_instance_summary_from_event_log';
 
 const ONE_HOUR_IN_MILLIS = 60 * 60 * 1000;
@@ -189,7 +189,43 @@ describe('alertInstanceSummaryFromEventLog', () => {
       .addActiveInstance('instance-1', 'action group A')
       .advanceTime(10000)
       .addExecute()
-      .addResolvedInstance('instance-1')
+      .addRecoveredInstance('instance-1')
+      .getEvents();
+
+    const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({
+      alert,
+      events,
+      dateStart,
+      dateEnd,
+    });
+
+    const { lastRun, status, instances } = summary;
+    expect({ lastRun, status, instances }).toMatchInlineSnapshot(`
+      Object {
+        "instances": Object {
+          "instance-1": Object {
+            "actionGroupId": undefined,
+            "activeStartDate": undefined,
+            "muted": false,
+            "status": "OK",
+          },
+        },
+        "lastRun": "2020-06-18T00:00:10.000Z",
+        "status": "OK",
+      }
+    `);
+  });
+
+  test('legacy alert with currently inactive instance', async () => {
+    const alert = createAlert({});
+    const eventsFactory = new EventsFactory();
+    const events = eventsFactory
+      .addExecute()
+      .addNewInstance('instance-1')
+      .addActiveInstance('instance-1', 'action group A')
+      .advanceTime(10000)
+      .addExecute()
+      .addLegacyResolvedInstance('instance-1')
       .getEvents();
 
     const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({
@@ -224,7 +260,7 @@ describe('alertInstanceSummaryFromEventLog', () => {
       .addActiveInstance('instance-1', 'action group A')
       .advanceTime(10000)
       .addExecute()
-      .addResolvedInstance('instance-1')
+      .addRecoveredInstance('instance-1')
       .getEvents();
 
     const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({
@@ -406,7 +442,7 @@ describe('alertInstanceSummaryFromEventLog', () => {
       .advanceTime(10000)
       .addExecute()
       .addActiveInstance('instance-1', 'action group A')
-      .addResolvedInstance('instance-2')
+      .addRecoveredInstance('instance-2')
       .getEvents();
 
     const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({
@@ -451,7 +487,7 @@ describe('alertInstanceSummaryFromEventLog', () => {
       .advanceTime(10000)
       .addExecute()
       .addActiveInstance('instance-1', 'action group A')
-      .addResolvedInstance('instance-2')
+      .addRecoveredInstance('instance-2')
       .advanceTime(10000)
       .addExecute()
       .addActiveInstance('instance-1', 'action group B')
@@ -561,12 +597,24 @@ export class EventsFactory {
     return this;
   }
 
-  addResolvedInstance(instanceId: string): EventsFactory {
+  addRecoveredInstance(instanceId: string): EventsFactory {
+    this.events.push({
+      '@timestamp': this.date,
+      event: {
+        provider: EVENT_LOG_PROVIDER,
+        action: EVENT_LOG_ACTIONS.recoveredInstance,
+      },
+      kibana: { alerting: { instance_id: instanceId } },
+    });
+    return this;
+  }
+
+  addLegacyResolvedInstance(instanceId: string): EventsFactory {
     this.events.push({
       '@timestamp': this.date,
       event: {
         provider: EVENT_LOG_PROVIDER,
-        action: EVENT_LOG_ACTIONS.resolvedInstance,
+        action: LEGACY_EVENT_LOG_ACTIONS.resolvedInstance,
       },
       kibana: { alerting: { instance_id: instanceId } },
     });

diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts
@@ -6,7 +6,7 @@
 
 import { SanitizedAlert, AlertInstanceSummary, AlertInstanceStatus } from '../types';
 import { IEvent } from '../../../event_log/server';
-import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER } from '../plugin';
+import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER, LEGACY_EVENT_LOG_ACTIONS } from '../plugin';
 
 export interface AlertInstanceSummaryFromEventLogParams {
   alert: SanitizedAlert;
@@ -80,7 +80,8 @@ export function alertInstanceSummaryFromEventLog(
         status.status = 'Active';
         status.actionGroupId = event?.kibana?.alerting?.action_group_id;
         break;
-      case EVENT_LOG_ACTIONS.resolvedInstance:
+      case LEGACY_EVENT_LOG_ACTIONS.resolvedInstance:
+      case EVENT_LOG_ACTIONS.recoveredInstance:
         status.status = 'OK';
         status.activeStartDate = undefined;
         status.actionGroupId = undefined;

diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts
@@ -82,9 +82,12 @@ export const EVENT_LOG_ACTIONS = {
   execute: 'execute',
   executeAction: 'execute-action',
   newInstance: 'new-instance',
-  resolvedInstance: 'resolved-instance',
+  recoveredInstance: 'recovered-instance',
   activeInstance: 'active-instance',
 };
+export const LEGACY_EVENT_LOG_ACTIONS = {
+  resolvedInstance: 'resolved-instance',
+};
 
 export interface PluginSetupContract {
   registerType: AlertTypeRegistry['register'];

diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts
@@ -26,12 +26,12 @@ import { alertsMock, alertsClientMock } from '../mocks';
 import { eventLoggerMock } from '../../../event_log/server/event_logger.mock';
 import { IEventLogger } from '../../../event_log/server';
 import { SavedObjectsErrorHelpers } from '../../../../../src/core/server';
-import { Alert, ResolvedActionGroup } from '../../common';
+import { Alert, RecoveredActionGroup } from '../../common';
 import { omit } from 'lodash';
 const alertType = {
   id: 'test',
   name: 'My test alert',
-  actionGroups: [{ id: 'default', name: 'Default' }, ResolvedActionGroup],
+  actionGroups: [{ id: 'default', name: 'Default' }, RecoveredActionGroup],
   defaultActionGroupId: 'default',
   executor: jest.fn(),
   producer: 'alerts',
@@ -114,7 +114,7 @@ describe('Task Runner', () => {
         },
       },
       {
-        group: ResolvedActionGroup.id,
+        group: RecoveredActionGroup.id,
         id: '2',
         actionTypeId: 'action',
         params: {
@@ -517,7 +517,7 @@ describe('Task Runner', () => {
     `);
   });
 
-  test('fire resolved actions for execution for the alertInstances which is in the resolved state', async () => {
+  test('fire recovered actions for execution for the alertInstances which is in the recovered state', async () => {
     taskRunnerFactoryInitializerParams.actionsPlugin.isActionTypeEnabled.mockReturnValue(true);
     taskRunnerFactoryInitializerParams.actionsPlugin.isActionExecutable.mockReturnValue(true);
 
@@ -650,7 +650,7 @@ describe('Task Runner', () => {
         Array [
           Object {
             "event": Object {
-              "action": "resolved-instance",
+              "action": "recovered-instance",
             },
             "kibana": Object {
               "alerting": Object {
@@ -666,7 +666,7 @@ describe('Task Runner', () => {
                 },
               ],
             },
-            "message": "test:1: 'alert-name' resolved instance: '2'",
+            "message": "test:1: 'alert-name' instance '2' has recovered",
           },
         ],
         Array [

diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts
@@ -39,7 +39,7 @@ import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_l
 import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error';
 import { AlertsClient } from '../alerts_client';
 import { partiallyUpdateAlert } from '../saved_objects';
-import { ResolvedActionGroup } from '../../common';
+import { RecoveredActionGroup } from '../../common';
 
 const FALLBACK_RETRY_INTERVAL = '5m';
 
@@ -219,7 +219,7 @@ export class TaskRunner {
       alertInstance.hasScheduledActions()
     );
 
-    generateNewAndResolvedInstanceEvents({
+    generateNewAndRecoveredInstanceEvents({
       eventLogger,
       originalAlertInstances,
       currentAlertInstances: instancesWithScheduledActions,
@@ -229,7 +229,7 @@ export class TaskRunner {
     });
 
     if (!muteAll) {
-      scheduleActionsForResolvedInstances(
+      scheduleActionsForRecoveredInstances(
         alertInstances,
         executionHandler,
         originalAlertInstances,
@@ -436,7 +436,7 @@ export class TaskRunner {
   }
 }
 
-interface GenerateNewAndResolvedInstanceEventsParams {
+interface GenerateNewAndRecoveredInstanceEventsParams {
   eventLogger: IEventLogger;
   originalAlertInstances: Dictionary<AlertInstance>;
   currentAlertInstances: Dictionary<AlertInstance>;
@@ -445,18 +445,20 @@ interface GenerateNewAndResolvedInstanceEventsParams {
   namespace: string | undefined;
 }
 
-function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInstanceEventsParams) {
+function generateNewAndRecoveredInstanceEvents(
+  params: GenerateNewAndRecoveredInstanceEventsParams
+) {
   const { eventLogger, alertId, namespace, currentAlertInstances, originalAlertInstances } = params;
   const originalAlertInstanceIds = Object.keys(originalAlertInstances);
   const currentAlertInstanceIds = Object.keys(currentAlertInstances);
 
   const newIds = without(currentAlertInstanceIds, ...originalAlertInstanceIds);
-  const resolvedIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds);
+  const recoveredIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds);
 
-  for (const id of resolvedIds) {
+  for (const id of recoveredIds) {
     const actionGroup = originalAlertInstances[id].getLastScheduledActions()?.group;
-    const message = `${params.alertLabel} resolved instance: '${id}'`;
-    logInstanceEvent(id, EVENT_LOG_ACTIONS.resolvedInstance, message, actionGroup);
+    const message = `${params.alertLabel} instance '${id}' has recovered`;
+    logInstanceEvent(id, EVENT_LOG_ACTIONS.recoveredInstance, message, actionGroup);
   }
 
   for (const id of newIds) {
@@ -496,7 +498,7 @@ function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInst
   }
 }
 
-function scheduleActionsForResolvedInstances(
+function scheduleActionsForRecoveredInstances(
   alertInstancesMap: Record<string, AlertInstance>,
   executionHandler: ReturnType<typeof createExecutionHandler>,
   originalAlertInstances: Record<string, AlertInstance>,
@@ -505,22 +507,22 @@ function scheduleActionsForResolvedInstances(
 ) {
   const currentAlertInstanceIds = Object.keys(currentAlertInstances);
   const originalAlertInstanceIds = Object.keys(originalAlertInstances);
-  const resolvedIds = without(
+  const recoveredIds = without(
     originalAlertInstanceIds,
     ...currentAlertInstanceIds,
     ...mutedInstanceIds
   );
-  for (const id of resolvedIds) {
+  for (const id of recoveredIds) {
     const instance = alertInstancesMap[id];
-    instance.updateLastScheduledActions(ResolvedActionGroup.id);
+    instance.updateLastScheduledActions(RecoveredActionGroup.id);
     instance.unscheduleActions();
     executionHandler({
-      actionGroup: ResolvedActionGroup.id,
+      actionGroup: RecoveredActionGroup.id,
       context: {},
       state: {},
       alertInstanceId: id,
     });
-    instance.scheduleActions(ResolvedActionGroup.id);
+    instance.scheduleActions(RecoveredActionGroup.id);
   }
 }
 

diff --git a/...fra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/...fra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
@@ -9,6 +9,7 @@ import moment from 'moment';
 import { getCustomMetricLabel } from '../../../../common/formatters/get_custom_metric_label';
 import { toMetricOpt } from '../../../../common/snapshot_metric_i18n';
 import { AlertStates, InventoryMetricConditions } from './types';
+import { RecoveredActionGroup } from '../../../../../alerts/common';
 import { AlertExecutorOptions } from '../../../../../alerts/server';
 import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types';
 import { InfraBackendLibs } from '../../infra_types';
@@ -95,7 +96,9 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
       }
     }
     if (reason) {
-      alertInstance.scheduleActions(FIRED_ACTIONS.id, {
+      const actionGroupId =
+        nextState === AlertStates.OK ? RecoveredActionGroup.id : FIRED_ACTIONS.id;
+      alertInstance.scheduleActions(actionGroupId, {
         group: item,
         alertState: stateToAlertMessage[nextState],
         reason,

diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts
@@ -6,6 +6,7 @@
 import { first, last } from 'lodash';
 import { i18n } from '@kbn/i18n';
 import moment from 'moment';
+import { RecoveredActionGroup } from '../../../../../alerts/common';
 import { AlertExecutorOptions } from '../../../../../alerts/server';
 import { InfraBackendLibs } from '../../infra_types';
 import {
@@ -81,7 +82,9 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) =>
       if (reason) {
         const firstResult = first(alertResults);
         const timestamp = (firstResult && firstResult[group].timestamp) ?? moment().toISOString();
-        alertInstance.scheduleActions(FIRED_ACTIONS.id, {
+        const actionGroupId =
+          nextState === AlertStates.OK ? RecoveredActionGroup.id : FIRED_ACTIONS.id;
+        alertInstance.scheduleActions(actionGroupId, {
           group,
           alertState: stateToAlertMessage[nextState],
           reason,