Skip to content

Commit

Permalink
[Metrics UI] Implement Resolved action group in Metrics alerts (#83687)…
Browse files Browse the repository at this point in the history 
… (#84687)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@Zacqary @kibanamachine
Zacqary and kibanamachine committed Dec 1, 2020
1 parent c826947 commit 5eebb6d
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 47 deletions.
11 changes: 10 additions & 1 deletion ...fra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
View file
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 { ResolvedActionGroup } from '../../../../../alerts/common';
import { AlertExecutorOptions } from '../../../../../alerts/server';
import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types';
import { InfraBackendLibs } from '../../infra_types';
Expand All @@ -18,6 +19,7 @@ import {
buildErrorAlertReason,
buildFiredAlertReason,
buildNoDataAlertReason,
buildRecoveredAlertReason,
stateToAlertMessage,
} from '../common/messages';
import { evaluateCondition } from './evaluate_condition';
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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,
Expand Down
88 changes: 44 additions & 44 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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 = '*';
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) =>
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -98,7 +107,6 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) =>
});
}

// Future use: ability to fetch display current alert state
alertInstance.replaceState({
alertState: nextState,
});
Expand Down

0 comments on commit 5eebb6d

Please sign in to comment.