From aa032c559e1d7b5a0e9ade2f1f309d2c999b7479 Mon Sep 17 00:00:00 2001 From: Zacqary Adam Xeper Date: Mon, 23 Mar 2020 15:27:24 -0500 Subject: [PATCH] [Metrics Alerts] Remove metric field from doc count on backend (#60679) (#60946) * Remove metric field from doc count on backend * Fix tests * Type fix Co-authored-by: Elastic Machine Co-authored-by: Elastic Machine --- .../metric_threshold_executor.test.ts | 3 +- .../metric_threshold_executor.ts | 41 +++++++++++------ .../register_metric_threshold_alert_type.ts | 44 ++++++++++++++----- .../lib/alerting/metric_threshold/types.ts | 16 +++++-- .../apis/infra/metrics_alerting.ts | 35 +++++---------- 5 files changed, 86 insertions(+), 53 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..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; @@ -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..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 @@ -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,25 +114,32 @@ export const getElasticsearchMetricQuery = ( } : baseAggs; + const rangeFilters = [ + { + range: { + '@timestamp': { + gte: `now-${interval}`, + }, + }, + }, + ]; + + const metricFieldFilters = metric + ? [ + { + exists: { + field: metric, + }, + }, + ] + : []; + const parsedFilterQuery = getParsedFilterQuery(filterQuery); return { query: { bool: { - filter: [ - { - range: { - '@timestamp': { - gte: `now-${interval}`, - }, - }, - }, - { - exists: { - field: metric, - }, - }, - ], + filter: [...rangeFilters, ...metricFieldFilters], ...parsedFilterQuery, }, }, 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}}' );