[TSVB] Group by "Everything" fails to calculate data with "Overall" metrics (#42074) (#42189)

* [TSVB] Group by "Everything" fails to calculate data with "Overall" metrics

Closes: #42015

* add tests

* fix grammar

* hasPipelineAggregations -> hasSiblingPipelineAggregation
This commit is contained in:
Alexey Antonov 2019-07-30 00:06:39 +03:00 committed by GitHub
parent 173d4848b8
commit 8b1245c3ae
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 274 additions and 2 deletions

View file

@ -19,16 +19,20 @@
const { set, get, isEmpty } = require('lodash');
const isEmptyFilter = (filter = {}) => Boolean(filter.match_all) && isEmpty(filter.match_all);
const hasSiblingPipelineAggregation = (aggs = {}) => Object.keys(aggs).length > 1;
/* For grouping by the 'Everything', the splitByEverything request processor
* creates fake .filter.match_all filter (see split_by_everything.js) to simplify the request processors code.
* But filters are not supported by all of available search strategies (e.g. Rollup search).
* This method removes that aggregation.
*
* Important: for Sibling Pipeline aggregation we cannot apply this logic
*
*/
function removeEmptyTopLevelAggregation(doc, series) {
const filter = get(doc, `aggs.${series.id}.filter`);
if (isEmptyFilter(filter)) {
if (isEmptyFilter(filter) && !hasSiblingPipelineAggregation(doc.aggs[series.id].aggs)) {
const meta = get(doc, `aggs.${series.id}.meta`);
set(doc, `aggs`, doc.aggs[series.id].aggs);
set(doc, `aggs.timeseries.meta`, meta);

View file

@ -0,0 +1,129 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { normalizeQuery } from './normalize_query';
describe('normalizeQuery', () => {
const req = 'req';
const seriesId = '61ca57f1-469d-11e7-af02-69e470af7417';
let next;
let panel;
let series;
const getMockedDoc = () => ({
size: 0,
query: {},
aggs: {
[seriesId]: {
aggs: {
timeseries: {
date_histogram: {
field: 'order_date',
fixed_interval: '10s',
},
aggs: {
[seriesId]: {
bucket_script: {
buckets_path: {
count: '_count',
},
script: {
source: 'count * 1',
lang: 'expression',
},
gap_policy: 'skip',
},
},
},
},
},
meta: {
timeField: 'order_date',
intervalString: '10s',
bucketSize: 10,
seriesId: [seriesId],
},
},
},
});
beforeEach(() => {
next = jest.fn(x => x);
panel = {};
series = {
id: seriesId,
};
});
test('should remove the top level aggregation if filter.match_all is empty', () => {
const doc = getMockedDoc();
doc.aggs[seriesId].filter = {
match_all: {},
};
const modifiedDoc = normalizeQuery(req, panel, series)(next)(doc);
expect(modifiedDoc.aggs[seriesId]).toBeUndefined();
expect(modifiedDoc.aggs.timeseries).toBeDefined();
expect(modifiedDoc.aggs.timeseries.meta).toEqual({
timeField: 'order_date',
intervalString: '10s',
bucketSize: 10,
seriesId: [seriesId],
});
});
test('should not remove the top level aggregation if filter.match_all is not empty', () => {
const doc = getMockedDoc();
doc.aggs[seriesId].filter = {
match_all: { filter: 1 },
};
const modifiedDoc = normalizeQuery(req, panel, series)(next)(doc);
expect(modifiedDoc.aggs[seriesId]).toBeDefined();
expect(modifiedDoc.aggs[seriesId].aggs.timeseries).toBeDefined();
expect(modifiedDoc.aggs.timeseries).toBeUndefined();
});
test('should not remove the top level aggregation for Sibling Pipeline queries', () => {
const doc = getMockedDoc();
const pipelineId = 'd4167fe0-afb0-11e9-b141-7b94c69f37eb';
doc.aggs[seriesId].filter = {
match_all: {},
};
doc.aggs[seriesId].aggs[pipelineId] = {
extended_stats_bucket: {
buckets_path: 'timeseries>61ca57f2-469d-11e7-af02-69e470af7417',
},
};
const modifiedDoc = normalizeQuery(req, panel, series)(next)(doc);
expect(modifiedDoc.aggs[seriesId]).toBeDefined();
expect(modifiedDoc.aggs[seriesId].aggs.timeseries).toBeDefined();
expect(modifiedDoc.aggs[seriesId].aggs[pipelineId]).toBeDefined();
expect(modifiedDoc.aggs.timeseries).toBeUndefined();
});
});

View file

@ -19,9 +19,13 @@
const { set, get, isEmpty, forEach } = require('lodash');
const isEmptyFilter = (filter = {}) => Boolean(filter.match_all) && isEmpty(filter.match_all);
const hasSiblingPipelineAggregation = (aggs = {}) => Object.keys(aggs).length > 1;
/* Last query handler in the chain. You can use this handler
* as the last place where you can modify the "doc" (request body) object before sending it to ES.
* Important: for Sibling Pipeline aggregation we cannot apply this logic
*
*/
export function normalizeQuery() {
return () => doc => {
@ -31,7 +35,7 @@ export function normalizeQuery() {
forEach(series, (value, seriesId) => {
const filter = get(value, `filter`);
if (isEmptyFilter(filter)) {
if (isEmptyFilter(filter) && !hasSiblingPipelineAggregation(value.aggs)) {
const agg = get(value, 'aggs.timeseries');
const meta = {
...get(value, 'meta'),

View file

@ -0,0 +1,135 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { normalizeQuery } from './normalize_query';
describe('normalizeQuery', () => {
const req = 'req';
const seriesId = '61ca57f1-469d-11e7-af02-69e470af7417';
let next;
let panel;
let series;
const getMockedDoc = () => ({
size: 0,
query: {},
aggs: {
pivot: {
terms: {
field: 'currency',
},
aggs: {
[seriesId]: {
aggs: {
timeseries: {
date_histogram: {
field: 'order_date',
extended_bounds: {
min: 1564397420526,
max: 1564398320526,
},
fixed_interval: '10s',
},
aggs: {
[seriesId]: {
bucket_script: {
buckets_path: {
count: '_count',
},
script: {
source: 'count * 1',
lang: 'expression',
},
gap_policy: 'skip',
},
},
},
},
},
meta: {
timeField: 'order_date',
intervalString: '10s',
bucketSize: 10,
},
},
},
},
},
});
beforeEach(() => {
next = jest.fn(x => x);
panel = {};
series = {
id: seriesId,
};
});
test('should remove the top level aggregation if filter.match_all is empty', () => {
const doc = getMockedDoc();
doc.aggs.pivot.aggs[seriesId].filter = {
match_all: {},
};
const modifiedDoc = normalizeQuery(req, panel, series)(next)(doc);
expect(modifiedDoc.aggs.pivot.aggs[seriesId].aggs.timeseries).toBeUndefined();
expect(modifiedDoc.aggs.pivot.aggs[seriesId].aggs[seriesId]).toBeDefined();
expect(modifiedDoc.aggs.pivot.aggs[seriesId].meta).toEqual({
seriesId,
timeField: 'order_date',
intervalString: '10s',
bucketSize: 10,
});
});
test('should not remove the top level aggregation if filter.match_all is not empty', () => {
const doc = getMockedDoc();
doc.aggs.pivot.aggs[seriesId].filter = {
match_all: { filter: 1 },
};
const modifiedDoc = normalizeQuery(req, panel, series)(next)(doc);
expect(modifiedDoc.aggs.pivot.aggs[seriesId].aggs.timeseries).toBeDefined();
expect(modifiedDoc.aggs.pivot.aggs[seriesId].aggs[seriesId]).toBeUndefined();
});
test('should not remove the top level aggregation for Sibling Pipeline queries', () => {
const doc = getMockedDoc();
const pipelineId = 'd4167fe0-afb0-11e9-b141-7b94c69f37eb';
doc.aggs.pivot.aggs[seriesId].filter = {
match_all: {},
};
doc.aggs.pivot.aggs[seriesId].aggs[pipelineId] = {
extended_stats_bucket: {
buckets_path: 'timeseries>61ca57f2-469d-11e7-af02-69e470af7417',
},
};
const modifiedDoc = normalizeQuery(req, panel, series)(next)(doc);
expect(modifiedDoc.aggs.pivot.aggs[seriesId].aggs.timeseries).toBeDefined();
expect(modifiedDoc.aggs.pivot.aggs[seriesId].aggs[seriesId]).toBeUndefined();
});
});