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

dimaanj · 2021-04-12T10:46:45Z

Summary

added support for colons in field names for metric
for metric with field name :te:st:1

for split with field name :te:st1:

kibanamachine · 2021-04-12T10:46:47Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

alexwizp · 2021-04-12T15:11:29Z

src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js

@@ -30,7 +31,7 @@ export default function createDateAgg(config, tlConfig, scriptedFields) {

  dateAgg.time_buckets.aggs = {};
  _.each(config.metric, function (metric) {
-    metric = metric.split(':');
+    metric = metric.split(/:(.+)/).slice(0, OPERANDS_NUMBER);


Please check the 'percentiles' aggregation. I see that in code below that we are getting third item from the metric array.

alexwizp · 2021-04-13T12:15:08Z

src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js

-      dateAgg.time_buckets.aggs[metricName][metric[0]] = buildAggBody(metric[1], scriptedFields);
-      if (metric[0] === 'percentiles' && metric[2]) {
-        let percentList = metric[2].split(',');
+      dateAgg.time_buckets.aggs[metricName][metric[0]] = buildAggBody(field, scriptedFields);


what if we do it a little bit obvious. I see we are dealing with 3 cases.

const [metricName, metricArgs] = metric.split(/:(.+)/); if (metricName === 'count') { } else if (metricName === 'percentiles') { } else if (metricName, metricArgs){ }

oh... sorry, I see we need

dateAgg.time_buckets.aggs[metricName] = {}; dateAgg.time_buckets.aggs[metricName][metric[0]] = buildAggBody(field, scriptedFields);

for percentiles too.

Now, it's cleaned up from array indexes at least

stratoula · 2021-04-15T08:34:52Z

@elasticmachine merge upstream

stratoula · 2021-04-15T08:36:02Z

Jenkins test this

stratoula · 2021-04-15T08:36:38Z

src/plugins/vis_type_timelion/server/series_functions/es/es.test.js

+          'cardinality::sample',
+          'sum::beer',
+          'percentiles::bytes:1',
+          'percentiles:::bytes:1.2,1.3,2.7',


Nice ❤️

stratoula · 2021-04-15T08:39:02Z

src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js

+  (config.metric || []).forEach((metric) => {
+    const metricBody = {};
+    const [metricName, metricArgs] = metric.split(/:(.+)/);
+    if (metricName === 'count') {


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?

alexwizp · 2021-04-15T08:48:55Z

needs more work, suggestions are not working fine with fields started from :

stratoula · 2021-04-15T08:49:54Z

Yes and split also doesn't work

stratoula · 2021-04-19T07:45:49Z

@elasticmachine merge upstream

elasticmachine · 2021-04-19T07:48:00Z

Pinging @elastic/kibana-app (Team:KibanaApp)

stratoula

Thank you Dima, it seems to work fine now 👏 LGTM

@alexwizp can you also check it again?

alexwizp · 2021-04-19T10:25:55Z

src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js

 import { buildAggBody } from './agg_body';
 import { search } from '../../../../../../plugins/data/server';
+import { METRIC_TYPES } from '../../../../../data/common/search/aggs/metrics/index';


wrong import. Please replace to '../../../../../data/server'

stratoula · 2021-04-19T13:23:07Z

src/plugins/vis_type_timelion/server/series_functions/es/lib/create_date_agg.js

+    const metricBody = {};
+    const [metricName, metricArgs] = metric.split(/:(.+)/);
+    console.log('METRIC_TYPES.COUNT', METRIC_TYPES.COUNT);
+    console.log('METRIC_TYPES.PERCENTILES', METRIC_TYPES.PERCENTILES);


@dmitriynj please remove the console.logs :D

Sorry, done

stratoula · 2021-04-20T07:15:02Z

Jenkins test this

alexwizp

LGTM! Thank you

kibanamachine · 2021-04-20T09:18:08Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`visTypeTimelion`	69.3KB	69.2KB	-43.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeTimelion`	24.2KB	24.4KB	+154.0B

History

💚 Build #119900 succeeded 16b1b51

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @dmitriynj

…split parameter (elastic#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>

…split parameter (#96770) (#97606) * [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> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

samuelkampa · 2021-04-20T15:07:18Z

@dmitriynj , @alexwizp , & @stratoula : Thanks for all your hard work on this! Our team is stoked about the change. Can't wait for the next release!

[Timelion] add colons support in field names for metric

8e49305

alexwizp requested review from alexwizp and stratoula April 12, 2021 10:48

alexwizp added Feature:Timelion Timelion app and visualization v7.13.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 labels Apr 12, 2021

alexwizp reviewed Apr 12, 2021

View reviewed changes

[Timelion] support percent for metric

fd0eac0

alexwizp reviewed Apr 13, 2021

View reviewed changes

dimaanj added 2 commits April 13, 2021 16:44

[Timelion] get rid of array indexes

07377d1

[Timelion] get rid of lodash methods

8a4a472

alexwizp self-requested a review April 15, 2021 08:02

Merge branch 'master' into escape-colons-in-metric

16b1b51

stratoula reviewed Apr 15, 2021

View reviewed changes

dimaanj added 4 commits April 15, 2021 21:51

[Timelion] support colons in split. fix expression suggestions

f447ded

[Timelion] escape colons for metric

1d1ba2f

[Timelion] use metric types common constants

bf68a07

[Timelion] support one symbol field name

671bfae

stratoula added the release_note:fix label Apr 19, 2021

Merge branch 'master' into escape-colons-in-metric

78b5eea

stratoula marked this pull request as ready for review April 19, 2021 07:47

stratoula requested a review from a team April 19, 2021 07:47

kibanamachine added the 💝community label Apr 19, 2021

stratoula changed the title ~~[Timelion] Cannot escape colons in field names in the metric parameter~~ [Timelion] Fixes bug with escape colons in field names in the metric/split parameter Apr 19, 2021

stratoula approved these changes Apr 19, 2021

View reviewed changes

dimaanj self-assigned this Apr 19, 2021

alexwizp reviewed Apr 19, 2021

View reviewed changes

[Timelion] resolve duplicate imports

53bdf8d

stratoula reviewed Apr 19, 2021

View reviewed changes

[Timelion] remove console logs

dd44a37

alexwizp approved these changes Apr 20, 2021

View reviewed changes

alexwizp removed the 💝community label Apr 20, 2021

dimaanj merged commit 4faff46 into elastic:master Apr 20, 2021

dimaanj mentioned this pull request Apr 20, 2021

[7.x] [Timelion] Fixes bug with escape colons in field names in the metric/split parameter (#96770) #97606

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dimaanj commented Apr 12, 2021 •

edited

Loading

kibanamachine commented Apr 12, 2021

alexwizp Apr 12, 2021 •

edited

Loading

dimaanj Apr 13, 2021

alexwizp Apr 13, 2021

alexwizp Apr 13, 2021

dimaanj Apr 13, 2021

stratoula commented Apr 15, 2021

stratoula commented Apr 15, 2021

stratoula Apr 15, 2021

stratoula Apr 15, 2021

dimaanj Apr 16, 2021

alexwizp commented Apr 15, 2021

stratoula commented Apr 15, 2021 •

edited

Loading

stratoula commented Apr 19, 2021

elasticmachine commented Apr 19, 2021

stratoula left a comment

alexwizp Apr 19, 2021

dimaanj Apr 19, 2021

stratoula Apr 19, 2021

dimaanj Apr 19, 2021

stratoula commented Apr 20, 2021

alexwizp left a comment

kibanamachine commented Apr 20, 2021

samuelkampa commented Apr 20, 2021

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

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

Conversation

dimaanj commented Apr 12, 2021 • edited Loading

Summary

kibanamachine commented Apr 12, 2021

alexwizp Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stratoula commented Apr 15, 2021

stratoula commented Apr 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexwizp commented Apr 15, 2021

stratoula commented Apr 15, 2021 • edited Loading

stratoula commented Apr 19, 2021

elasticmachine commented Apr 19, 2021

stratoula left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stratoula commented Apr 20, 2021

alexwizp left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 20, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

samuelkampa commented Apr 20, 2021

dimaanj commented Apr 12, 2021 •

edited

Loading

alexwizp Apr 12, 2021 •

edited

Loading

stratoula commented Apr 15, 2021 •

edited

Loading