From c8af703ebbb9bed8466593d4e5d43a365d54201d Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Thu, 19 Mar 2020 14:15:46 -0500 Subject: [PATCH 1/3] Remove metric field from doc count on backend --- .../metric_threshold_executor.test.ts | 1 + .../metric_threshold_executor.ts | 45 ++++++++++++------- .../register_metric_threshold_alert_type.ts | 44 +++++++++++++----- .../lib/alerting/metric_threshold/types.ts | 16 +++++-- .../apis/infra/metrics_alerting.ts | 35 +++++---------- 5 files changed, 87 insertions(+), 54 deletions(-) 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 a6b9b70feede2..d414538c78b3a 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 @@ -228,6 +228,7 @@ describe('The metric threshold alert type', () => { comparator, threshold, aggType: 'count', + metric: undefined, }, ], }, 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 8c509c017cf20..49767536e1a25 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 @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { mapValues } from 'lodash'; +import { mapValues, isEmpty } from 'lodash'; import { i18n } from '@kbn/i18n'; import { InfraDatabaseSearchResponse } from '../../adapters/framework/adapter_types'; import { createAfterKeyHandler } from '../../../utils/create_afterkey_handler'; @@ -63,6 +63,12 @@ export const getElasticsearchMetricQuery = ( groupBy?: string, filterQuery?: string ) => { + if (aggType === 'count' && metric) { + throw new Error('Cannot aggregate document count with a metric'); + } + if (aggType !== 'count' && !metric) { + throw new Error('Can only aggregate without a metric if using the document count aggregator'); + } const interval = `${timeSize}${timeUnit}`; const aggregations = @@ -108,26 +114,33 @@ export const getElasticsearchMetricQuery = ( } : baseAggs; + const rangeFilters = [ + { + range: { + '@timestamp': { + gte: `now-${interval}`, + }, + }, + }, + ]; + + const metricFieldFilters = metric + ? [ + { + exists: { + field: metric, + }, + }, + ] + : []; + const parsedFilterQuery = getParsedFilterQuery(filterQuery); + const queryFilters = !isEmpty(parsedFilterQuery) ? [parsedFilterQuery] : []; return { query: { bool: { - filter: [ - { - range: { - '@timestamp': { - gte: `now-${interval}`, - }, - }, - }, - { - exists: { - field: metric, - }, - }, - ], - ...parsedFilterQuery, + filter: [...rangeFilters, ...metricFieldFilters, ...queryFilters], }, }, size: 0, diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts index 501d7549e1712..ed3a9b2f4fe36 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts @@ -17,22 +17,44 @@ export async function registerMetricThresholdAlertType(alertingPlugin: PluginSet } const alertUUID = uuid.v4(); + const baseCriterion = { + threshold: schema.arrayOf(schema.number()), + comparator: schema.oneOf([ + schema.literal('>'), + schema.literal('<'), + schema.literal('>='), + schema.literal('<='), + schema.literal('between'), + ]), + timeUnit: schema.string(), + timeSize: schema.number(), + indexPattern: schema.string(), + }; + + 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'), + ]), + }); + + const countCriterion = schema.object({ + ...baseCriterion, + aggType: schema.literal('count'), + metric: schema.never(), + }); + alertingPlugin.registerType({ id: METRIC_THRESHOLD_ALERT_TYPE_ID, name: 'Metric Alert - Threshold', validate: { params: schema.object({ - criteria: schema.arrayOf( - schema.object({ - threshold: schema.arrayOf(schema.number()), - comparator: schema.string(), - aggType: schema.string(), - metric: schema.string(), - timeUnit: schema.string(), - timeSize: schema.number(), - indexPattern: schema.string(), - }) - ), + criteria: schema.arrayOf(schema.oneOf([countCriterion, nonCountCriterion])), groupBy: schema.maybe(schema.string()), filterQuery: schema.maybe(schema.string()), }), diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts index 07739c9d81bc4..557a071ec9175 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts @@ -25,12 +25,22 @@ export enum AlertStates { export type TimeUnit = 's' | 'm' | 'h' | 'd'; -export interface MetricExpressionParams { - aggType: MetricsExplorerAggregation; - metric: string; +interface BaseMetricExpressionParams { timeSize: number; timeUnit: TimeUnit; indexPattern: string; threshold: number[]; comparator: Comparator; } + +interface NonCountMetricExpressionParams extends BaseMetricExpressionParams { + aggType: Exclude; + metric: string; +} + +interface CountMetricExpressionParams extends BaseMetricExpressionParams { + aggType: 'count'; + metric: never; +} + +export type MetricExpressionParams = NonCountMetricExpressionParams | CountMetricExpressionParams; diff --git a/x-pack/test/api_integration/apis/infra/metrics_alerting.ts b/x-pack/test/api_integration/apis/infra/metrics_alerting.ts index 09f5a498ddc00..4f17f9db67483 100644 --- a/x-pack/test/api_integration/apis/infra/metrics_alerting.ts +++ b/x-pack/test/api_integration/apis/infra/metrics_alerting.ts @@ -13,11 +13,13 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function({ getService }: FtrProviderContext) { const client = getService('legacyEs'); const index = 'test-index'; - const baseParams = { - metric: 'test.metric', - timeUnit: 'm', - timeSize: 5, - }; + const getSearchParams = (aggType: string) => + ({ + aggType, + timeUnit: 'm', + timeSize: 5, + ...(aggType !== 'count' ? { metric: 'test.metric' } : {}), + } as MetricExpressionParams); describe('Metrics Threshold Alerts', () => { before(async () => { await client.index({ @@ -30,10 +32,7 @@ export default function({ getService }: FtrProviderContext) { describe('querying the entire infrastructure', () => { for (const aggType of aggs) { it(`should work with the ${aggType} aggregator`, async () => { - const searchBody = getElasticsearchMetricQuery({ - ...baseParams, - aggType, - } as MetricExpressionParams); + const searchBody = getElasticsearchMetricQuery(getSearchParams(aggType)); const result = await client.search({ index, body: searchBody, @@ -44,10 +43,7 @@ export default function({ getService }: FtrProviderContext) { } it('should work with a filterQuery', async () => { const searchBody = getElasticsearchMetricQuery( - { - ...baseParams, - aggType: 'avg', - } as MetricExpressionParams, + getSearchParams('avg'), undefined, '{"bool":{"should":[{"match_phrase":{"agent.hostname":"foo"}}],"minimum_should_match":1}}' ); @@ -62,13 +58,7 @@ export default function({ getService }: FtrProviderContext) { describe('querying with a groupBy parameter', () => { for (const aggType of aggs) { it(`should work with the ${aggType} aggregator`, async () => { - const searchBody = getElasticsearchMetricQuery( - { - ...baseParams, - aggType, - } as MetricExpressionParams, - 'agent.id' - ); + const searchBody = getElasticsearchMetricQuery(getSearchParams(aggType), 'agent.id'); const result = await client.search({ index, body: searchBody, @@ -79,10 +69,7 @@ export default function({ getService }: FtrProviderContext) { } it('should work with a filterQuery', async () => { const searchBody = getElasticsearchMetricQuery( - { - ...baseParams, - aggType: 'avg', - } as MetricExpressionParams, + getSearchParams('avg'), 'agent.id', '{"bool":{"should":[{"match_phrase":{"agent.hostname":"foo"}}],"minimum_should_match":1}}' ); From f852ae271f71bfbd5e7d1177e1e6b98d86e51571 Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Thu, 19 Mar 2020 16:27:39 -0500 Subject: [PATCH 2/3] Fix tests --- .../metric_threshold/metric_threshold_executor.test.ts | 2 +- .../alerting/metric_threshold/metric_threshold_executor.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 d414538c78b3a..feaa404ae960a 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 @@ -17,7 +17,7 @@ const alertInstances = new Map(); const services = { callCluster(_: string, { body }: any) { - const metric = body.query.bool.filter[1].exists.field; + const metric = body.query.bool.filter[1]?.exists.field; if (body.aggs.groupings) { if (body.aggs.groupings.composite.after) { return mocks.compositeEndResponse; 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 49767536e1a25..f1c5af7515041 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 @@ -135,12 +135,12 @@ export const getElasticsearchMetricQuery = ( : []; const parsedFilterQuery = getParsedFilterQuery(filterQuery); - const queryFilters = !isEmpty(parsedFilterQuery) ? [parsedFilterQuery] : []; return { query: { bool: { - filter: [...rangeFilters, ...metricFieldFilters, ...queryFilters], + filter: [...rangeFilters, ...metricFieldFilters], + ...parsedFilterQuery, }, }, size: 0, From d7298fa3e755d6a935f9618243abf3470d8d188f Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Thu, 19 Mar 2020 17:35:19 -0500 Subject: [PATCH 3/3] Type fix --- .../lib/alerting/metric_threshold/metric_threshold_executor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f1c5af7515041..778889ba0c7a5 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 @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { mapValues, isEmpty } from 'lodash'; +import { mapValues } from 'lodash'; import { i18n } from '@kbn/i18n'; import { InfraDatabaseSearchResponse } from '../../adapters/framework/adapter_types'; import { createAfterKeyHandler } from '../../../utils/create_afterkey_handler';