Fix a bug in address resolution order

vickenty · vickenty · commit 87832f03f347 · 2023-03-08T14:55:33.000+01:00
Before this patch, named pipe from the environment would be used even
if the application supplies host and port pair explicitly.

Make sure the builder uses configuration in this order:
1. explicitly supplied by the application via builder methods
2. auto-detected settings from the environment variables
3. built-in defaults
diff --git a/src/main/java/com/timgroup/statsd/NonBlockingStatsDClientBuilder.java b/src/main/java/com/timgroup/statsd/NonBlockingStatsDClientBuilder.java
@@ -207,18 +207,7 @@ protected NonBlockingStatsDClientBuilder resolve() {
         }
 
         int packetSize = maxPacketSizeBytes;
-        Callable<SocketAddress> lookup = addressLookup;
-
-        if (lookup == null) {
-            String namedPipeFromEnv = System.getenv(NonBlockingStatsDClient.DD_NAMED_PIPE_ENV_VAR);
-            String resolvedNamedPipe = namedPipe == null ? namedPipeFromEnv : namedPipe;
-            
-            if (resolvedNamedPipe == null) {
-                lookup = staticStatsDAddressResolution(hostname, port);
-            } else {
-                lookup = staticNamedPipeResolution(resolvedNamedPipe);
-            }
-        }
+        Callable<SocketAddress> lookup = getAddressLookup();
 
         if (packetSize == 0) {
             packetSize = (port == 0) ? NonBlockingStatsDClient.DEFAULT_UDS_MAX_PACKET_SIZE_BYTES :
@@ -230,7 +219,7 @@ protected NonBlockingStatsDClientBuilder resolve() {
             if (telemetryHostname == null) {
                 telemetryLookup = lookup;
             } else {
-                telemetryLookup = staticStatsDAddressResolution(telemetryHostname, telemetryPort);
+                telemetryLookup = staticAddress(telemetryHostname, telemetryPort);
             }
         }
 
@@ -241,6 +230,32 @@ protected NonBlockingStatsDClientBuilder resolve() {
         return resolved;
     }
 
+    private Callable<SocketAddress> getAddressLookup() {
+        // First, use explicit configuration on the builder.
+        if (addressLookup != null) {
+            return addressLookup;
+        }
+
+        if (namedPipe != null) {
+            return staticNamedPipeResolution(namedPipe);
+        }
+
+        if (hostname != null) {
+            return staticAddress(hostname, port);
+        }
+
+        // Next, try various environment variables.
+        String namedPipeFromEnv = System.getenv(NonBlockingStatsDClient.DD_NAMED_PIPE_ENV_VAR);
+        if (namedPipeFromEnv != null) {
+            return staticNamedPipeResolution(namedPipeFromEnv);
+        }
+
+        String hostFromEnv = getHostnameFromEnvVar();
+        int portFromEnv = getPortFromEnvVar(port);
+
+        return staticAddress(hostFromEnv, portFromEnv);
+    }
+
     /**
      * Create dynamic lookup for the given host name and port.
      *
@@ -288,20 +303,6 @@ public static Callable<SocketAddress> staticAddressResolution(final String hostn
         };
     }
 
-    protected static Callable<SocketAddress> staticStatsDAddressResolution(String hostname, int port)
-            throws StatsDClientException {
-        try {
-            if (hostname == null) {
-                hostname = getHostnameFromEnvVar();
-                port = getPortFromEnvVar(port);
-            }
-
-            return staticAddressResolution(hostname, port);
-        } catch (final Exception e) {
-            throw new StatsDClientException("Failed to lookup StatsD host", e);
-        }
-    }
-
     protected static Callable<SocketAddress> staticNamedPipeResolution(String namedPipe) {
         final NamedPipeSocketAddress socketAddress = new NamedPipeSocketAddress(namedPipe);
         return new Callable<SocketAddress>() {
@@ -311,6 +312,14 @@ protected static Callable<SocketAddress> staticNamedPipeResolution(String namedP
         };
     }
 
+    private static Callable<SocketAddress> staticAddress(final String hostname, final int port) {
+        try {
+            return staticAddressResolution(hostname, port);
+        } catch (Exception e) {
+            throw new StatsDClientException("Failed to lookup StatsD host", e);
+        }
+    }
+
     /**
      * Retrieves host name from the environment variable "DD_AGENT_HOST".
      *
