Skip to content

Commit

Permalink
resolves elastic#58905
Browse files Browse the repository at this point in the history
The current index threshold alert uses a `size` limit on term aggregation, when used, but does not sort the buckets, so it's just using descending count on the grouped buckets as the sort to determine what to return.

The watcher API for the index threshold notes this as "top N of", implying a sort.

This PR applies sorting when the using `groupBy: top`, and the `aggType != count`.  For count, ES is already sorting the way we want.

The sort is calculated as a separate agg beside the date_range aggregation, which is the same metrics agg specified in the query - `aggType(aggField)`.  This field is then referenced in a new `order` property in the terms agg, using 'asc' sorting for `min`, and `desc` sorting for `avg`, `max`, and `sum`.

This doesn't change the shape of the output at all, just changes which term buckets will be returned, if there are more term buckets than requested with the `termSize` parameter.
  • Loading branch information
pmuellr committed Mar 16, 2020
1 parent 6a64865 commit 840dbb2
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ export async function timeSeriesQuery(
},
},
};

// if not count add an order
if (!isCountAgg) {
const sortOrder = aggType === 'min' ? 'asc' : 'desc';
aggParent.aggs.groupAgg.terms.order = {
sortValueAgg: sortOrder,
};
}

aggParent = aggParent.aggs.groupAgg;
}

Expand All @@ -89,6 +98,16 @@ export async function timeSeriesQuery(
},
},
};

// if not count, add a sorted value agg
if (!isCountAgg) {
aggParent.aggs.sortValueAgg = {
[aggType]: {
field: aggField,
},
};
}

aggParent = aggParent.aggs.dateAgg;

// finally, the metric aggregation, if requested
Expand All @@ -106,13 +125,20 @@ export async function timeSeriesQuery(
const logPrefix = 'indexThreshold timeSeriesQuery: callCluster';
logger.debug(`${logPrefix} call: ${JSON.stringify(esQuery)}`);

// note there are some commented out console.log()'s below, which are left
// in, as they are VERY useful when debugging these queries; debug logging
// isn't as nice since it's a single long JSON line.

// console.log('time_series_query.ts request\n', JSON.stringify(esQuery, null, 4));
try {
esResult = await callCluster('search', esQuery);
} catch (err) {
// console.log('time_series_query.ts error\n', JSON.stringify(err, null, 4));
logger.warn(`${logPrefix} error: ${JSON.stringify(err.message)}`);
throw new Error('error running search');
}

// console.log('time_series_query.ts response\n', JSON.stringify(esResult, null, 4));
logger.debug(`${logPrefix} result: ${JSON.stringify(esResult)}`);
return getResultFromEs(isCountAgg, isGroupAgg, esResult);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,6 @@ export default function timeSeriesQueryEndpointTests({ getService }: FtrProvider

const expected = {
results: [
{
group: 'groupA',
metrics: [
[START_DATE_MINUS_2INTERVALS, 4 / 1],
[START_DATE_MINUS_1INTERVALS, (4 + 2) / 2],
[START_DATE_MINUS_0INTERVALS, (4 + 2 + 1) / 3],
],
},
{
group: 'groupB',
metrics: [
Expand All @@ -212,12 +204,50 @@ export default function timeSeriesQueryEndpointTests({ getService }: FtrProvider
[START_DATE_MINUS_0INTERVALS, (5 + 3 + 2) / 3],
],
},
{
group: 'groupA',
metrics: [
[START_DATE_MINUS_2INTERVALS, 4 / 1],
[START_DATE_MINUS_1INTERVALS, (4 + 2) / 2],
[START_DATE_MINUS_0INTERVALS, (4 + 2 + 1) / 3],
],
},
],
};

expect(await runQueryExpect(query, 200)).eql(expected);
});

it('should return correct sorted group for average', async () => {
const query = getQueryBody({
aggType: 'avg',
aggField: 'testedValue',
groupBy: 'top',
termField: 'group',
termSize: 1,
dateStart: START_DATE_MINUS_2INTERVALS,
dateEnd: START_DATE_MINUS_0INTERVALS,
});
const result = await runQueryExpect(query, 200);
expect(result.results.length).to.be(1);
expect(result.results[0].group).to.be('groupB');
});

it('should return correct sorted group for min', async () => {
const query = getQueryBody({
aggType: 'min',
aggField: 'testedValue',
groupBy: 'top',
termField: 'group',
termSize: 1,
dateStart: START_DATE_MINUS_2INTERVALS,
dateEnd: START_DATE_MINUS_0INTERVALS,
});
const result = await runQueryExpect(query, 200);
expect(result.results.length).to.be(1);
expect(result.results[0].group).to.be('groupA');
});

it('should return an error when passed invalid input', async () => {
const query = { ...getQueryBody(), aggType: 'invalid-agg-type' };
const expected = {
Expand Down

0 comments on commit 840dbb2

Please sign in to comment.