derivative metric aggregation

[ppisljar]

Summary

(Note: See this PR also in combination with #10070 and #10033).

Kibana Visualizations like line, bar, area charts now support the majority of Elasticsearch's pipeline aggregations. These include the majority of parent and sibling aggregations . These aggregations give you a new look into their data. For example, the moving average aggregations allows users to smooth over outlier data points over time, or the derivative enables users to display rate of change.

---

derivative metric aggregation - cant find existing issue

[thomasneirynck]

jenkins, test this

[thomasneirynck]

Do we need the 'Metric' combo-box? It only ever seems to include a single options? (maybe I''m missing something).

[thomasneirynck]

1. Anyway we can slightly indent the derivate-form? This is especially relevant for second-order derivatives.

2. I don't think we need the Custom Label option for derivative (?). This probably need to be completely rewritten at each step. E.g. Distance->Velocity->Acceleration. We only care about acceleration since that's the only one that will get displayed.

[ppisljar]

Metric combo box: yeah, as you might select an existing metric. This follows the same pattern you see if you choose Term bucket agg and look at order by combo box. You can select a pre-existing metric or you could add a custom metric.

I will update that design also matches that. i think that would address your 1) point.

regarding 2), i would maybe just leave it like that, for the sake of simplicity first and because it matches the pattern seen in other parts (like the order by in terms agg mentioned above). I agree that's not the best, but i would fix that in separate PR and in every place where it applies.

[stacey-gammon]

Switch to a named export?

[ppisljar]

every other agg type is using default export ... what do you think would be better? keeping this similar to other aggs ? or going with named export ? (which wont be used as this is only exporting a single thing)

[stacey-gammon]

It's in our style guide, but I see your point. Up to you.

[stacey-gammon]

Speaking of custom labels... am I supposed to see goodbye as a label somewhere on this viz?

[EDIT]

Looks like the Hello overrides the goodbye portion of the custom label. I feel like the custom label should be the whole thing, not derivative of [custom label].

[stacey-gammon]

Playing around and was able to run into this error. Albeit, I really don't know what I'm doing. :)

[stacey-gammon]

I think this name is confusing - it's not a DerivativeAggType but a MetricAggType. The way it is named now, it looks like a subclass of MetricAggType, but if that was the case, you wouldn't have to pass in all the options to specialize the metric for a derivative type.

[stacey-gammon]

Looks like this function could be pulled out into a separate file and then tested separately?

[ppisljar]

you mean makeLabel ? I would argue that it would be less readable after. I usually look at the following:

• can the code be reused by something else ? if yes, separate out in a new file
• is the original file too complex or long ? if yes, separate out
• is it impossible to test ? if yes, separate out

but i think on all above the answer is no (i can test, the file is not really long, and this seems specific to just this agg, wont be reused elsewhere)

[stacey-gammon]

I suppose I disagree on the second point, I think this file would be more readable with less LOC. The more focused functions are, the more I can concentrate on what it is doing, and switch to other focus areas as a follow up. This file has a whole controller in it, which could also probably be split out. It also becomes tough to follow with a lot of indentation.

For instance, the creation of the params argument is a bit confusing for me to follow. It looks like the object parameters are later translated into classes based on the type and/or name field of the object. I got a little lost however because there are two agg_params.js files. More functions generally mean more comments, or barring that, self documenting code using the function name.

I also look at this file and wonder if we can separate the business logic from the angular portions. makeLabel has nothing angular specific about it, but this file has a reference to $scope and Private, which is angular specific.

Anyway, I'm fine if you leave as is, just expressing my opinion. This is the first I've looked at the visualize code, so trying to follow the logic is a bit tough.

[ppisljar]

regarding labels ....
if you have Derivative of Count ... if you put custom label on count you could have Derivative of CUSTOM ... but if you put custom label on derivative you can completely override it with CUSTOM LABEL.

As @thomasneirynck pointed out above custom labels on intermediate levels don't make much sense, and i would prefer to solve that in separate PR, as similar issue already exists in visualize, hence i would like to fix it everywhere and its not only related to this PR.

[ppisljar]

regarding the error, i can't see what buckets have you selected ... clould you paste whole request ?

• this happens because you are trying to use derivative with something thats not histogram or date_histogram
• the error should be properly handled by visualize editor (coloring derivative red ... ) ... this doesn't happen obviously

[ppisljar]

tests are failing on FAIL: chrome on any platform - kibana - visualize app - visualize app - tile map chart - should zoom out to level 1 from default level 2, which is unrelated to this pr

[stacey-gammon]

Here is the request that caused the error. I didn't select anything for the x-axis, I guess that is the issue.

{
  "size": 0,
  "query": {
    "bool": {
      "must": [
        {
          "query_string": {
            "analyze_wildcard": true,
            "query": "*"
          }
        },
        {
          "range": {
            "@timestamp": {
              "gte": 1485357911849,
              "lte": 1485358811850,
              "format": "epoch_millis"
            }
          }
        }
      ],
      "must_not": []
    }
  },
  "_source": {
    "excludes": []
  },
  "aggs": {
    "2": {
      "avg": {
        "field": "bytes"
      }
    },
    "3": {
      "derivative": {
        "buckets_path": "2"
      }
    }
  }
}

[ppisljar]

@stacey-gammon I extracted the functions to new files, also the error should be fixed now.

[stacey-gammon]

Nice, thanks!

So it's a bit confusing now where the error is coming from when not using the historgram for x-axis. I guess there is nothing we can do about that since we dont' support tooltips on that error icon?

I think I ran across another bug. If I first select custom metric for a sub derivative and run the visualization, I see results. If I then go back and select the non custom metric and hit run, nothing changes in the view.

[ppisljar]

you rock @stacey-gammon !

regarding errors ... yeah they suck at the moment ... but I plan to start working on that shortly (in a separate PR)

[ppisljar]

the bug is fixed (seems i introduced it during moving of makelabels function).
also i added separate tests for the make_nested_label

[ppisljar]

jenkins, test this

[stacey-gammon]

Think I found something else. For some reason If I click x-axis first, but don't select anything, the custom derivative section looks funny. If I don't click x-axis first, everything seems to work. See if you can repro.

[ppisljar]

@stacey-gammon thanks ! it should be fixed now, also i fixed the tests (selenium ones are still failing due to tile map issue). can you take another look ?

[thomasneirynck]

I like the new error messaging in place. It's useful, and would work better than an app-wide toast IMHO.

[cjcenizal]

@alexfrancoeur I think the second example is better than the first one, since it grabs your attention more. It would be good to style these errors consistently, and then we can update their design en masse.

[tbragin]

@ppisljar I also like the new error handling in the context of what we're trying to do. It's consistent with the other place where we signal something about agg ordering (pie chart), and I think it gives the user something actionable to react to when the aggregations are incorrectly ordered for derivative.

I have a minor tweak proposed for help text:

• instead of "Last bucket must be "date histogram" or "histogram" when using derivative metric!"
• i propose "Last bucket aggregation must be "Date Histogram" or "Histogram" when using "Derivative" metric aggregation!"