From 5eebb6d7175f2bfba94ac53b32262babf58dfe18 Mon Sep 17 00:00:00 2001 From: Zacqary Adam Xeper Date: Tue, 1 Dec 2020 13:52:43 -0600 Subject: [PATCH] [Metrics UI] Implement Resolved action group in Metrics alerts (#83687) (#84687) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../inventory_metric_threshold_executor.ts | 11 ++- .../metric_threshold_executor.test.ts | 88 +++++++++---------- .../metric_threshold_executor.ts | 12 ++- 3 files changed, 64 insertions(+), 47 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts index b56ede19743939..14785f64cffac0 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts +++ b/x-pack/plugins/infra/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 { ResolvedActionGroup } from '../../../../../alerts/common'; import { AlertExecutorOptions } from '../../../../../alerts/server'; import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types'; import { InfraBackendLibs } from '../../infra_types'; @@ -18,6 +19,7 @@ import { buildErrorAlertReason, buildFiredAlertReason, buildNoDataAlertReason, + buildRecoveredAlertReason, stateToAlertMessage, } from '../common/messages'; import { evaluateCondition } from './evaluate_condition'; @@ -56,6 +58,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) = const inventoryItems = Object.keys(first(results)!); for (const item of inventoryItems) { const alertInstance = services.alertInstanceFactory(`${item}`); + const prevState = alertInstance.getState(); // AND logic; all criteria must be across the threshold const shouldAlertFire = results.every((result) => // Grab the result of the most recent bucket @@ -80,6 +83,10 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) = reason = results .map((result) => buildReasonWithVerboseMetricName(result[item], buildFiredAlertReason)) .join('\n'); + } else if (nextState === AlertStates.OK && prevState?.alertState === AlertStates.ALERT) { + reason = results + .map((result) => buildReasonWithVerboseMetricName(result[item], buildRecoveredAlertReason)) + .join('\n'); } if (alertOnNoData) { if (nextState === AlertStates.NO_DATA) { @@ -95,7 +102,9 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) = } } if (reason) { - alertInstance.scheduleActions(FIRED_ACTIONS.id, { + const actionGroupId = + nextState === AlertStates.OK ? ResolvedActionGroup.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.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts index 3a52bb6b6ce710..b31afba8ac9cc3 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts @@ -6,6 +6,7 @@ import { createMetricThresholdExecutor, FIRED_ACTIONS } from './metric_threshold_executor'; import { Comparator, AlertStates } from './types'; import * as mocks from './test_mocks'; +import { ResolvedActionGroup } from '../../../../../alerts/common'; import { AlertExecutorOptions } from '../../../../../alerts/server'; import { alertsMock, @@ -20,7 +21,7 @@ interface AlertTestInstance { state: any; } -let persistAlertInstances = false; // eslint-disable-line +let persistAlertInstances = false; describe('The metric threshold alert type', () => { describe('querying the entire infrastructure', () => { @@ -343,50 +344,49 @@ describe('The metric threshold alert type', () => { }); }); - // describe('querying a metric that later recovers', () => { - // const instanceID = '*'; - // const execute = (threshold: number[]) => - // executor({ - // - // services, - // params: { - // criteria: [ - // { - // ...baseCriterion, - // comparator: Comparator.GT, - // threshold, - // }, - // ], - // }, - // }); - // beforeAll(() => (persistAlertInstances = true)); - // afterAll(() => (persistAlertInstances = false)); + describe('querying a metric that later recovers', () => { + const instanceID = '*'; + const execute = (threshold: number[]) => + executor({ + services, + params: { + criteria: [ + { + ...baseCriterion, + comparator: Comparator.GT, + threshold, + }, + ], + }, + }); + beforeAll(() => (persistAlertInstances = true)); + afterAll(() => (persistAlertInstances = false)); - // test('sends a recovery alert as soon as the metric recovers', async () => { - // await execute([0.5]); - // expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); - // expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); - // await execute([2]); - // expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); - // expect(getState(instanceID).alertState).toBe(AlertStates.OK); - // }); - // test('does not continue to send a recovery alert if the metric is still OK', async () => { - // await execute([2]); - // expect(mostRecentAction(instanceID)).toBe(undefined); - // expect(getState(instanceID).alertState).toBe(AlertStates.OK); - // await execute([2]); - // expect(mostRecentAction(instanceID)).toBe(undefined); - // expect(getState(instanceID).alertState).toBe(AlertStates.OK); - // }); - // test('sends a recovery alert again once the metric alerts and recovers again', async () => { - // await execute([0.5]); - // expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); - // expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); - // await execute([2]); - // expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); - // expect(getState(instanceID).alertState).toBe(AlertStates.OK); - // }); - // }); + test('sends a recovery alert as soon as the metric recovers', async () => { + await execute([0.5]); + expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); + expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); + await execute([2]); + expect(mostRecentAction(instanceID).id).toBe(ResolvedActionGroup.id); + expect(getState(instanceID).alertState).toBe(AlertStates.OK); + }); + test('does not continue to send a recovery alert if the metric is still OK', async () => { + await execute([2]); + expect(mostRecentAction(instanceID)).toBe(undefined); + expect(getState(instanceID).alertState).toBe(AlertStates.OK); + await execute([2]); + expect(mostRecentAction(instanceID)).toBe(undefined); + expect(getState(instanceID).alertState).toBe(AlertStates.OK); + }); + test('sends a recovery alert again once the metric alerts and recovers again', async () => { + await execute([0.5]); + expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); + expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); + await execute([2]); + expect(mostRecentAction(instanceID).id).toBe(ResolvedActionGroup.id); + expect(getState(instanceID).alertState).toBe(AlertStates.OK); + }); + }); describe('querying a metric with a percentage metric', () => { const instanceID = '*'; 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 index 4dec552c5bd6c6..7c3918c37ebbf5 100644 --- 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,12 +6,14 @@ import { first, last } from 'lodash'; import { i18n } from '@kbn/i18n'; import moment from 'moment'; +import { ResolvedActionGroup } from '../../../../../alerts/common'; import { AlertExecutorOptions } from '../../../../../alerts/server'; import { InfraBackendLibs } from '../../infra_types'; import { buildErrorAlertReason, buildFiredAlertReason, buildNoDataAlertReason, + buildRecoveredAlertReason, stateToAlertMessage, } from '../common/messages'; import { createFormatter } from '../../../../common/formatters'; @@ -40,6 +42,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => const groups = Object.keys(first(alertResults)!); for (const group of groups) { const alertInstance = services.alertInstanceFactory(`${group}`); + const prevState = alertInstance.getState(); // AND logic; all criteria must be across the threshold const shouldAlertFire = alertResults.every((result) => @@ -64,6 +67,10 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => reason = alertResults .map((result) => buildFiredAlertReason(formatAlertResult(result[group]))) .join('\n'); + } else if (nextState === AlertStates.OK && prevState?.alertState === AlertStates.ALERT) { + reason = alertResults + .map((result) => buildRecoveredAlertReason(formatAlertResult(result[group]))) + .join('\n'); } if (alertOnNoData) { if (nextState === AlertStates.NO_DATA) { @@ -81,7 +88,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 ? ResolvedActionGroup.id : FIRED_ACTIONS.id; + alertInstance.scheduleActions(actionGroupId, { group, alertState: stateToAlertMessage[nextState], reason, @@ -98,7 +107,6 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => }); } - // Future use: ability to fetch display current alert state alertInstance.replaceState({ alertState: nextState, });