Skip to content

Commit

Permalink
[7.x] [Metrics UI] Fix metric threshold preview regression (#107674) (#…
Browse files Browse the repository at this point in the history 
…107814)

* [Metrics UI] Fix metric threshold preview regression (#107674)

# Conflicts:
#	x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts
#	x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts

* Fix bad merge
  • Loading branch information
@Zacqary
Zacqary authored Aug 6, 2021
1 parent d089b3c commit 1743212
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 33 deletions.
64 changes: 40 additions & 24 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/evaluate_alert.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { mapValues, first, last, isNaN } from 'lodash';
import moment from 'moment';
import { ElasticsearchClient } from 'kibana/server';
import {
isTooManyBucketsPreviewException,
Expand Down Expand Up @@ -95,8 +96,6 @@ export const evaluateAlert = <Params extends EvaluatedAlertParams = EvaluatedAle
);
};

const MINIMUM_BUCKETS = 5;

const getMetric: (
esClient: ElasticsearchClient,
params: MetricExpressionParams,
Expand All @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down
26 changes: 26 additions & 0 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
13 changes: 8 additions & 5 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/lib/metric_query.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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');
});
Expand All @@ -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);
});
});
Expand Down

0 comments on commit 1743212

Please sign in to comment.