samplerate support all metrics, not just counters #168

Closed
Dieterbe opened this Issue Oct 15, 2012 · 3 comments

Projects

None yet

3 participants

Contributor

our use case: tracking php memory usage means,percentile, etc of php pagerequests by using the timing metric. obviously this caused a whole lot of requests (thousands per second), making statsd memory usage go through the roof.

the README suggests samplerate is only accepted for counters. and statsd.js only has explicit code for samplerate support for counters.
I think this can be extended (note that sample client libs such as https://github.com/etsy/statsd/blob/master/examples/php-example.php have explicit support for sampling all metric types, although it's not fully implemented in statsd.js or even documented in the readme), so you can already send any metric sampled in your client.

here's what needs/can be done in statsd.js (in addition to updating the readme):

  • timers: can easily be modified for correct samplerate support: only the "sum", "count" fields needs to be multiplied by 1/sampleRate. mean/upper/stddev/lower and their percentile-variants will stay correct (assuming decent sampling) (if #162 gets merged, the freqs also need to be multiplied by 1/sampleRate)
  • gauge: no code needs a change, when statsd receives sampled gauge metrics, things will keep working just fine
  • sets: multiply count by 1/sampleRate.
Contributor

I cleaned up the PHP example and removed the sampleRate parameters in my pull request since stats.js didn't support it. If your proposal goes in, we can get those parameters added back in.

See: #163

@jmacd3 jmacd3 referenced this issue in statsite/statsite Oct 18, 2012
Closed

Sampling Question #10

Contributor

i noticed that timers are the only metric types that consume (by far) more cpu than any other metric, so sampling them is pretty much the only way to keep statsd cpu usage under control as your number of metrics increases

Contributor

Did your version of stats.js support sample rate on timers? The mainline version did not at the time I made the change.

@pathzzrd pathzzrd closed this Mar 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment