Statsd output is not good

[drawks]

Aside from the actual broken protocol packets in #207 the implementation of instrumentation with statsd in this project is just really bad. Exporting gauges of pre-aggregated metrics is a serious anti-pattern to typical/normal statsd instrumentation best practices. I'm curious if there is some specific reason why this model was adopted. Is there any appetite for re-instrumenting in a more standard manner?

[magiconair]

You could ask the original committer of the approach or provide a better way of doing this. However, before you do this I'd like to understand what "really bad" means in this context. So far nobody has complained about it and aside from the one bug it seems to have been working for people. I usually prefer a fact based approach on discussions and I'm curious which problems the current implementation creates and how you would solve them.

[drawks]

statsd is meant primarily as a stat aggregator. a typical pattern for using statsd is to emit events on instrumented code paths and allow the statsd implementation handle the aggregation. For example emit a counter event when a code path is hit and then allow for statsd to flush that counter at a configurable interval so you get roll up aggregates in hits per time unit where time unit is a property of the statsd server. Or to emit a timer event when an instrumented code path is hit and allow for statsd to bucket that timer into a configurable histograms/calculate quantiles etc.

The pattern in use in fabio is to handle aggregation internally and expose pre-aggregated metrics only as gauges. This limits the ability of consumers of the metrics to shape the data as desired. For instance a consumer is limited to only the quantiles which you've chosen to expose via the code.

AFAICT it looks like this is a side effect of using github.com/rcrowley/go-metrics which handles aggregation with intent of periodic flushing as gauges to backends that depend on that model, i.e. graphite

[discobean]

Using raw metrics when sending to statsd would certainly make life simpler

[magiconair]

Can you provide a specific example? Which metric, what is the current format, what is the desired format? Please keep in mind that I'm not a statsd expert and that you may have to explain more to me than to people already familiar with the matter.

[drawks]

I'm not sure I can give a better explanation without just writing an example PR. but....

Within the fabio code you've got the notion of a Registry interface that has a few functions to service counters and timers. In order to implement a more normally functioning statsd client each call to Registry.Counter.Inc(n) would emit a single statsd udp packet destined for a statsd server and likewise for Registry.Timer.Update and Registry.Timer.UpdateSince.

The current implementation of statsd sending relies on github.com/magiconair/go-metrics-statsd which is pasted on top of github.com/rcrowley/go-metrics that has a notion of storing the aggregates internally to the process.

[discobean]

I have found a few key points that are frustrating us:

1. The metrics for count are absolute count received since the process started
2. I actually have no idea what min/max/mean/std-dev/50-percentile etc actually show
3. The metrics for one-minute, five-minute are not correct

I'll elaborate on 2 and 3:

min/max/mean/std-dev/50-percentile:

As a test I have pushed through some data to fabio and recorded the metrics. The initial values for min/max/mean are: 0/0/0

I push through one request, the values are now each: 44462314.0/44462314.0/44462314.0.

What exactly is 44462314.0? My guess is it is latency, but is it in ms? microseconds? Probably it is just me, but I have no idea. The actual request time from my client was ~353ms

Now each flush of metrics the value of min/max/mean remains the same, even though I am not putting any traffic through. So the mean is always 44462314.0, forever.

So the value is the min/mean/mean value for all the requests since the beginning of the process? That data is not really usable.

one-minute, five-minute issue

I found after putting through requests over time, the one-minute value never goes to 0. Even though there is no traffic going out for the past minute :S

If I wait sometime, and then push traffic through for a minute, the 1 minute value != the values of traffic I actually sent in that last minute.

What would be a solution

I think the metrics required would be:

1. the number of requests
2. the min/max/mean/average latency

Importantly they would be relative to that metrics period. So if metrics.interval is set to 60, the data would be the total request count for 60 seconds, etc..

For our purposes only, we just require metrics per service, 2 values: count/latency, we do not require a full suite of metrics per URL/host/fabioserver combination as that is just too much (and we push metrics to cloudwatch which makes it more expensive).

ie. service-xyz.count and service-xyz.latency_min/max/avg/mean

Or something along those lines...

[drawks]

I've issued a quick PR which demonstrates how a simple raw statsd sink can be implemented which doesn't do any internal aggregation and fits more directly into the intended typical statsd instrumentation model. This PR probably isn't appropriate to merge as written, but builds and works as intended in some limited local testing.

[magiconair]

Sorry for the delay but I'm on vacation with the family.

The key point here is that you want to send an event to statsd when it happens and want to do the aggregation yourself. So far I've relied on go-metrics to do The Right Thing (™) but I recall some discussions around integrating the circonus support that the histograms in go-metrics aren't that useful. Mostly because they're a port of the codahale model from Java. I didn't dig deeper there. Maybe @maier can help here.

@drawks thx for the PR. I'll try to look at it soon. Kids waking up now :)

[magiconair]

@drawks I think your PR would be good enough for an experimental inclusion. Don't you need to add newlines between multiple metrics?

The downside of metrics that are not pre-aggregated is that the API will no longer provide these values. If you are using this provider you probably don't care about that.

However, integrating one of the libraries you suggested was simple enough that I've tried that in #329. To avoid the alloc per metrics I need to refactor the registry interface a bit but I'd appreciate if you could test the integration.

You should set:

./fabio -metrics.target statsd_raw -metrics.interval 100ms

I'll sleep a bit on the naming of statsd_raw vs raw_stats and the corresponding struct names. Also need to update the documentation.