Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add support for gauge deltas. #72

Merged
merged 2 commits into from

8 participants

James Socol Daniel Schauenberg Don't Add Me To Your Organization a.k.a The Travis Bot Dieter Plaetinck NachoSoto Diego Varese Marcelo Fontenele da Silva Santos Dan Rowe
James Socol
  • 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?

Daniel Schauenberg
Owner

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.

James Socol

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.

Don't Add Me To Your Organization a.k.a The Travis Bot

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

James Socol

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.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged eb7d650 into d1368ed).

James Socol

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.

James Socol

Also includes tests for gauges and gauge deltas/modifications.

Daniel Schauenberg
Owner

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

Deleted user

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?

James Socol

@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.

Deleted user

@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 ;)

Dieter Plaetinck

@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?

Daniel Schauenberg
Owner

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?

NachoSoto

I would love to see this merged in

James Socol

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

Diego Varese

I'd love this functionality as well

James Socol

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

Daniel Schauenberg
Owner

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

James Socol and others added some commits
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
James Socol jsocol Remove numStats asserts. c583fc3
James Socol

@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.

Daniel Schauenberg
Owner

sweet, thanks so much!

Daniel Schauenberg mrtazz merged commit bc52dad into from
Diego Varese

How do we use these gauge deltas?

James Socol

@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.

Marcelo Fontenele da Silva Santos

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

Diego Varese

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

Daniel Schauenberg
Owner

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

James Socol

@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.

James Socol

In pull #256.

Dan Rowe

Closing since #256 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 1, 2013
  1. James Socol

    Add support for gauge deltas.

    James Socol authored jsocol committed
    * Add tests for gauges and new gauge deltas.
    * Add support for +/- prefix for gauge values to support modifying
      gauge values in-place.
  2. James Socol

    Remove numStats asserts.

    jsocol authored
This page is out of date. Refresh to see the latest.
Showing with 65 additions and 1 deletion.
  1. +5 −1 stats.js
  2. +60 −0 test/graphite_tests.js
6 stats.js
View
@@ -191,7 +191,11 @@ config.configFile(process.argv[2], function (config, oldConfig) {
timers[key].push(Number(fields[0] || 0));
timer_counters[key] += (1 / sampleRate);
} else if (fields[1].trim() == "g") {
- gauges[key] = Number(fields[0] || 0);
+ if (gauges[key] && fields[0].match(/^[-+]/)) {
+ gauges[key] += Number(fields[0] || 0);
+ } else {
+ gauges[key] = Number(fields[0] || 0);
+ }
} else if (fields[1].trim() == "s") {
if (! sets[key]) {
sets[key] = new set.Set();
60 test/graphite_tests.js
View
@@ -298,5 +298,65 @@ module.exports = {
});
});
});
+ },
+
+ gauges_are_valid: function(test) {
+ test.expect(2);
+
+ var testvalue = 70;
+ var me = this;
+ this.acceptor.once('connection', function(c) {
+ statsd_send('a_test_value:' + testvalue + '|g', me.sock, '127.0.0.1', 8125, function() {
+ collect_for(me.acceptor, me.myflush*2, function(strings) {
+ test.ok(strings.length > 0, 'should receive some data');
+ var hashes = _.map(strings, function(x) {
+ var chunks = x.split(' ');
+ var data = {};
+ data[chunks[0]] = chunks[1];
+ return data;
+ });
+
+ var gaugevalue_test = function(post) {
+ var mykey = 'stats.gauges.a_test_value';
+ return _.include(_.keys(post), mykey) && (post[mykey] == testvalue);
+ };
+ test.ok(_.any(hashes, gaugevalue_test), 'stats.gauges.a_test_value should be ' + testvalue);
+
+ test.done();
+ });
+ });
+ });
+ },
+
+ gauge_modifications_are_valid: function(test) {
+ test.expect(2);
+
+ var teststartvalue = 50;
+ var testdeltavalue = '-3';
+ var testresult = teststartvalue + Number(testdeltavalue);
+ var me = this;
+ this.acceptor.once('connection', function(c) {
+ statsd_send('test_value:' + teststartvalue + '|g', me.sock, '127.0.0.1', 8125, function() {
+ statsd_send('test_value:' + testdeltavalue + '|g', me.sock, '127.0.0.1', 8125, function() {
+ collect_for(me.acceptor, me.myflush * 2, function(strings) {
+ test.ok(strings.length > 0, 'should receive some data');
+ var hashes = _.map(strings, function(x) {
+ var chunks = x.split(' ');
+ var data = {};
+ data[chunks[0]] = chunks[1];
+ return data;
+ });
+
+ var gaugevalue_test = function(post) {
+ var mykey = 'stats.gauges.test_value';
+ return _.include(_.keys(post), mykey) && (post[mykey] == testresult);
+ };
+ test.ok(_.any(hashes, gaugevalue_test), 'stats.gauges.test_value should be ' + testresult);
+
+ test.done();
+ });
+ });
+ });
+ });
}
}
Something went wrong with that request. Please try again.