[Timelion] Fixes bug with escape colons in field names in the metric/…

…split parameter (#96770) * [Timelion] add colons support in field names for metric * [Timelion] support percent for metric * [Timelion] get rid of array indexes * [Timelion] get rid of lodash methods * [Timelion] support colons in split. fix expression suggestions * [Timelion] escape colons for metric * [Timelion] use metric types common constants * [Timelion] support one symbol field name * [Timelion] resolve duplicate imports * [Timelion] remove console logs Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Apr 20, 2021 · 4faff46 · 4faff46
1 parent b227b8f
commit 4faff46
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 30 deletions.
diff --git a/src/plugins/vis_type_timelion/common/types.ts b/src/plugins/vis_type_timelion/common/types.ts
@@ -16,6 +16,7 @@ export interface TimelionFunctionArgsSuggestion {
 export interface TimelionFunctionArgs {
   name: string;
   help?: string;
+  insertText?: string;
   multi?: boolean;
   types: TimelionFunctionArgsTypes[];
   suggestions?: TimelionFunctionArgsSuggestion[];

diff --git a/src/plugins/vis_type_timelion/public/components/timelion_expression_input_helpers.ts b/src/plugins/vis_type_timelion/public/components/timelion_expression_input_helpers.ts
@@ -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 = {

diff --git a/src/plugins/vis_type_timelion/public/helpers/arg_value_suggestions.ts b/src/plugins/vis_type_timelion/public/helpers/arg_value_suggestions.ts
@@ -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[]) {
@@ -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);
@@ -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);
@@ -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 }));
       },
     },
   };

diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js b/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js
@@ -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', () => {
@@ -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');
+      });
     });
   });
 
@@ -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');
@@ -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', () => {

diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/lib/build_request.js b/src/plugins/vis_type_timelion/server/series_functions/es/lib/build_request.js
@@ -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');
     }

diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js b/src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js
@@ -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) {
@@ -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;