Metrics/librato

[taraspos]

This PR adds librato client.
Important, this is a client for Librato source metrics and NOT new Librato tagged metrics.
The problem that it is impossible to migrate fro source to target metrics without of creation of a new Librato account (and data loss), so plenty of people uses source metrics.
However, API seems pretty similar, so this package potentially may be modified to use any of the APIs.

Some additional fixes:

1. FillCounter could generate n == 0 which didn't add any values to the counter, which made some tests unreliable.

[peterbourgon]

Comment to the Add function says:

// Add locates the time series identified by the name and label values in
// the vector space, and appends the delta to the last value in the list of
// observations.

however, real behaviour is different. Instead of incrementing existing observation, it creates new incremented observation.

The comment is wrong but the behavior is correct: Add should create a new incremented observation. Comment should be

// Add locates the time series identified by the name and label values in
// the vector space, adds delta to the last value in the list of observations,
// and appends the new value to the list.

[taraspos]

OK, I will change it back.
Can you please explain a bit, why is it like that?

My use case:
I see that most of the Gauges implementation just take the last(values).
Since librato allows us to send batch metrics, I wanted to keep this feature and allow to send the batch of metrics added by gauge.Set(val), however, this makes gauge.Add(val) useless, because it will send the batch of ever incrementing metrics.

So I was thinking of the way as Add should work (at least as I imagine it).
So basically if I wanted to have aggregated sum of values over time, I would use Add(val) and if I wanted to report batch of metrics, I would use Sum(val).

[peterbourgon]

Can you please explain a bit, why is it like that?

Each point in the lv vectorspace represents not a single value but a timeseries, a sequence of values collected over time. Operations on points in the vectorspace (Set, Add, Observe, ...) always append values to the timeseries. The Add method is a bit unique in that the value that it appends is a function of the previous value, which is why that extra code is necessary.

Since librato allows us to send batch metrics, I wanted to keep this feature and allow to send the batch of metrics added by gauge.Set(val), however, this makes gauge.Add(val) useless, because it will send the batch of ever incrementing metrics.

I don't quite understand the point you're making. Sending one metric or sending a batch of metrics shouldn't depend on how Go kit represents timeseries data in memory. Specifically, with a gauge, if all you care about is the current value at a given time, and you don't care about the history of those values, then you can just take the last value in the observations []float64.

[peterbourgon]

Should you even be using the lv package? Does Librato provide Gauge, Counter, and Histogram-like data types that can easily be coerced to the Go kit interface expectations?

[taraspos]

Librato has the support of Counters and Gauges.
I took the implementation of cloudwatch as an example.
For the histogram, Librato doesn't have it, but I like the idea of having percentiles, so I also took an example of cloudwatch implementation where Histogram used to generate multiple gauges with percentiles.

[peterbourgon]

So maybe just use Librato Counters and Gauges directly, without lv stuff?

[taraspos]

I also wanted to be able to do some client-side aggregation, to be able to save a bit on the number of metrics/requests sent to librato. All the lv stuff seemed handy for that purpose.

For some metrics, I need to be able to do average over the period of time, for some sum and for others to send them as is.

So for averages, I used generic.NewSimpleHistogram() on top of separate averages lv.Space and hoped to be able to use gauge.Add() for sum and gauge.Set for the rest.

One of the solutions I had in mind is creating similar sums lb.Space which will be aggregated as sum before sending, do you think that will be a better approach?

[taraspos]

I reverted that commit about Add() function, also, refactored the gauges a bit.

So, for Gauge I implemented used Librato Gauge Aggregation. However, for collecting metrics this way, the gauge.Add() function is meaningless, so I had to mock it, so gauge.Set() and gauge.Add() both will behave as gauge.Set().

But I still need the Gauge that can be aggregated as sum of values over period of time. In ordered to do so, I added new space sums *lv.Space. However, for that, I had to mock gauge.Set(), since in this case using it has no sense as well. So fo for this gauge gauge.Set() and gauge.Add() both will behave as gauge.Add().

Please, let me know, what you think, and I will try to test it on real librato account tomorrow.

[taraspos]

Hey, @peterbourgon I tested this on the real account, added some comments and improvements and now it is ready! Please, take a look when you have time. Thanks!

[taraspos]

Hey @peterbourgon.
Kind reminder about this PR :)

[sagikazarmark]

@trane9991 I don't want to speak for @peterbourgon , but lately Go kit doesn't accept any more changes like this in the core. There is no reason why a library like this would have to live there.

So I'd suggest releasing this as a separate library and then open a PR to the README to link to your project.

Also see #843 for future plans.

[peterbourgon]

Second @sagikazarmark. Sorry.