NonBlockingStatsDClient handles empty or null prefixes poorly

[gaffonso]

The NonBlockingStatsDClient implementation requires a "prefix" in its constructor. It then uses this prefix in several places, here's one example:

public void count(String aspect, int delta) {
    send(String.format("%s.%s:%d|c", prefix, aspect, delta));
}

This is problematic for us as our keys already have the "prefix" baked in, we don't need prefixes added by the client (it's actually problematic in our case).

The way the count(...) (and other) code is written, instantiating the NonBlockingStatsDClient with an empty string, yields keys with just a "." prefixed to them...

org.junit.ComparisonFailure: 
Expected :'preva.FUNCTIONAL_TEST.my.logins.result.valid-login.rate-per-s:1|c'
Actual   :'.preva.FUNCTIONAL_TEST.my.logins.result.valid-login.rate-per-s:1|c'

...and instantiating the NonBlockingStatsDClient with a null string, yields keys with a "null." prefixed to them:

org.junit.ComparisonFailure: 
Expected :'preva.FUNCTIONAL_TEST.my.logins.result.valid-login.rate-per-s:1|c'
Actual   :'null.preva.FUNCTIONAL_TEST.my.logins.result.valid-login.rate-per-s:1|c'

Would you be interested in a pull-request that modified the behavior? Perhaps simply a conditional that checked for empty or null prefix-strings and "did the right thing" in those cases?

For now we can subclass the 3 methods that send(...) data. It's an easy override. But we'd very much prefer to use the "core" library without subclass customizations.

Thanks!

Gary

[gaffonso]

Ahh, poop. Just saw that NonBlockingStatsDClient is "final". So no subclassing. I can patch locally and release (for our own use) a customized version of the library. But that's starting to be an uncomfortable deviation for our team (we vastly prefer maven-hosted "release" libs).

Look forward to your reply regarding the pull request (and a new maven release).

[scarytom]

Apologies for leaving this dormant for so long. I've just pushed a fix that allows empty or null prefixes to be handled sensibly.