Add scripted_metric in Visualize

[reda-alaoui]

Fixes #2646

This PR adds the ability to use Scripted Metric Aggregations in Kibana visualizations.
Each scripted metric aggregation is composed of 5 fields:

• selectbox lang
• textarea init_script
• textarea map_script
• textarea combine_script
• textarea reduce_script

lang defines the language of the 4 scripts and is by default bound to painless.
Inline scripts are expected inside textareas.

Inspired by the changes made by @pineur in #5558.

[spalger]

I don't think that defaulting to "percent" is necessary here. Just use the default instance of "number"

[reda-alaoui]

Ok I will correct that

[spalger]

We'll need to add the ability to choose the scripting language to use here, which means it will need to reach out to the API to discover which scripting languages are available.

Fetching the languages is currently only done in the field editor, so check that out for some inspiration.

We also don't want to fetch the list of languages until we need it, so how about you add a controller to the new "lang" param like we do in the terms agg and do the fetching in there.

[reda-alaoui]

Currently, because of ES, lang can't be passed globally for the 4 scripts.
If someone needs to use another language, a JSON object must be passed instead of a string for each script.

For example:
{ "inline": "....", "lang": "painless" }

Do you want me to add lang selection for each script?
If so, I think that a selectbox must be also added to switch between file/inline/stored and a new field to hold script params map. It is turning into a heavy development ^^

[spalger]

I think that a global language selector is sufficient. Each param can specify a write() method that can produce the { "inline": "", "lang": "" } version of the param:

{
  name: 'reduce_script',
  type: 'string',
  write(aggConfig, output) {
    output.params.reduce_script = {
      lang: aggConfig.params.lang,
      inline: aggConfig.params.reduce_script
    }
  }
}

[spalger]

This is looking good, let me know if you need help with the requested changes

[elasticmachine]

Can one of the admins verify this patch?

[reda-alaoui]

All requested changes applied ;)

[spalger]

Hope you don't mind, but I moved things around a little bit Cosium#1

[spalger]

jenkins, test this

[epixa]

@reda-alaoui Can you update the description of this PR with details about the nature of the change? Keeping a reference to previous PRs/issues is great, but people should be able to figure out what this PR is all about without clicking into other PRs. That'll be greatly helpful for both reviewers and for when we link to this PR from release notes. Thanks!

[reda-alaoui]

@epixa, I updated the description

[ppisljar]

hmm ?

[reda-alaoui]

Corrected ^^

[epixa]

@reda-alaoui That is great, thanks!

[ppisljar]

seems i need to enclose everything inside the fields in quotes ?

for example in init field i need to put quotes around:
"_agg['transactions'] = []"

if i do it without
_agg['transactions'] = []

i get a compilation error. i think it would be nice to auto add those quotes, what do you think ?

[reda-alaoui]

@ppisljar, I just did a test and I don't have your issue.
I am able to write script without quotes into the script fields and it does work.

But I noticed that blank script field were used to build a script object missing its inline attribute:

"init_script": {
  "lang": "painless"
}

I corrected this in my latest commit 674e7d0.

[reda-alaoui]

@ppisljar, can you provide your compilation error?

[ppisljar]

