Permalink
Browse files

don't show Bad Line warning for empty strings

  • Loading branch information...
1 parent c947f66 commit 2c6503400f52866d04c2ecdbb6c8b766e82f4054 @mrtazz mrtazz committed Mar 30, 2013
Showing with 3 additions and 0 deletions.
  1. +3 −0 stats.js
View
@@ -163,6 +163,9 @@ config.configFile(process.argv[2], function (config, oldConfig) {
}
for (var midx in metrics) {
+ if (metrics[midx].length == 0) {
+ continue;
+ }
if (config.dumpMessages) {
l.log(metrics[midx].toString());
}

7 comments on commit 2c65034

Contributor

draco2003 replied Mar 30, 2013

Should that be ===?

Owner

mrtazz replied Mar 31, 2013

I don't think it matters for the 0 comparison, but we can change it.

Contributor

Dieterbe replied Apr 1, 2013

why this? why just ignore empty strings?

Owner

mrtazz replied Apr 1, 2013

what should we do instead? We had #143 open forever and echo "foo.bar:1|c" | nc has been the canonical way of getting data into StatsD since the beginning. Chances are, if you're sending malformed packets, you are not sending an empty string but something else.

Contributor

Dieterbe replied Apr 1, 2013

ok, read through #143 -- the shell client has this fixed (by using printf/echo -n) since a long time already.
If we tell users to use echo we should change that to echo -n (but we don't seem to do that anywhere)
if anyone still complains about bogus errors, they should just update their clients.

the benefit of reporting correct errors when a client is accidentially sending empty lines far outweights (IMHO) the "benefit" of hiding errors for users who haven't updated their clients. we can also just mention this in the changelog, that way users have no excuse.

Contributor

mheffner replied Apr 1, 2013

It's extremely annoying that a newline terminated input doesn't work, eg. echo 'foo.bar:1|c' | nc. Stripping trailing newlines is common place, especially when you already support "foo.bar:1|c\nfoo.bar2:1|c". So 👍 on fixing this.

That being said, I could see arguments of whether echo | nc or printf "\n" | nc should be allowed (ie, no metric at all).

Contributor

Dieterbe replied Apr 4, 2013

That being said, I could see arguments of whether echo | nc or printf "\n" | nc should be allowed (ie, no metric at all).

o_O
(btw hi mike!)

Please sign in to comment.