Constant tags; compatibility with non-datadog recordExecutionTime

- Add support for initialization-time constant tag list (similar use
  case to constant prefix).

- Convert millisecond times to floating-point histogram type for
  compatibility with upstream semantics.

Author: charles-dyfis-net

README.md

@@ -26,15 +26,26 @@ import com.timgroup.statsd.StatsDClient;
 import com.timgroup.statsd.NonBlockingStatsDClient;
 
 public class Foo {
-  private static final StatsDClient statsd = new NonBlockingStatsDClient("my.prefix", "statsd-host", 8125);
+
+  private static final StatsDClient statsd = new NonBlockingStatsDClient(
+    "my.prefix",                          /* prefix to any stats; may be null or empty string */
+    "statsd-host",                        /* common case: localhost */
+    8125,                                 /* port */
+    new String[] {"tag:value"}            /* DataDog extension: Constant tags, always applied */
+  );
 
   public static final void main(String[] args) {
     statsd.incrementCounter("foo");
     statsd.recordGaugeValue("bar", 100);
     statsd.recordGaugeValue("baz", 0.01); /* DataDog extension: support for floating-point gauges */
+    statsd.recordHistogram("qux", 15)     /* DataDog extension: histograms */
+    statsd.recordHistogram("qux", 15.5)   /* ...also floating-point */
+
+    /* Compatibility note: Unlike upstream statsd, DataDog expects execution times to be a
+     * floating-point value in seconds, not a millisecond value. This library
+     * does the conversion from ms to fractional seconds.
+     */
     statsd.recordExecutionTime("bag", 25, "cluster:foo"); /* DataDog extension: cluster tag */
-    statsd.recordHistogram("qux", 15)   /* DataDog extension: histograms */
-    statsd.recordHistogram("qux", 15.5) /* ...also floating-point */
   }
 }
 ```

src/main/java/com/timgroup/statsd/NonBlockingStatsDClient.java

@@ -52,6 +52,7 @@ public final class NonBlockingStatsDClient implements StatsDClient {
     private final String prefix;
     private final DatagramSocket clientSocket;
     private final StatsDClientErrorHandler handler;
+    private final String[] constantTags;
 
     private final ExecutorService executor = Executors.newSingleThreadExecutor(new ThreadFactory() {
         final ThreadFactory delegate = Executors.defaultThreadFactory();
@@ -71,7 +72,7 @@ public final class NonBlockingStatsDClient implements StatsDClient {
      * be established. Once a client has been instantiated in this way, all
      * exceptions thrown during subsequent usage are consumed, guaranteeing
      * that failures in metrics will not affect normal code execution.
-     * 
+     *
      * @param prefix
      *     the prefix to apply to keys sent via this client
      * @param hostname
@@ -82,9 +83,34 @@ public final class NonBlockingStatsDClient implements StatsDClient {
      *     if the client could not be started
      */
     public NonBlockingStatsDClient(String prefix, String hostname, int port) throws StatsDClientException {
-        this(prefix, hostname, port, NO_OP_HANDLER);
+        this(prefix, hostname, port, null, NO_OP_HANDLER);
+    }
+
+    /**
+     * Create a new StatsD client communicating with a StatsD instance on the
+     * specified host and port. All messages send via this client will have
+     * their keys prefixed with the specified string. The new client will
+     * attempt to open a connection to the StatsD server immediately upon
+     * instantiation, and may throw an exception if that a connection cannot
+     * be established. Once a client has been instantiated in this way, all
+     * exceptions thrown during subsequent usage are consumed, guaranteeing
+     * that failures in metrics will not affect normal code execution.
+     * 
+     * @param prefix
+     *     the prefix to apply to keys sent via this client
+     * @param hostname
+     *     the host name of the targeted StatsD server
+     * @param port
+     *     the port of the targeted StatsD server
+     * @param constantTags
+     *     tags to be added to all content sent
+     * @throws StatsDClientException
+     *     if the client could not be started
+     */
+    public NonBlockingStatsDClient(String prefix, String hostname, int port, String[] constantTags) throws StatsDClientException {
+        this(prefix, hostname, port, constantTags, NO_OP_HANDLER);
     }
-    
+
     /**
      * Create a new StatsD client communicating with a StatsD instance on the
      * specified host and port. All messages send via this client will have
@@ -102,19 +128,25 @@ public NonBlockingStatsDClient(String prefix, String hostname, int port) throws
      *     the host name of the targeted StatsD server
      * @param port
      *     the port of the targeted StatsD server
+     * @param constantTags
+     *     tags to be added to all content sent
      * @param errorHandler
      *     handler to use when an exception occurs during usage
      * @throws StatsDClientException
      *     if the client could not be started
      */
-    public NonBlockingStatsDClient(String prefix, String hostname, int port, StatsDClientErrorHandler errorHandler) throws StatsDClientException {
+    public NonBlockingStatsDClient(String prefix, String hostname, int port, String[] constantTags, StatsDClientErrorHandler errorHandler) throws StatsDClientException {
         if(prefix != null && prefix.length() > 0) {
             this.prefix = String.format("%s.", prefix);
         } else {
             this.prefix = "";
         }
         this.handler = errorHandler;
-        
+        if(constantTags != null && constantTags.length == 0) {
+            constantTags = null;
+        }
+        this.constantTags = constantTags;
+
         try {
             this.clientSocket = new DatagramSocket();
             this.clientSocket.connect(new InetSocketAddress(hostname, port));
@@ -147,14 +179,26 @@ public void stop() {
      * Generate a suffix conveying the given tag list to the client
      */
     String tagString(String[] tags) {
-        if(tags == null || tags.length == 0) {
+        boolean have_call_tags = (tags != null && tags.length > 0);
+        boolean have_constant_tags = (constantTags != null && constantTags.length > 0);
+        if(!have_call_tags && !have_constant_tags) {
             return "";
         }
         StringBuilder sb = new StringBuilder("|#");
-        for(int n=tags.length - 1; n>=0; n--) {
-            sb.append(tags[n]);
-            if(n > 0) {
-                sb.append(",");
+        if(have_constant_tags) {
+            for(int n=constantTags.length - 1; n>=0; n--) {
+                sb.append(constantTags[n]);
+                if(n > 0 || have_call_tags) {
+                    sb.append(",");
+                }
+            }
+        }
+        if (have_call_tags) {
+            for(int n=tags.length - 1; n>=0; n--) {
+                sb.append(tags[n]);
+                if(n > 0) {
+                    sb.append(",");
+                }
             }
         }
         return sb.toString();
@@ -290,7 +334,7 @@ public void gauge(String aspect, int value, String... tags) {
      */
     @Override
     public void recordExecutionTime(String aspect, long timeInMs, String... tags) {
-        send(String.format("%s%s:%d|ms%s", prefix, aspect, timeInMs, tagString(tags)));
+        recordHistogramValue(aspect, (timeInMs * 0.001), tags);
     }
 
     /**

src/test/java/com/timgroup/statsd/NonBlockingStatsDClientTest.java

@@ -36,7 +36,7 @@ public void stop() throws Exception {
     sends_counter_value_to_statsd_with_null_tags() throws Exception {
         final DummyStatsDServer server = new DummyStatsDServer(STATSD_SERVER_PORT);
 
-        client.count("mycount", 24, null);
+        client.count("mycount", 24, (java.lang.String[]) null);
         server.waitForMessage();
 
         assertThat(server.messagesReceived(), contains("my.prefix.mycount:24|c"));
@@ -209,7 +209,7 @@ public void stop() throws Exception {
         client.recordExecutionTime("mytime", 123);
         server.waitForMessage();
 
-        assertThat(server.messagesReceived(), contains("my.prefix.mytime:123|ms"));
+        assertThat(server.messagesReceived(), contains("my.prefix.mytime:0.123|h"));
     }
 
     @Test(timeout=5000L) public void
@@ -219,10 +219,30 @@ public void stop() throws Exception {
         client.recordExecutionTime("mytime", 123, "foo:bar", "baz");
         server.waitForMessage();
 
-        assertThat(server.messagesReceived(), contains("my.prefix.mytime:123|ms|#baz,foo:bar"));
+        assertThat(server.messagesReceived(), contains("my.prefix.mytime:0.123|h|#baz,foo:bar"));
     }
 
 
+    @Test(timeout=5000L) public void
+    sends_gauge_mixed_tags() throws Exception {
+        final DummyStatsDServer server = new DummyStatsDServer(STATSD_SERVER_PORT);
+        final NonBlockingStatsDClient empty_prefix_client = new NonBlockingStatsDClient("my.prefix", "localhost", STATSD_SERVER_PORT, new String[] {"instance:foo", "app:bar"});
+        empty_prefix_client.gauge("value", 423, "baz");
+        server.waitForMessage();
+
+        assertThat(server.messagesReceived(), contains("my.prefix.value:423|g|#app:bar,instance:foo,baz"));
+    }
+
+    @Test(timeout=5000L) public void
+    sends_gauge_constant_tags_only() throws Exception {
+        final DummyStatsDServer server = new DummyStatsDServer(STATSD_SERVER_PORT);
+        final NonBlockingStatsDClient empty_prefix_client = new NonBlockingStatsDClient("my.prefix", "localhost", STATSD_SERVER_PORT, new String[] {"instance:foo", "app:bar"});
+        empty_prefix_client.gauge("value", 423);
+        server.waitForMessage();
+
+        assertThat(server.messagesReceived(), contains("my.prefix.value:423|g|#app:bar,instance:foo"));
+    }
+
     @Test(timeout=5000L) public void
     sends_gauge_empty_prefix() throws Exception {
         final DummyStatsDServer server = new DummyStatsDServer(STATSD_SERVER_PORT);