Add support for gauge deltas. #72

Merged
merged 2 commits into from Mar 1, 2013

Projects

None yet

8 participants

@jsocol
Contributor
jsocol commented Apr 9, 2012
  • Add tests for gauges and new gauge deltas.
  • Add support for +/- prefix for gauge values to support modifying
    gauge values in-place.

I realized after committing this that it makes it impossible to set an existing gauge to a negative number. An option would be to make a new "type" (e.g. gd for "gauge delta") that would modify existing gauge values in-place. Thoughts?

Owner
mrtazz commented Apr 13, 2012

Gauges are used to track (arbitrary) values in your application. I'm not sure how deltas fit in here, because that means that you calculate them in your application and send a delta to StatsD instead of the new value you already have. I'm not opposed to this, I just don't want to make StatsD more complex with something that can already be accomplished in a simpler way.

Contributor
jsocol commented Apr 13, 2012

There are a number of use-cases where allowing the app to increment or decrement a gauge is simpler and faster for the app than recalculating the whole value. Particularly event-driven changes.

A graph of open (web)socket connections, for example, is appropriate for a gauge, and allowing onConnect and onDisconnect handlers to bump the value without recalculating simplifies the step in the app. Another example are things like running totals per period (essentially a time integral of a counter, but without the averaging). Any event where the event handlers know how much something changed but not necessarily the total.

Incrementing or decrementing a gauge value would also be more thread-safe, since StatsD itself is single threaded. If two client app processes set a gauge value, the latter wins. But if both adjust the gauge value, StatsD will do the arithmetic safely because addition and subtraction commute.

There may be a simpler way from StatsD's point of view, but I think there are pretty real use-cases where it is much simpler for the client apps to not need to recalculate the whole gauge value before sending it.

This pull request passes (merged 0759f0b into d1368ed).

Contributor
jsocol commented Apr 27, 2012

I'm going to add another commit on top of this that changes it to a distinct "type", then this can be merged with or without that.

This pull request passes (merged eb7d650 into d1368ed).

Contributor
jsocol commented May 11, 2012

As I said 2 weeks ago. Changed gauge modifications to a unique type (gd). I think all the points in my comment before, especially about "thread" safety of gauge values, are still valid and make this a useful change. Making it a different type makes sense and I'll happily update the Python client.

Contributor
jsocol commented May 11, 2012

Also includes tests for gauges and gauge deltas/modifications.

Owner
mrtazz commented May 11, 2012

I will test it some more before merging, but looks good so far.

@ghost
ghost commented May 28, 2012

I'm trying to figure out how to best report gauges from my app to statsd and this looks interesting. But how do you ensure the gauge value doesn't revert to zero in case statsd crashes? Do you somehow detect this and add the previous total after statsd has restarted?

Contributor
jsocol commented May 29, 2012

@thovoll That's an excellent point. It seems to run directly into my point about multiple app processes overwriting each other.

I think providing both a way to set a gauge and to modify a gauge lets the app best decide how to manage those two risks. For example, you could have a cron that reset an absolute value periodically but let your app processes only use deltas, or if your app only changes gauges very rarely, maybe you don't think two processes will overwrite each other, so you just use absolute values. Or if you have gauge that changes frequently, maybe its movement is more important than its value, so you only use deltas.

@ghost
ghost commented May 29, 2012

@jsocol Makes sense. In my case I'll need to track the gauge value outside of statsd and just report it to statsd as a POG - plain old gauge ;)

Contributor
Dieterbe commented Jun 8, 2012

@jsocol at first I agreed entirely with you, I have a similar problem, I deal with user uploads and want to track the number of concurrent uploads, accross multiple processes and machines. At first it seemed best to have statsd maintain the total value so that my app can become simpler (just relaying start/stop events to increment/decrement calls for statsd), but @thovoll's remark is very legit and solving this properly seems to become complicated quickly, so I think maintaining a gauge per server as an absolute value is the best way, even if that means IPC (if you have multiple processes on the same machine), it can cause some locking if your gauge changes immensely often, and it means you'll have more metrics (per-machine instead of global) although we could make statsd aggregate them. An idea for the latter would be sending statsd gauges like ("my_gauge", "machine_a", 20), ("my_gauge", "machine_b", 16), and so on. statsd could then maintain an array of numbers for "my_gauge" instead of a number itself, using "machine_a", "machine_b" as keys in this per-gauge array to update the numbers for each machine, and it can just add them all together and send the total to graphite. this seems fairly simple to implement. your thoughts?

Owner
mrtazz commented Dec 17, 2012

Hey, so sorry for the lack of updates on this. I thought about this and actually like the +- notation a lot better. I think it makes a lot more sense than a completely new distinct type. What do you think about that? Would you be up for updating it to be mergeable with the current master?

Contributor

I would love to see this merged in

Contributor
jsocol commented Dec 17, 2012

@mrtazz I should be able to get it updated and back to the +- notation this week. Thanks for the feedback!

diegovar commented Mar 1, 2013

I'd love this functionality as well

Contributor
jsocol commented Mar 1, 2013

Sorry, this bitrotted quite a bit and I've had a hard time finding time to make the tests work again.

Owner
mrtazz commented Mar 1, 2013

@jsocol let me know if you need help with anything on that.

James Socol and others added some commits Apr 9, 2012
@jsocol James Socol Add support for gauge deltas.
* Add tests for gauges and new gauge deltas.
* Add support for +/- prefix for gauge values to support modifying
  gauge values in-place.
3eecd18
@jsocol jsocol Remove numStats asserts. c583fc3
Contributor
jsocol commented Mar 1, 2013

@mrtazz I removed two asserts about numStats, rebased to master, and everything's passing now. I can squash these together--or make any other changes you'd like.

Owner
mrtazz commented Mar 1, 2013

sweet, thanks so much!

@mrtazz mrtazz merged commit bc52dad into etsy:master Mar 1, 2013

1 check passed

default The Travis build passed
Details
diegovar commented Mar 1, 2013

How do we use these gauge deltas?

Contributor
jsocol commented Mar 1, 2013

@diegovar Send "+1|g" or "-1|g". If there's a sign, and the gauge value already exists, statsd will add/subtract to the value instead of replacing it.

@jsocol How can it differentiate a negative delta from a negative gauge value? Or maybe there can be no negative gauge values?

diegovar commented Mar 1, 2013

If the gauge doesn't exist, is it initialized as 0?

Owner
mrtazz commented Mar 1, 2013

good point, @jsocol can you check that and maybe also update the README?

Contributor
jsocol commented Mar 1, 2013

@mfontenele That was why I changed it to a distinct type at one point, but @mrtazz asked above to change it back. With this change, you can't explicitly set a gauge to a negative value.

@diegovar If it doesn't exist, it will treat it like 0 and then add or subtract (e.g. setting a new gauge to "-3" will set it to "0 - 3").

@mrtazz I can update the readme, sure.

Contributor
jsocol commented Mar 1, 2013

In pull #256.

Contributor

Closing since #256 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment