Remove all offset logic for date_range aggs

elastic · Jul 28, 2021 · 6087cfe · 6087cfe
1 parent 5616385
commit 6087cfe
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 36 deletions.
diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts
@@ -235,29 +235,30 @@ const getValuesFromAggregations = (
       }));
     }
     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 };
-          })
-          // P95 & P99 can only be calculated accurately on full buckets
-          .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 };
+      });
     }
 
     if (aggType === Aggregators.AVERAGE) {
-      return (
-        buckets
-          .map((bucket) => ({
-            key: bucket.key_as_string ?? bucket.from_as_string,
-            value: bucket.aggregatedValue?.value ?? null,
-          }))
-          // AVG can only be calculated accurately on full buckets
-          .filter(dropPartialBuckets(dropPartialBucketsOptions))
-      );
+      return buckets.map((bucket) => ({
+        key: bucket.key_as_string ?? bucket.from_as_string,
+        value: bucket.aggregatedValue?.value ?? null,
+      }));
+      // AVG can only be calculated accurately on full buckets
+      // .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) => ({

diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts
@@ -38,17 +38,6 @@ export const getElasticsearchMetricQuery = (
   const intervalAsMS = intervalAsSeconds * 1000;
   const to = timeframe.end;
   const from = timeframe.start;
-  let offset = '0ms';
-  let offsetInMS = 0;
-
-  if (
-    [Aggregators.P95, Aggregators.P99, Aggregators.AVERAGE, Aggregators.RATE].indexOf(aggType) > -1
-  ) {
-    // Only calculate offset for aggs that require a full bucket to evaluate
-    offset = calculateDateHistogramOffset({ from, to, interval, field: timefield });
-  }
-
-  offsetInMS = parseInt(offset, 10);
 
   const aggregations =
     aggType === Aggregators.COUNT
@@ -72,7 +61,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,
@@ -86,8 +75,8 @@ export const getElasticsearchMetricQuery = (
             date_range: {
               field: timefield,
               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,
+                to: from + intervalAsMS * (i + 1),
               })),
             },
             aggregations,

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
@@ -202,7 +202,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 () => {
@@ -221,7 +221,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');
     });