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
1 change: 1 addition & 0 deletions src/plugins/vis_type_timelion/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface TimelionFunctionArgsSuggestion {
export interface TimelionFunctionArgs {
name: string;
help?: string;
insertText?: string;
multi?: boolean;
types: TimelionFunctionArgsTypes[];
suggestions?: TimelionFunctionArgsSuggestion[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,9 @@ export function getSuggestion(

break;
case SUGGESTION_TYPE.ARGUMENT_VALUE:
const param = suggestion.name.split(':');

if (param.length === 1 || param[1]) {
insertText = `${param.length === 1 ? insertText : param[1]},`;
const defaultText = (suggestion as TimelionFunctionArgs).insertText;
if (defaultText) {
insertText = `${defaultText},`;
}

command = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export function getArgValueSuggestions() {

return (await indexPatterns.find(search, size)).map(({ title }) => ({
name: title,
insertText: title,
}));
},
async metric(partial: string, functionArgs: TimelionExpressionFunction[]) {
Expand Down Expand Up @@ -81,7 +82,14 @@ export function getArgValueSuggestions() {
containsFieldName(valueSplit[1], field) &&
!indexPatternsUtils.isNestedField(field)
)
.map((field) => ({ name: `${valueSplit[0]}:${field.name}`, help: field.type }));
.map((field) => {
const suggestionValue = field.name.replaceAll(':', '\\:');
return {
name: `${valueSplit[0]}:${suggestionValue}`,
help: field.type,
insertText: suggestionValue,
};
});
},
async split(partial: string, functionArgs: TimelionExpressionFunction[]) {
const indexPattern = await getIndexPattern(functionArgs);
Expand All @@ -105,7 +113,7 @@ export function getArgValueSuggestions() {
containsFieldName(partial, field) &&
!indexPatternsUtils.isNestedField(field)
)
.map((field) => ({ name: field.name, help: field.type }));
.map((field) => ({ name: field.name, help: field.type, insertText: field.name }));
},
async timefield(partial: string, functionArgs: TimelionExpressionFunction[]) {
const indexPattern = await getIndexPattern(functionArgs);
Expand All @@ -121,7 +129,7 @@ export function getArgValueSuggestions() {
containsFieldName(partial, field) &&
!indexPatternsUtils.isNestedField(field)
)
.map((field) => ({ name: field.name }));
.map((field) => ({ name: field.name, insertText: field.name }));
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,35 @@ 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.2,1.3,2.7',
'percentiles:\\:bytes\\:123:20.0,50.0,100.0',
'percentiles:a:2',
];
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.2, 1.3, 2.7] },
});
expect(agg.time_buckets.aggs['percentiles(:bytes:123)']).toEqual({
percentiles: { field: ':bytes:123', percents: [20.0, 50.0, 100.0] },
});
expect(agg.time_buckets.aggs['percentiles(a)']).toEqual({
percentiles: { field: 'a', percents: [2] },
});
});

test('adds a scripted metric agg for each scripted metric', () => {
Expand Down Expand Up @@ -158,6 +180,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 Expand Up @@ -305,10 +334,10 @@ describe('es', () => {

describe('config.split', () => {
test('adds terms aggs, in order, under the filters agg', () => {
config.split = ['beer:5', 'wine:10'];
config.split = ['beer:5', 'wine:10', ':lemo:nade::15', ':jui:ce:723::45'];
const request = fn(config, tlConfig, emptyScriptedFields);

const aggs = request.params.body.aggs.q.aggs;
let aggs = request.params.body.aggs.q.aggs;

expect(aggs.beer.meta.type).toEqual('split');
expect(aggs.beer.terms.field).toEqual('beer');
Expand All @@ -317,6 +346,18 @@ describe('es', () => {
expect(aggs.beer.aggs.wine.meta.type).toEqual('split');
expect(aggs.beer.aggs.wine.terms.field).toEqual('wine');
expect(aggs.beer.aggs.wine.terms.size).toEqual(10);

aggs = aggs.beer.aggs.wine.aggs;
expect(aggs).toHaveProperty(':lemo:nade:');
expect(aggs[':lemo:nade:'].meta.type).toEqual('split');
expect(aggs[':lemo:nade:'].terms.field).toEqual(':lemo:nade:');
expect(aggs[':lemo:nade:'].terms.size).toEqual(15);

aggs = aggs[':lemo:nade:'].aggs;
expect(aggs).toHaveProperty(':jui:ce:723:');
expect(aggs[':jui:ce:723:'].meta.type).toEqual('split');
expect(aggs[':jui:ce:723:'].terms.field).toEqual(':jui:ce:723:');
expect(aggs[':jui:ce:723:'].terms.size).toEqual(45);
});

test('adds scripted terms aggs, in order, under the filters agg', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ export default function buildRequest(config, tlConfig, scriptedFields, timeout)

let aggCursor = aggs.q.aggs;

_.each(config.split, function (clause) {
clause = clause.split(':');
if (clause[0] && clause[1]) {
const termsAgg = buildAggBody(clause[0], scriptedFields);
termsAgg.size = parseInt(clause[1], 10);
aggCursor[clause[0]] = {
(config.split || []).forEach((clause) => {
const [field, arg] = clause.split(/:(\d+$)/);
if (field && arg) {
const termsAgg = buildAggBody(field, scriptedFields);
termsAgg.size = parseInt(arg, 10);
aggCursor[field] = {
meta: { type: 'split' },
terms: termsAgg,
aggs: {},
};
aggCursor = aggCursor[clause[0]].aggs;
aggCursor = aggCursor[field].aggs;
} else {
throw new Error('`split` requires field:limit');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Side Public License, v 1.
*/

import _ from 'lodash';
import { buildAggBody } from './agg_body';
import { search } from '../../../../../../plugins/data/server';
import { search, METRIC_TYPES } from '../../../../../data/server';

const { dateHistogramInterval } = search.aggs;

export default function createDateAgg(config, tlConfig, scriptedFields) {
Expand All @@ -29,29 +29,39 @@ 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 === METRIC_TYPES.COUNT) {
// 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 splittedArgs = metricArgs.split(/(.*[^\\]):/).filter(Boolean);
const field = splittedArgs[0].replace(/\\:/g, ':');
const percentArgs = splittedArgs[1];
const metricKey = metricName + '(' + field + ')';

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

if (metricName === METRIC_TYPES.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