Skip to content

Commit

Permalink
[Metrics Alerts] Add outside range comparator (#63993)
Browse files Browse the repository at this point in the history
  • Loading branch information
Zacqary committed Apr 22, 2020
1 parent cd05428 commit 12aae67
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { i18n } from '@kbn/i18n';
import {
MetricExpressionParams,
Comparator,
Aggregators,
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
} from '../../../../server/lib/alerting/metric_threshold/types';
import { euiStyled } from '../../../../../observability/public';
Expand All @@ -31,6 +32,8 @@ import {
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
} from '../../../../../triggers_actions_ui/public/common';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { builtInComparators } from '../../../../../triggers_actions_ui/public/common/constants';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { IErrorObject } from '../../../../../triggers_actions_ui/public/types';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { AlertsContextValue } from '../../../../../triggers_actions_ui/public/application/context/alerts_context';
Expand Down Expand Up @@ -64,24 +67,25 @@ type MetricExpression = Omit<MetricExpressionParams, 'metric'> & {
metric?: string;
};

enum AGGREGATION_TYPES {
COUNT = 'count',
AVERAGE = 'avg',
SUM = 'sum',
MIN = 'min',
MAX = 'max',
RATE = 'rate',
CARDINALITY = 'cardinality',
}

const defaultExpression = {
aggType: AGGREGATION_TYPES.AVERAGE,
aggType: Aggregators.AVERAGE,
comparator: Comparator.GT,
threshold: [],
timeSize: 1,
timeUnit: 'm',
} as MetricExpression;

const customComparators = {
...builtInComparators,
[Comparator.OUTSIDE_RANGE]: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.outsideRangeLabel', {
defaultMessage: 'Is not between',
}),
value: Comparator.OUTSIDE_RANGE,
requiredValues: 2,
},
};

