Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Timelion] Fixes bug with escape colons in field names in the metric/split parameter #96770

Merged
merged 12 commits into from
Apr 20, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,31 @@ describe('es', () => {
const emptyScriptedFields = [];

test('adds a metric agg for each metric', () => {
config.metric = ['sum:beer', 'avg:bytes', 'percentiles:bytes'];
config.metric = [
'sum:beer',
'avg:bytes',
'percentiles:bytes',
'cardinality::sample',
'sum::beer',
'percentiles::bytes:1',
'percentiles:::bytes:1.2,1.3,2.7',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice ❤️

];
agg = createDateAgg(config, tlConfig, emptyScriptedFields);
expect(agg.time_buckets.aggs['sum(beer)']).toEqual({ sum: { field: 'beer' } });
expect(agg.time_buckets.aggs['avg(bytes)']).toEqual({ avg: { field: 'bytes' } });
expect(agg.time_buckets.aggs['percentiles(bytes)']).toEqual({
percentiles: { field: 'bytes' },
});
expect(agg.time_buckets.aggs['cardinality(:sample)']).toEqual({
cardinality: { field: ':sample' },
});
expect(agg.time_buckets.aggs['sum(:beer)']).toEqual({ sum: { field: ':beer' } });
expect(agg.time_buckets.aggs['percentiles(:bytes)']).toEqual({
percentiles: { field: ':bytes', percents: [1] },
});
expect(agg.time_buckets.aggs['percentiles(::bytes)']).toEqual({
percentiles: { field: '::bytes', percents: [1.2, 1.3, 2.7] },
});
});

test('adds a scripted metric agg for each scripted metric', () => {
Expand Down Expand Up @@ -158,6 +176,13 @@ describe('es', () => {
expect(typeof agg.time_buckets.aggs.count.bucket_script).toBe('object');
expect(agg.time_buckets.aggs.count.bucket_script.buckets_path).toEqual('_count');
});

test('has a special `count` metric with redundant field which use a script', () => {
config.metric = ['count:beer'];
agg = createDateAgg(config, tlConfig, emptyScriptedFields);
expect(typeof agg.time_buckets.aggs.count.bucket_script).toBe('object');
expect(agg.time_buckets.aggs.count.bucket_script.buckets_path).toEqual('_count');
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import _ from 'lodash';
import { buildAggBody } from './agg_body';
import { search } from '../../../../../../plugins/data/server';
const { dateHistogramInterval } = search.aggs;
Expand All @@ -29,29 +28,37 @@ export default function createDateAgg(config, tlConfig, scriptedFields) {
};

dateAgg.time_buckets.aggs = {};
_.each(config.metric, function (metric) {
metric = metric.split(':');
if (metric[0] === 'count') {
(config.metric || []).forEach((metric) => {
const metricBody = {};
const [metricName, metricArgs] = metric.split(/:(.+)/);
if (metricName === 'count') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this is how it was before but do you want to use instead of count and percentiles the METRIC_TYPES.COUNT and METRIC_TYPES.PERCENTILES from the data plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

// This is pretty lame, but its how the "doc_count" metric has to be implemented at the moment
// It simplifies the aggregation tree walking code considerably
dateAgg.time_buckets.aggs[metric] = {
metricBody[metricName] = {
bucket_script: {
buckets_path: '_count',
script: { source: '_value', lang: 'expression' },
},
};
} else if (metric[0] && metric[1]) {
const metricName = metric[0] + '(' + metric[1] + ')';
dateAgg.time_buckets.aggs[metricName] = {};
dateAgg.time_buckets.aggs[metricName][metric[0]] = buildAggBody(metric[1], scriptedFields);
if (metric[0] === 'percentiles' && metric[2]) {
let percentList = metric[2].split(',');
} else if (metricName && metricArgs) {
const [field, percentArgs] = metricArgs.split(/:(\d.*)/);
const metricKey = metricName + '(' + field + ')';

metricBody[metricKey] = { [metricName]: buildAggBody(field, scriptedFields) };

if (metricName === 'percentiles' && percentArgs) {
let percentList = percentArgs.split(',');
percentList = percentList.map((x) => parseFloat(x));
dateAgg.time_buckets.aggs[metricName][metric[0]].percents = percentList;
metricBody[metricKey][metricName].percents = percentList;
}
} else {
throw new Error('`metric` requires metric:field or simply count');
}

dateAgg.time_buckets.aggs = {
...dateAgg.time_buckets.aggs,
...metricBody,
};
});

return dateAgg;
Expand Down