[7.x] [Metrics UI] Fix metric threshold preview regression (#107674)

x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts

@@ -6,6 +6,7 @@
  */
 
 import { mapValues, first, last, isNaN } from 'lodash';
+import moment from 'moment';
 import { ElasticsearchClient } from 'kibana/server';
 import {
   isTooManyBucketsPreviewException,
@@ -95,8 +96,6 @@ export const evaluateAlert = <Params extends EvaluatedAlertParams = EvaluatedAle
   );
 };
 
-const MINIMUM_BUCKETS = 5;
-
 const getMetric: (
   esClient: ElasticsearchClient,
   params: MetricExpressionParams,
@@ -121,10 +120,15 @@ const getMetric: (
   const intervalAsSeconds = getIntervalInSeconds(interval);
   const intervalAsMS = intervalAsSeconds * 1000;
 
-  const to = roundTimestamp(timeframe ? timeframe.end : Date.now(), timeUnit);
-  // We need enough data for 5 buckets worth of data. We also need
-  // to convert the intervalAsSeconds to milliseconds.
-  const minimumFrom = to - intervalAsMS * MINIMUM_BUCKETS;
+  const to = moment(timeframe ? timeframe.end : Date.now())
+    .add(1, timeUnit)
+    .startOf(timeUnit)
+    .valueOf();
+
+  // Rate aggregations need 5 buckets worth of data
+  const minimumBuckets = aggType === Aggregators.RATE ? 5 : 1;
+
+  const minimumFrom = to - intervalAsMS * minimumBuckets;
 
   const from = roundTimestamp(
     timeframe && timeframe.start <= minimumFrom ? timeframe.start : minimumFrom,
@@ -223,30 +227,42 @@ const getValuesFromAggregations = (
   try {
     const { buckets } = aggregations.aggregatedIntervals;
     if (!buckets.length) return null; // No Data state
+
     if (aggType === Aggregators.COUNT) {
-      return buckets
-        .map((bucket) => ({
-          key: bucket.from_as_string,
-          value: bucket.doc_count,
-        }))
-        .filter(dropPartialBuckets(dropPartialBucketsOptions));
+      return buckets.map((bucket) => ({
+        key: bucket.from_as_string,
+        value: bucket.doc_count,
+      }));
     }
     if (aggType === Aggregators.P95 || aggType === Aggregators.P99) {
-      return buckets
-        .map((bucket) => {
-          const values = bucket.aggregatedValue?.values || [];
-          const firstValue = first(values);
-          if (!firstValue) return null;
-          return { key: bucket.from_as_string, value: firstValue.value };
-        })
-        .filter(dropPartialBuckets(dropPartialBucketsOptions));
+      return buckets.map((bucket) => {
+        const values = bucket.aggregatedValue?.values || [];
+        const firstValue = first(values);
+        if (!firstValue) return null;
+        return { key: bucket.from_as_string, value: firstValue.value };
+      });
     }
-    return buckets
-      .map((bucket) => ({
+
+    if (aggType === Aggregators.AVERAGE) {
+      return buckets.map((bucket) => ({
         key: bucket.key_as_string ?? bucket.from_as_string,
         value: bucket.aggregatedValue?.value ?? null,
-      }))
-      .filter(dropPartialBuckets(dropPartialBucketsOptions));
+      }));
+    }
+
+    if (aggType === Aggregators.RATE) {
+      return buckets
+        .map((bucket) => ({
+          key: bucket.key_as_string ?? bucket.from_as_string,
+          value: bucket.aggregatedValue?.value ?? null,
+        }))
+        .filter(dropPartialBuckets(dropPartialBucketsOptions));
+    }
+
+    return buckets.map((bucket) => ({
+      key: bucket.key_as_string ?? bucket.from_as_string,
+      value: bucket.aggregatedValue?.value ?? null,
+    }));
   } catch (e) {
     return NaN; // Error state
   }

x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts

@@ -64,4 +64,30 @@ describe("The Metric Threshold Alert's getElasticsearchMetricQuery", () => {
       );
     });
   });
+
+  describe('when passed a timeframe of 1 hour', () => {
+    const testTimeframe = {
+      start: moment().subtract(1, 'hour').valueOf(),
+      end: moment().valueOf(),
+    };
+    const searchBodyWithoutGroupBy = getElasticsearchMetricQuery(
+      expressionParams,
+      timefield,
+      testTimeframe
+    );
+    const searchBodyWithGroupBy = getElasticsearchMetricQuery(
+      expressionParams,
+      timefield,
+      testTimeframe,
+      groupBy
+    );
+    test("generates 1 hour's worth of buckets", () => {
+      // @ts-ignore
+      expect(searchBodyWithoutGroupBy.aggs.aggregatedIntervals.date_range.ranges.length).toBe(60);
+      expect(
+        // @ts-ignore
+        searchBodyWithGroupBy.aggs.groupings.aggs.aggregatedIntervals.date_range.ranges.length
+      ).toBe(60);
+    });
+  });
 });

