Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] Revert the Revert of "[Alerting] renames Resolved action group to Recovered (#84123)" (#84662) #84685

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,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';
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
Loading