export const Expressions: React.FC<Props> = props => {
const { setAlertParams, alertParams, errors, alertsContext } = props;
const { source, createDerivedIndexPattern } = useSourceViaHttp({
Expand Down Expand Up @@ -339,7 +343,7 @@ const StyledExpression = euiStyled.div`
export const ExpressionRow: React.FC<ExpressionRowProps> = props => {
const { setAlertParams, expression, errors, expressionId, remove, fields, canDelete } = props;
const {
aggType = AGGREGATION_TYPES.MAX,
aggType = Aggregators.MAX,
metric,
comparator = Comparator.GT,
threshold = [],
Expand Down Expand Up @@ -410,6 +414,7 @@ export const ExpressionRow: React.FC<ExpressionRowProps> = props => {
<ThresholdExpression
thresholdComparator={comparator || Comparator.GT}
threshold={threshold}
customComparators={customComparators}
onChangeSelectedThresholdComparator={updateComparator}
onChangeSelectedThreshold={updateThreshold}
errors={errors}
Expand Down Expand Up @@ -442,46 +447,46 @@ export const aggregationType: { [key: string]: any } = {
}),
fieldRequired: true,
validNormalizedTypes: ['number'],
value: AGGREGATION_TYPES.AVERAGE,
value: Aggregators.AVERAGE,
},
max: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.max', {
defaultMessage: 'Max',
}),
fieldRequired: true,
validNormalizedTypes: ['number', 'date'],
value: AGGREGATION_TYPES.MAX,
value: Aggregators.MAX,
},
min: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.min', {
defaultMessage: 'Min',
}),
fieldRequired: true,
validNormalizedTypes: ['number', 'date'],
value: AGGREGATION_TYPES.MIN,
value: Aggregators.MIN,
},
cardinality: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.cardinality', {
defaultMessage: 'Cardinality',
}),
fieldRequired: false,
value: AGGREGATION_TYPES.CARDINALITY,
value: Aggregators.CARDINALITY,
validNormalizedTypes: ['number'],
},
rate: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.rate', {
defaultMessage: 'Rate',
}),
fieldRequired: false,
value: AGGREGATION_TYPES.RATE,
value: Aggregators.RATE,
validNormalizedTypes: ['number'],
},
count: {
text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.count', {
defaultMessage: 'Document count',
}),
fieldRequired: false,
value: AGGREGATION_TYPES.COUNT,
value: Aggregators.COUNT,
validNormalizedTypes: ['number'],
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
*/

import { i18n } from '@kbn/i18n';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { MetricExpressionParams } from '../../../../server/lib/alerting/metric_threshold/types';
import {
MetricExpressionParams,
Comparator,
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
} from '../../../../server/lib/alerting/metric_threshold/types';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { ValidationResult } from '../../../../../triggers_actions_ui/public/types';

Expand Down Expand Up @@ -60,7 +63,7 @@ export function validateMetricThreshold({
);
}

if (c.comparator === 'between' && (!c.threshold || c.threshold.length < 2)) {
if (c.comparator === Comparator.BETWEEN && (!c.threshold || c.threshold.length < 2)) {
errors[id].threshold1.push(
i18n.translate('xpack.infra.metrics.alertFlyout.error.thresholdRequired', {
defaultMessage: 'Threshold is required.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ describe('The metric threshold alert type', () => {
expect(mostRecentAction(instanceID)).toBe(undefined);
expect(getState(instanceID).alertState).toBe(AlertStates.OK);
});
test('alerts as expected with the outside range comparator', async () => {
await execute(Comparator.OUTSIDE_RANGE, [0, 0.75]);
expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id);
expect(getState(instanceID).alertState).toBe(AlertStates.ALERT);
await execute(Comparator.OUTSIDE_RANGE, [0, 1.5]);
expect(mostRecentAction(instanceID)).toBe(undefined);
expect(getState(instanceID).alertState).toBe(AlertStates.OK);
});
test('reports expected values to the action context', async () => {
await execute(Comparator.GT, [0.75]);
const { action } = mostRecentAction(instanceID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { InfraDatabaseSearchResponse } from '../../adapters/framework/adapter_ty
import { createAfterKeyHandler } from '../../../utils/create_afterkey_handler';
import { getAllCompositeData } from '../../../utils/get_all_composite_data';
import { networkTraffic } from '../../../../common/inventory_models/shared/metrics/snapshot/network_traffic';
import { MetricExpressionParams, Comparator, AlertStates } from './types';
import { MetricExpressionParams, Comparator, Aggregators, AlertStates } from './types';
import { AlertServices, AlertExecutorOptions } from '../../../../../alerting/server';
import { getIntervalInSeconds } from '../../../utils/get_interval_in_seconds';
import { getDateHistogramOffset } from '../../snapshot/query_helpers';
Expand Down Expand Up @@ -39,7 +39,7 @@ const getCurrentValueFromAggregations = (
const { buckets } = aggregations.aggregatedIntervals;
if (!buckets.length) return null; // No Data state
const mostRecentBucket = buckets[buckets.length - 1];
if (aggType === 'count') {
if (aggType === Aggregators.COUNT) {
return mostRecentBucket.doc_count;
}
const { value } = mostRecentBucket.aggregatedValue;
Expand Down Expand Up @@ -70,10 +70,10 @@ export const getElasticsearchMetricQuery = (
groupBy?: string,
filterQuery?: string
) => {
if (aggType === 'count' && metric) {
if (aggType === Aggregators.COUNT && metric) {
throw new Error('Cannot aggregate document count with a metric');
}
if (aggType !== 'count' && !metric) {
if (aggType !== Aggregators.COUNT && !metric) {
throw new Error('Can only aggregate without a metric if using the document count aggregator');
}
const interval = `${timeSize}${timeUnit}`;
Expand All @@ -85,9 +85,9 @@ export const getElasticsearchMetricQuery = (
const offset = getDateHistogramOffset(from, interval);

const aggregations =
aggType === 'count'
aggType === Aggregators.COUNT
? {}
: aggType === 'rate'
: aggType === Aggregators.RATE
? networkTraffic('aggregatedValue', metric)
: {
aggregatedValue: {
Expand Down Expand Up @@ -242,7 +242,8 @@ const getMetric: (
const comparatorMap = {
[Comparator.BETWEEN]: (value: number, [a, b]: number[]) =>
value >= Math.min(a, b) && value <= Math.max(a, b),
// `threshold` is always an array of numbers in case the BETWEEN comparator is
[Comparator.OUTSIDE_RANGE]: (value: number, [a, b]: number[]) => value < a || value > b,
// `threshold` is always an array of numbers in case the BETWEEN/OUTSIDE_RANGE comparator is
// used; all other compartors will just destructure the first value in the array
[Comparator.GT]: (a: number, [b]: number[]) => a > b,
[Comparator.LT]: (a: number, [b]: number[]) => a < b,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ import { i18n } from '@kbn/i18n';
import uuid from 'uuid';
import { schema } from '@kbn/config-schema';
import { PluginSetupContract } from '../../../../../alerting/server';
import { METRIC_EXPLORER_AGGREGATIONS } from '../../../../common/http_api/metrics_explorer';
import { createMetricThresholdExecutor, FIRED_ACTIONS } from './metric_threshold_executor';
import { METRIC_THRESHOLD_ALERT_TYPE_ID } from './types';
import { METRIC_THRESHOLD_ALERT_TYPE_ID, Comparator } from './types';

const oneOfLiterals = (arrayOfLiterals: Readonly<string[]>) =>
schema.string({
validate: value =>
arrayOfLiterals.includes(value) ? undefined : `must be one of ${arrayOfLiterals.join(' | ')}`,
});

export async function registerMetricThresholdAlertType(alertingPlugin: PluginSetupContract) {
if (!alertingPlugin) {
Expand All @@ -20,27 +27,15 @@ export async function registerMetricThresholdAlertType(alertingPlugin: PluginSet

const baseCriterion = {
threshold: schema.arrayOf(schema.number()),
comparator: schema.oneOf([
schema.literal('>'),
schema.literal('<'),
schema.literal('>='),
schema.literal('<='),
schema.literal('between'),
]),
comparator: oneOfLiterals(Object.values(Comparator)),
timeUnit: schema.string(),
timeSize: schema.number(),
};

const nonCountCriterion = schema.object({
...baseCriterion,
metric: schema.string(),
aggType: schema.oneOf([
schema.literal('avg'),
schema.literal('min'),
schema.literal('max'),
schema.literal('rate'),
schema.literal('cardinality'),
]),
aggType: oneOfLiterals(METRIC_EXPLORER_AGGREGATIONS),
});

const countCriterion = schema.object({
Expand Down
15 changes: 12 additions & 3 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { MetricsExplorerAggregation } from '../../../../common/http_api/metrics_explorer';

export const METRIC_THRESHOLD_ALERT_TYPE_ID = 'metrics.alert.threshold';

export enum Comparator {
Expand All @@ -14,6 +12,17 @@ export enum Comparator {
GT_OR_EQ = '>=',
LT_OR_EQ = '<=',
BETWEEN = 'between',
OUTSIDE_RANGE = 'outside',
}

export enum Aggregators {
COUNT = 'count',
AVERAGE = 'avg',
SUM = 'sum',
MIN = 'min',
MAX = 'max',
RATE = 'rate',
CARDINALITY = 'cardinality',
}

export enum AlertStates {
Expand All @@ -34,7 +43,7 @@ interface BaseMetricExpressionParams {
}

interface NonCountMetricExpressionParams extends BaseMetricExpressionParams {
aggType: Exclude<MetricsExplorerAggregation, 'count'>;
aggType: Exclude<Aggregators, Aggregators.COUNT>;
metric: string;
}

Expand Down

0 comments on commit 12aae67

Please sign in to comment.