[Metrics Alerts] Remove metric field from doc count on backend (#60679)

* Remove metric field from doc count on backend * Fix tests * Type fix Co-authored-by: Elastic Machine <[email protected]>
elastic · Mar 23, 2020 · fce8007 · fce8007
1 parent fbe3637
commit fce8007
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 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
@@ -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
@@ -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/...lugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts b/...lugins/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
@@ -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<MetricsExplorerAggregation, 'count'>;
+  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
@@ -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}}'
         );