Scriptable Metrics Aggregation #7075

colings86 · 2014-07-29T09:06:58Z

A metrics aggregation which runs specified scripts at the init, collect, combine, and reduce phases

clintongormley · 2014-07-29T11:02:14Z

Looking forward to reading the docs :)

colings86 · 2014-07-29T11:08:13Z

@clintongormley yep. Long way from finished yet as need to write tests and read the docs. Put the PR up as @brwe wanted to have a look at it and so others could have a play around with it but its not ready for review yet

mattweber · 2014-07-29T14:05:34Z

+1! So cool, can't wait to give this a try. Thanks!

colings86 · 2014-07-30T08:14:58Z

Following gist can be followed as an example of using the scriptable metrics aggregation.

https://gist.github.com/colings86/0994f1b188b41be6052a

jpountz · 2014-07-30T08:31:44Z

src/main/java/org/elasticsearch/search/aggregations/AggregationBuilders.java

@@ -156,4 +157,8 @@ public static TopHitsBuilder topHits(String name) {
    public static GeoBoundsBuilder geoBounds(String name) {
        return new GeoBoundsBuilder(name);
    }
+
+    public static ScriptBuilder script(String name) {


Maybe the name should reflect the fact that this is a metric aggregation to leave some namespace if we have a scripted bucket agg some day?

jpountz · 2014-07-30T08:55:19Z

I left some comments but my major concern is about the client: do we need to expose it to the scripts? I'm worried that it could allow to do crazy things?

colings86 · 2014-07-30T09:06:35Z

@jpountz I agree that exposing the client is dangerous and it allows for some crazy things. I spoke to @imotov about this and he said that it was something explicitly requested for the scripted facets. I would be happy to not provide it to the scripts, it would also make the code a bit cleaner

uboness · 2014-07-30T10:35:29Z

+1 on not exposing it... we shouldn't encourage doing crazy things with scripts

rjernst · 2014-08-05T22:50:28Z

One note: expression scripts can only be used for "search", they do not allow "executable". It looks like only the map phase uses search, and the rest executable?

colings86 · 2014-08-06T06:45:36Z

Yeah you are right. Actually since the expression language doesn't support assignment (i think, correct me if I am wrong) I don't think you would be able to use it even in the map script since the map script has to update some state in the params. Also, currently it is assumed that the same language is used for all the scripts in a single aggregations. I think the use cases for this aggregation are almost always going to need more complex scripts than the Lucene expression language can provide, so I see them mostly using groovy and maybe other languages through plugins

jpountz · 2014-08-06T10:43:10Z

src/main/java/org/elasticsearch/search/aggregations/AggregationModule.java

@@ -18,6 +18,8 @@
 */
 package org.elasticsearch.search.aggregations;

+import org.elasticsearch.search.aggregations.metrics.scripted.ScriptedMetricParser;


Can you put this import with the other ones?

jpountz · 2014-08-06T11:41:14Z

Left some comments but it looks great in general!

clintongormley · 2014-08-07T12:18:20Z

docs/reference/search/aggregations/metrics/scripted-metric-aggregation.asciidoc

+    "aggs": {
+        "profit": {
+            "scripted_metric": {
+                "init_script" : "aggregation['transactions'] = []",


I think we should use a shorter variable than aggregation in the script. How about _agg?

colings86 · 2014-08-19T13:59:11Z

@jpountz I made the code changes you suggested. I also updated the docs with a worked example and left some replies to your comments. Let me know what you think.

jpountz · 2014-08-20T14:15:50Z

It looks good in general, docs are great. In my opinion, the only things that need to be done before merging it would be:

removing the aggregation context from the script context
maybe rename aggregation to _agg in the script context as @clintongormley suggested

jpountz · 2014-08-20T16:26:38Z

LGTM

A metrics aggregation which runs specified scripts at the init, collect, combine, and reduce phases Closes #5923

diegoasth · 2014-10-06T20:30:49Z

Great News! I am trying to use a scripted metric aggregation inside a date_histogram aggregation but unfortunately it seems to take docs outside the date-bucket into consideration, but this is not what I expected..

diegoasth · 2014-10-06T20:34:17Z

{
"size": 0,
"aggs": {
"histo": {
"date_histogram": {
"field": "timestamp",
"interval": "1h",
"min_doc_count": 0,
"format": "yyyy-MM-dd HH:mm"
},
"aggs": {
"profit": {
"scripted_metric": {
"init_script": " _agg['kpiA'] =0;",
"map_script": "_agg.kpiA += doc['kpiA'].value; ",
"combine_script": "teste =0; teste += _agg.kpiA; return teste;",
"reduce_script": "profit = 0; for (a in _aggs) { profit += a;}; return profit;"
}
}
}
}
}
}

colings86 · 2014-10-07T07:41:14Z

@diegoasth I have tried to reproduce the issue your are seeing in the gist below [1], in both 1.4 and in the master branch, without success. Could you take a look at the gist and try it on your system to see if it reproduces your issue. If not, could you create a similar gist to illustrate the steps to reproduce the issue you are seeing?

[1] https://gist.github.com/colings86/d1a9c76898b4b2783b89

diegoasth · 2014-10-08T18:23:36Z

Hi @colings86 , thanks or your attention.

in deed, our gist does not reproduce the issue, so I created another one that shows it. My goal is to use a scripted_metric inside a date_histogram (let me know if it is not possible). I expect that the scripts (init_script, map_script, combine_script and reduce_script) consider only the documents in that specific date-bucket. For the sake of comparison, I used the native SUM aggregation inside the date_histogram and it worked fine, returning the expected results per bucket.

https://gist.github.com/diegoasth/406148525616f9f527ab

colings86 · 2014-10-09T07:52:56Z

@diegoasth this is indeed a bug. Thanks for raising it and for providing the steps to reproduce it. I'll look into getting it fixed

colings86 · 2014-10-09T12:20:32Z

@diegoasth I have raised the following issue for this bug:

#8036

diegoasth · 2014-10-09T13:22:28Z

thank you very much. The ability to wite my custom aggregations (inside ES) is a very awaited feature.

clintongormley assigned colings86 Jul 29, 2014

jpountz reviewed Jul 30, 2014
View reviewed changes

colings86 added the review label Aug 6, 2014

jpountz reviewed Aug 6, 2014
View reviewed changes

jpountz removed the review label Aug 6, 2014

clintongormley reviewed Aug 7, 2014
View reviewed changes

jpountz added the review label Aug 20, 2014

jpountz removed the review label Aug 20, 2014

colings86 added the review label Aug 20, 2014

jpountz removed the review label Aug 20, 2014

Aggregations: Scriptable Metrics Aggregation

7f943f0

A metrics aggregation which runs specified scripts at the init, collect, combine, and reduce phases Closes #5923

colings86 merged commit 7f943f0 into elastic:master Aug 20, 2014

colings86 assigned colings86 and unassigned colings86 Aug 21, 2014

colings86 deleted the feature/scriptableMetricsAggregation branch August 21, 2014 15:10

clintongormley added >feature v1.4.0.Beta1 v2.0.0-beta1 labels Sep 10, 2014

colings86 mentioned this pull request Oct 9, 2014

Aggregations: scripted metric agg does not separate parent buckets #8036

Closed

clintongormley added the :Analytics/Aggregations Aggregations label Jun 6, 2015

clintongormley changed the title ~~Aggregations: Scriptable Metrics Aggregation~~ Scriptable Metrics Aggregation Jun 6, 2015

colings86 mentioned this pull request Aug 4, 2016

Should we remove/modify some of the experiment tags in the documentation #19798

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scriptable Metrics Aggregation #7075

Scriptable Metrics Aggregation #7075

colings86 commented Jul 29, 2014

clintongormley commented Jul 29, 2014

colings86 commented Jul 29, 2014

mattweber commented Jul 29, 2014

colings86 commented Jul 30, 2014

jpountz Jul 30, 2014

jpountz commented Jul 30, 2014

colings86 commented Jul 30, 2014

uboness commented Jul 30, 2014

rjernst commented Aug 5, 2014

colings86 commented Aug 6, 2014

jpountz Aug 6, 2014

jpountz commented Aug 6, 2014

clintongormley Aug 7, 2014

jpountz Aug 20, 2014

colings86 commented Aug 19, 2014

jpountz commented Aug 20, 2014

jpountz commented Aug 20, 2014

diegoasth commented Oct 6, 2014

diegoasth commented Oct 6, 2014

colings86 commented Oct 7, 2014

diegoasth commented Oct 8, 2014

colings86 commented Oct 9, 2014

colings86 commented Oct 9, 2014

diegoasth commented Oct 9, 2014

Scriptable Metrics Aggregation #7075

Scriptable Metrics Aggregation #7075

Conversation

colings86 commented Jul 29, 2014

clintongormley commented Jul 29, 2014

colings86 commented Jul 29, 2014

mattweber commented Jul 29, 2014

colings86 commented Jul 30, 2014

jpountz Jul 30, 2014

Choose a reason for hiding this comment

jpountz commented Jul 30, 2014

colings86 commented Jul 30, 2014

uboness commented Jul 30, 2014

rjernst commented Aug 5, 2014

colings86 commented Aug 6, 2014

jpountz Aug 6, 2014

Choose a reason for hiding this comment

jpountz commented Aug 6, 2014

clintongormley Aug 7, 2014

Choose a reason for hiding this comment

jpountz Aug 20, 2014

Choose a reason for hiding this comment

colings86 commented Aug 19, 2014

jpountz commented Aug 20, 2014

jpountz commented Aug 20, 2014

diegoasth commented Oct 6, 2014

diegoasth commented Oct 6, 2014

colings86 commented Oct 7, 2014

diegoasth commented Oct 8, 2014

colings86 commented Oct 9, 2014

colings86 commented Oct 9, 2014

diegoasth commented Oct 9, 2014