Add Timelion percentiles aggregation support (#8953) #15154

wkruse · 2017-11-24T18:40:15Z

Elasticsearch already supports percentiles aggregation. In order to support it in Timelion we add percentiles aggregation function and optionally a list of comma separated percents to the request.

{
    "aggs" : {
        "load_time_outlier" : {
            "percentiles" : {
                "field" : "load_time",
                "percents" : [95, 99.99] 
            }
        }
    }
}

The response contains a list of buckets, each of them is a single series.

{
    ...

   "aggregations": {
      "load_time_outlier": {
         "values" : { 
            "95.0": 60,
            "99.99": 150
         }
      }
   }
}

Usage in Timelion:

.es(*,metric='percentiles:load_time:95,99.99')

Thanks to @nisxiya!

Elasticsearch: Percentiles Aggregation

Elasticsearch already supports percentiles aggregation. In order to support it in Timelion we add `percentiles` aggregation function and optionally a list of comma separated `percents` to the request. The response contains a list of buckets, each of them is a single series.

elasticmachine · 2017-11-24T18:40:16Z

Can one of the admins verify this patch?

wkruse · 2017-11-24T18:53:49Z

.es(*,metric="percentiles:bytes:50,95,99.99")

nreese

Thanks for the PR. This is really nice.

There are just a few additional files that need to be updated

arg_value_suggestions Need to add percentiles as an argument suggestion.
We should add another test case to es.js to verify that multi-value metric aggregataion results are flattened as expected.

nreese · 2017-11-28T16:41:26Z

src/core_plugins/timelion/server/series_functions/es/lib/create_date_agg.js

@@ -34,6 +34,11 @@ export default function createDateAgg(config, tlConfig, scriptedFields) {
      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(',');
+        percentList = percentList.map(x => parseFloat(x));


percents is an optional parameter of the percentiles aggregation. I think it should be optional in timelion as well. So percentiles:load_time and percentiles:load_time:95,99.99 are both valid.

It is optional. In case there are no percents, metric[2] is false and percentList is not added to the request. ES returns the default list[ 1, 5, 25, 50, 75, 95, 99 ] then.

you are right. disregard

nreese · 2017-11-28T16:41:49Z

src/core_plugins/timelion/server/series_functions/es/lib/create_date_agg.js

+      if (metric[0] === 'percentiles' && metric[2]) {
+        let percentList = metric[2].split(',');
+        percentList = percentList.map(x => parseFloat(x));
+        dateAgg.time_buckets.aggs[metricName][metric[0]].percents = percentList;


This needs to use the buildAggBody method so scripted fields can be supported.

Scripted fields are already supported.

36: dateAgg.time_buckets.aggs[metricName][metric[0]] = buildAggBody(metric[1], scriptedFields);

buildAggBody sets dateAgg.time_buckets.aggs[metricName][metric[0]] to either field or script, percents is added to the corresponding object afterwards.

you are right. disregard

wkruse · 2017-11-29T14:06:04Z

Done. Should we also mention percentiles aggregation function on the function reference page?

nreese · 2017-11-29T16:04:24Z

@wkruse yes, that text should also be updated. Maybe something like
An elasticsearch metric agg: avg, sum, min, max, percentiles or cardinality, followed by a field. Eg "sum:bytes", "percentiles:bytes:95,99,99.9" or just "count"

@thomasneirynck what are your thoughts on the updated metric description?

wkruse · 2017-11-30T09:00:36Z

@nreese Done.

nreese

looks good to me (lgtm)

thomasneirynck

This is a nice addition @wkruse, thanks a lot!

I think there's a small bug when dealing with time-ranges with empty data. Could you double check?

thx,

thomasneirynck · 2017-12-04T21:36:41Z

src/core_plugins/timelion/server/series_functions/es/lib/agg_response_to_series_list.js

-        series[key] = series[key] || [];
-        series[key].push(val.value);
+        if (val.values) {
+          _.forOwn(val.values, function (bucketValue, bucketKey) {


We should filter here on NaN values. These arise when there is no data in the time-range of the bucket. They get propagated to the client, and the code breaks on the front-end (https://github.com/thomasneirynck/kibana/blob/e274f118f720386c551d3941668387f686e3d52c/src/core_plugins/timelion/public/panels/timechart/schema.js#L168-L168).

TypeError: y.toFixed is not a function at setLegendNumbers

wkruse · 2017-12-05T17:07:34Z

Nice catch, thanks for looking into it @thomasneirynck!

First I tried to remove NaN values from the buckets, but it shifted the charts on the time axis.
Replacing NaN with 0 adds "gravitation".
Replacing 'NaN' string with the real NaN fixes the issue without affecting the visualization.
That is what I call scientific trial and error.
I sprinkled some NaN's into the test case.

wkruse · 2017-12-05T17:37:40Z

Input data are in JSON. NaN is encoded as 'NaN' as there are no support for NaN in JSON. That is why I had to decode it back to NaN. Now it makes sense! 👍

thomasneirynck · 2017-12-11T14:53:24Z

thanks for the quick update @wkruse, I'll try and take a look later today

thomasneirynck

thanks @wkruse 🙇‍♂️ !

@nreese description for metric is good by me.

nreese · 2017-12-11T22:12:35Z

Merged!

thanks @wkruse

…15154) * Add Timelion percentiles aggregation support (elastic#8953) Elasticsearch already supports percentiles aggregation. In order to support it in Timelion we add `percentiles` aggregation function and optionally a list of comma separated `percents` to the request. The response contains a list of buckets, each of them is a single series. * Add percentiles as an argument suggestion * Add test case for percentiles multi-value metric aggregation flattening * Add percentiles to the function reference help page * Fix broken frontend on NaN values

* Add Timelion percentiles aggregation support (#8953) Elasticsearch already supports percentiles aggregation. In order to support it in Timelion we add `percentiles` aggregation function and optionally a list of comma separated `percents` to the request. The response contains a list of buckets, each of them is a single series. * Add percentiles as an argument suggestion * Add test case for percentiles multi-value metric aggregation flattening * Add percentiles to the function reference help page * Fix broken frontend on NaN values

nreese requested review from nreese and thomasneirynck November 27, 2017 15:18

nreese added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Feature:Timelion Timelion app and visualization v6.2.0 v7.0.0 release_note:enhancement labels Nov 27, 2017

nreese reviewed Nov 28, 2017

View reviewed changes

wkruse added 2 commits November 29, 2017 11:35

Add percentiles as an argument suggestion

e508981

Add test case for percentiles multi-value metric aggregation flattening

9fb8ca2

Add percentiles to the function reference help page

dadf35b

nreese approved these changes Nov 30, 2017

View reviewed changes

thomasneirynck reviewed Dec 4, 2017

View reviewed changes

Fix broken frontend on NaN values

4f66a8d

thomasneirynck self-requested a review December 5, 2017 21:06

thomasneirynck approved these changes Dec 11, 2017

View reviewed changes

nreese merged commit 61ee81f into elastic:master Dec 11, 2017

nreese mentioned this pull request Dec 11, 2017

[6.x] Add Timelion percentiles aggregation support (#8953) (#15154) #15533

Merged

wkruse deleted the feature/timelion-percentiles-aggregation-support branch December 12, 2017 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Timelion percentiles aggregation support (#8953) #15154

Add Timelion percentiles aggregation support (#8953) #15154

wkruse commented Nov 24, 2017 •

edited by nreese

Loading

elasticmachine commented Nov 24, 2017

wkruse commented Nov 24, 2017

nreese left a comment •

edited

Loading

nreese Nov 28, 2017

wkruse Nov 29, 2017

nreese Nov 29, 2017

nreese Nov 28, 2017

wkruse Nov 29, 2017

nreese Nov 29, 2017

wkruse commented Nov 29, 2017

nreese commented Nov 29, 2017

wkruse commented Nov 30, 2017

nreese left a comment

thomasneirynck left a comment

thomasneirynck Dec 4, 2017

wkruse commented Dec 5, 2017

wkruse commented Dec 5, 2017

thomasneirynck commented Dec 11, 2017

thomasneirynck left a comment •

edited

Loading

nreese commented Dec 11, 2017

Add Timelion percentiles aggregation support (#8953) #15154

Add Timelion percentiles aggregation support (#8953) #15154

Conversation

wkruse commented Nov 24, 2017 • edited by nreese Loading

elasticmachine commented Nov 24, 2017

wkruse commented Nov 24, 2017

nreese left a comment • edited Loading

Choose a reason for hiding this comment

nreese Nov 28, 2017

Choose a reason for hiding this comment

wkruse Nov 29, 2017

Choose a reason for hiding this comment

nreese Nov 29, 2017

Choose a reason for hiding this comment

nreese Nov 28, 2017

Choose a reason for hiding this comment

wkruse Nov 29, 2017

Choose a reason for hiding this comment

nreese Nov 29, 2017

Choose a reason for hiding this comment

wkruse commented Nov 29, 2017

nreese commented Nov 29, 2017

wkruse commented Nov 30, 2017

nreese left a comment

Choose a reason for hiding this comment

thomasneirynck left a comment

Choose a reason for hiding this comment

thomasneirynck Dec 4, 2017

Choose a reason for hiding this comment

wkruse commented Dec 5, 2017

wkruse commented Dec 5, 2017

thomasneirynck commented Dec 11, 2017

thomasneirynck left a comment • edited Loading

Choose a reason for hiding this comment

nreese commented Dec 11, 2017

wkruse commented Nov 24, 2017 •

edited by nreese

Loading

nreese left a comment •

edited

Loading

thomasneirynck left a comment •

edited

Loading