Skip to content

Commit

Permalink
[Metrics UI] Fix metric threshold preview regression (#107674)
Browse files Browse the repository at this point in the history
  • Loading branch information
Zacqary authored Aug 5, 2021
1 parent 74107a1 commit e991326
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ export const evaluateAlert = <Params extends EvaluatedAlertParams = EvaluatedAle
);
};

const MINIMUM_BUCKETS = 5;

const getMetric: (
esClient: ElasticsearchClient,
params: MetricExpressionParams,
Expand Down Expand Up @@ -127,10 +125,10 @@ const getMetric: (
.startOf(timeUnit)
.valueOf();

// We need enough data for 5 buckets worth of data. We also need
// to convert the intervalAsSeconds to milliseconds.
// TODO: We only need to get 5 buckets for the rate query, so this logic should move there.
const minimumFrom = to - intervalAsMS * MINIMUM_BUCKETS;
// 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
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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,13 @@ export const getElasticsearchMetricQuery = (
aggregatedIntervals: {
date_range: {
field: timefield,
ranges: [
{
from: to - intervalAsMS - deliveryDelay,
to: to - deliveryDelay,
},
],
// 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 - deliveryDelay,
to: from + intervalAsMS * (i + 1) - deliveryDelay,
})),
},
aggregations,
},
Expand Down

0 comments on commit e991326

Please sign in to comment.