Error: Request to Elasticsearch failed: {"error":{"root_cause":[{"type":"script_exception","reason":"compile error","script_stack":["_agg['transactions'] = []","^---- HERE"],"script":"_agg['transactions'] = []","lang":"painless"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query_fetch","grouped":true,"failed_shards":[{"shard":0,"index":"logstash-0","node":"LFGTAxdyQHSnO8B8iMKL9w","reason":{"type":"script_exception","reason":"compile error","caused_by":{"type":"illegal_argument_exception","reason":"Variable [_agg] is not defined."},"script_stack":["_agg['transactions'] = []","^---- HERE"],"script":"_agg['transactions'] = []","lang":"painless"}}],"caused_by":{"type":"script_exception","reason":"compile error","caused_by":{"type":"illegal_argument_exception","reason":"Variable [_agg] is not defined."},"script_stack":["_agg['transactions'] = []","^---- HERE"],"script":"_agg['transactions'] = []","lang":"painless"}},"status":500}
    at https://localhost:5601/hzx/bundles/kibana.bundle.js?v=8467:74782:37
    at Function.Promise.try (https://localhost:5601/hzx/bundles/commons.bundle.js?v=8467:67701:21)
    at https://localhost:5601/hzx/bundles/commons.bundle.js?v=8467:67670:29
    at Array.map (native)
    at Function.Promise.map (https://localhost:5601/hzx/bundles/commons.bundle.js?v=8467:67669:29)
    at callResponseHandlers (https://localhost:5601/hzx/bundles/kibana.bundle.js?v=8467:74754:21)
    at https://localhost:5601/hzx/bundles/kibana.bundle.js?v=8467:74143:15
    at processQueue (https://localhost:5601/hzx/bundles/commons.bundle.js?v=8467:37125:29)
    at https://localhost:5601/hzx/bundles/commons.bundle.js?v=8467:37141:28
    at Scope.$eval (https://localhost:5601/hzx/bundles/commons.bundle.js?v=8467:38369:29)

[ppisljar]

im using example from doc: https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-metrics-scripted-metric-aggregation.html

[reda-alaoui]

@ppisljar, this is because the doc is wrong.
Me too I had your issue at the beginning.
The doc you are pointing at holds an example written for groovy, not painless.
In painless, _agg is hold by a variable params.

Use def to declare new variable and params._agg.
Example:

"params": {
    "init_script": "params._agg['result'] = 0d",
    "map_script": "params._agg['result'] += doc['foo'].value",
    "combine_script": "return params._agg['result']",
    "reduce_script": "def result = 0d; for(int i=0; i<params._aggs.length; i++){result+=params._aggs[i];} return result;"
  }

[manasranjan11]

Do we know when will this patch be available with official release ?

Is it possible to use aggregations like avg , percentage sum etc while doing calculations on a scripted field ?

[tbragin]

@reda-alaoui I just tried checking out this PR on top of master and I get this error, whenever I click on for instance "Split Rows" in vis builder. Has anyone encountered this?

Error: Uncaught TypeError: Cannot use 'in' operator to search for 'draggableItemCtrl' in undefined (http://localhost:5601/bundles/kibana.bundle.js?v=8467:70)
    at window.onerror (http://localhost:5601/bundles/commons.bundle.js?v=8467:67:13987)

[ppisljar]

@tbragin can't reproduce your problem, could you provide exact steps ? thanks

[ppisljar]

@Bargs metric and table visualizations use number formatter by default for this reason you get '' for all your values when you return 'foo'. #7302 depends on the field type to use the right formatter but in this case you could return anything from the script.

• One possible way forward i see would be to allow user to select the formatter he wants to use and make all formatters do some kind of validation on the value. For example number formatter should throw an error if value is NaN. We would then need to filter the available formatters per visualization type (for example point series, pie charts and maps should only support number). This still doesn't solve the case with mixed type. You could return an array from your script with different types like ['hello', 5, 5.12, 'world']. The best we could do in this case is require that choosen formatter can handle all the values (in this case String Formatter would work)
• another option would be to only allow numeric types to be returned. this would limit the scope but make this much simpler to handle.

@reda-alaoui this is also missing tests. At least unit tests should be there but this specific change could also benefit from some functional tests. Let me know if you need help with any of those. Its OK with me to write those later when we'll have more clarity on which way we want to take this but I just wanted you to know.

[spalger]

I agree with @ppisljar, this needs some tests to verify the new agg type

[tbragin]

@ppisljar Turns out it was a problem with the build, unrelated to this PR: #9151

I'll try again using another setup.

[ppisljar]

@Bargs can you give your opinion on this ?

[Bargs]

I think either option is reasonable at this time. We don't know how many users will want to return non-numeric values, so it may not be worth the additional complexity of exposing the field formatters. If we restrict the agg to numbers only we should add some help text explaining that, and show an error if we detect that the agg is returning a non-number.

Down the road if a lot of users want non-numbers we can expose the field formatters as an option at that time.

[ppisljar]

OK lets restrict this to only numeric types. If non numeric type is returned from script an error should be shown. What do you think @reda-alaoui ?

[tbragin]

@reda-alaoui A couple of questions and comments:

• I'm curious about the use cases you are personally looking to solve with scripted metric aggregation. The reason I ask is because the ones I hear about frequently have to do with calculating ratios (e.g. here). But now that there is date math (including ratio) support in pipeline aggregations, I wonder if that's a simpler interface against which to build interaction.

• I tried this out on a quintessential error rate example, with both Metric and Line visualizations, as well as on a dashboard, and noticed a few problems.

First, the example:

POST logstash-*/_search?size=0
{
    "query" : {
        "match_all" : {}
    },
    "aggs": {
        "profit": {
            "scripted_metric": {
                "init_script" : "params._agg.errors = 0; params._agg.total = 0; ",
                "map_script" : "if (doc['response.keyword'].value == '404') { params._agg.errors += 1} params._agg.total += 1; ", 
                "reduce_script" : "int errors = 0; double total = 0; for (a in params._aggs) { errors += a.errors; total += a.total; } return errors/total; "
            }
        }
    }
}

---

Problem 1. Sorting by metric resulting from the scripted metric aggregation does not work in the vis builder, and this is the default behavior when creating any sort of terms aggregations and sub-aggregations.

Example with sub-aggregation on a line chart

Example with Terms aggregation in table

---

Problem 2. I saw some really strange behavior, where the visualization that previously worked somehow gets confused and switches rows and columns. There seems to be no way to recover. Have you seen that?

[fbaligand]

+1 for this feature

[ppisljar]

@reda-alaoui do you plan to take this further ? if not, would you be ok with me taking this pr further ?

[reda-alaoui]

@ppisljar , I am sorry because I currently don't have time to finish this PR.
So be my guest :)

[deeptiantony]

Would like to know when this will be officially released? Is there any timeline?

[ppisljar]

there is no timeline, but we do plan to take this further.

[thomasneirynck]

Hi @reda-alaoui I'm inclined to close this, as this seems to be a little stalled for a while now.

Thanks for your efforts on this, it would certainly be a nice improvement. From our end, there's no immediate plan to add this, so don't hesitate to reopen this PR if you would like to continue to work on this. We'll ping you should plans changed around this on our end too.

Thanks again,

[fbaligand]

I'm sad to see this pr closed :(
I would love to use this feature !

[martin-g]

Very sad as well!
This is a functionality that I needed few times by now.