Skip to content

Commit

Permalink
Revert the Revert of "[Alerting] renames Resolved action group to Rec…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
gmmorris committed Dec 1, 2020
1 parent 68decb8 commit 6da6db2
Show file tree
Hide file tree
Showing 21 changed files with 152 additions and 95 deletions.
10 changes: 5 additions & 5 deletions x-pack/plugins/alerts/common/builtin_action_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
14 changes: 7 additions & 7 deletions x-pack/plugins/alerts/server/alert_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ describe('register()', () => {
name: 'Default',
},
{
id: 'resolved',
name: 'Resolved',
id: 'recovered',
name: 'Recovered',
},
],
defaultActionGroupId: 'default',
Expand All @@ -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.`
)
);
});
Expand Down Expand Up @@ -229,8 +229,8 @@ describe('get()', () => {
"name": "Default",
},
Object {
"id": "resolved",
"name": "Resolved",
"id": "recovered",
"name": "Recovered",
},
],
"actionVariables": Object {
Expand Down Expand Up @@ -287,8 +287,8 @@ describe('list()', () => {
"name": "Test Action Group",
},
Object {
"id": "resolved",
"name": "Resolved",
"id": "recovered",
"name": "Recovered",
},
],
"actionVariables": Object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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 } },
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/alerts/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down
12 changes: 6 additions & 6 deletions x-pack/plugins/alerts/server/task_runner/task_runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -114,7 +114,7 @@ describe('Task Runner', () => {
},
},
{
group: ResolvedActionGroup.id,
group: RecoveredActionGroup.id,
id: '2',
actionTypeId: 'action',
params: {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -650,7 +650,7 @@ describe('Task Runner', () => {
Array [
Object {
"event": Object {
"action": "resolved-instance",
"action": "recovered-instance",
},
"kibana": Object {
"alerting": Object {
Expand All @@ -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 [
Expand Down
32 changes: 17 additions & 15 deletions x-pack/plugins/alerts/server/task_runner/task_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -219,7 +219,7 @@ export class TaskRunner {
alertInstance.hasScheduledActions()
);

generateNewAndResolvedInstanceEvents({
generateNewAndRecoveredInstanceEvents({
eventLogger,
originalAlertInstances,
currentAlertInstances: instancesWithScheduledActions,
Expand All @@ -229,7 +229,7 @@ export class TaskRunner {
});

if (!muteAll) {
scheduleActionsForResolvedInstances(
scheduleActionsForRecoveredInstances(
alertInstances,
executionHandler,
originalAlertInstances,
Expand Down Expand Up @@ -436,7 +436,7 @@ export class TaskRunner {
}
}

interface GenerateNewAndResolvedInstanceEventsParams {
interface GenerateNewAndRecoveredInstanceEventsParams {
eventLogger: IEventLogger;
originalAlertInstances: Dictionary<AlertInstance>;
currentAlertInstances: Dictionary<AlertInstance>;
Expand All @@ -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) {
Expand Down Expand Up @@ -496,7 +498,7 @@ function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInst
}
}

function scheduleActionsForResolvedInstances(
function scheduleActionsForRecoveredInstances(
alertInstancesMap: Record<string, AlertInstance>,
executionHandler: ReturnType<typeof createExecutionHandler>,
originalAlertInstances: Record<string, AlertInstance>,
Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +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 { ResolvedActionGroup } from '../../../../../alerts/common';
import { RecoveredActionGroup } from '../../../../../alerts/common';
import { AlertExecutorOptions } from '../../../../../alerts/server';
import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types';
import { InfraBackendLibs } from '../../infra_types';
Expand Down Expand Up @@ -103,7 +103,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
}
if (reason) {
const actionGroupId =
nextState === AlertStates.OK ? ResolvedActionGroup.id : FIRED_ACTIONS.id;
nextState === AlertStates.OK ? RecoveredActionGroup.id : FIRED_ACTIONS.id;
alertInstance.scheduleActions(actionGroupId, {
group: item,
alertState: stateToAlertMessage[nextState],
Expand Down
Loading

0 comments on commit 6da6db2

Please sign in to comment.