x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts

@@ -38,8 +38,8 @@ export const getElasticsearchMetricQuery = (
   const intervalAsMS = intervalAsSeconds * 1000;
   const to = timeframe.end;
   const from = timeframe.start;
-  const offset = calculateDateHistogramOffset({ from, to, interval, field: timefield });
-  const offsetInMS = parseInt(offset, 10);
+
+  const deliveryDelay = 60 * 1000; // INFO: This allows us to account for any delay ES has in indexing the most recent data.
 
   const aggregations =
     aggType === Aggregators.COUNT
@@ -63,7 +63,7 @@ export const getElasticsearchMetricQuery = (
             date_histogram: {
               field: timefield,
               fixed_interval: interval,
-              offset,
+              offset: calculateDateHistogramOffset({ from, to, interval, field: timefield }),
               extended_bounds: {
                 min: from,
                 max: to,
@@ -76,9 +76,12 @@ export const getElasticsearchMetricQuery = (
           aggregatedIntervals: {
             date_range: {
               field: timefield,
+              // Generate an array of buckets, starting at `from` and ending at `to`
+              // This is usually only necessary for alert previews or rate aggs. Most alert evaluations
+              // will generate only one bucket from this logic.
               ranges: Array.from(Array(Math.floor((to - from) / intervalAsMS)), (_, i) => ({
-                from: from + intervalAsMS * i + offsetInMS,
-                to: from + intervalAsMS * (i + 1) + offsetInMS,
+                from: from + intervalAsMS * i - deliveryDelay,
+                to: from + intervalAsMS * (i + 1) - deliveryDelay,
               })),
             },
             aggregations,

x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts

@@ -232,7 +232,7 @@ describe('The metric threshold alert type', () => {
     });
     test('sends no alert when some, but not all, criteria cross the threshold', async () => {
       const instanceID = '*';
-      await execute(Comparator.LT_OR_EQ, [1.0], [3.0]);
+      await execute(Comparator.LT_OR_EQ, [1.0], [2.5]);
       expect(mostRecentAction(instanceID)).toBe(undefined);
     });
     test('alerts only on groups that meet all criteria when querying with a groupBy parameter', async () => {
@@ -251,7 +251,7 @@ describe('The metric threshold alert type', () => {
       expect(reasons[0]).toContain('test.metric.1');
       expect(reasons[1]).toContain('test.metric.2');
       expect(reasons[0]).toContain('current value is 1');
-      expect(reasons[1]).toContain('current value is 3.5');
+      expect(reasons[1]).toContain('current value is 3');
       expect(reasons[0]).toContain('threshold of 1');
       expect(reasons[1]).toContain('threshold of 3');
     });
@@ -274,9 +274,9 @@ describe('The metric threshold alert type', () => {
         },
       });
     test('alerts based on the doc_count value instead of the aggregatedValue', async () => {
-      await execute(Comparator.GT, [2]);
+      await execute(Comparator.GT, [0.9]);
       expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id);
-      await execute(Comparator.LT, [1.5]);
+      await execute(Comparator.LT, [0.5]);
       expect(mostRecentAction(instanceID)).toBe(undefined);
     });
   });