node-statsd
node-statsd copied to clipboard
increment(0) == increment(1)
I know I shouldn't have called increment(0) but let me tell you it took a long time to find that bug !
Because increment (and decrement) compare delta to 0/null and not to undefined
StatsDClient.prototype.increment = function (name, delta) {
this.counter(name, Math.abs(delta || 1));
};
could become :
StatsDClient.prototype.increment = function (name, delta) {
this.counter(name, Math.abs(delta!=undefined?delta: 1));
};
or
StatsDClient.prototype.increment = function (name, delta) {
if (delta==0) then return; // no op
this.counter(name, Math.abs(delta || 1));
};
Why was it hard to figure it out:
- my code could have been the problem (triple checked that)
- custom statsd -> influxdb connector (which I thought was faulty)
- influxdb could have been the problem too (checked that)
- statsd could have been the problem (checked that too)
I think you could document that special case... or allow delta == 0
Many thanks for all your hard work anyway!
+1 on this one, we doubled and trippel-checked our implementation until we looked at the source and realized that it did value || 1
and defaulted 0 into 1. It's kind of nice to not require you to write programming logic to avoid sending stats if you always have a counter that spans between 0 and infinity.
I just opened #65, but I'm not certain on whether it'll get merged or not considering the staleness of this repo. You guys can feel free to point to my fork if you want.
@todd No updates have been made here for quite awhile, so I've been keeping hot-shots up to date as an alternative to this. It included a fix for this issue awhile ago: https://github.com/brightcove/hot-shots/pull/13
@bdeitte Thanks for the heads up. I'll give your package a whirl.