Isolate tests from system environment

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

@@ -0,0 +1,19 @@
+package com.timgroup.statsd;
+
+import java.util.Map;
+
+class EnvMap {
+    private final Map<String, String> env;
+
+    EnvMap() {
+        env = null;
+    }
+
+    EnvMap(Map<String, String> provided) {
+        env = provided;
+    }
+
+    String get(String name) {
+        return env != null ? env.get(name) : System.getenv(name);
+    }
+}

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

@@ -76,10 +76,6 @@ String envName() {
             return (PREFIX + "_" + toString()).toUpperCase();
         }
 
-        String envVal() {
-            return System.getenv(envName());
-        }
-
         String tag() {
             return toString().toLowerCase();
         }
@@ -182,6 +178,7 @@ protected static String format(ThreadLocal<NumberFormat> formatter, Number value
     private final String containerID;
     private final String externalEnv;
     final TagsCardinality clientTagsCardinality;
+    final EnvMap env;
 
     /**
      * Create a new StatsD client communicating with a StatsD instance on the host and port
@@ -209,6 +206,7 @@ public NonBlockingStatsDClient(final NonBlockingStatsDClientBuilder builder)
         blocking = builder.blocking;
         maxPacketSizeBytes = builder.maxPacketSizeBytes;
         clientTagsCardinality = builder.tagsCardinality;
+        env = builder.env;
 
         {
             List<String> constantPreTags = new ArrayList<>();
@@ -220,7 +218,7 @@ public NonBlockingStatsDClient(final NonBlockingStatsDClientBuilder builder)
             // Support "dd.internal.entity_id" internal tag.
             updateTagsWithEntityID(constantPreTags, builder.entityID);
             for (final Literal literal : Literal.values()) {
-                final String envVal = literal.envVal();
+                final String envVal = env.get(literal.envName());
                 if (envVal != null && !envVal.trim().isEmpty()) {
                     constantPreTags.add(literal.tag() + ":" + envVal);
                 }
@@ -240,7 +238,7 @@ public NonBlockingStatsDClient(final NonBlockingStatsDClientBuilder builder)
 
         boolean originDetectionEnabled = isOriginDetectionEnabled(builder.originDetectionEnabled);
         containerID = getContainerID(builder.containerID, originDetectionEnabled);
-        externalEnv = originDetectionEnabled ? Utf8.sanitize(System.getenv("DD_EXTERNAL_ENV")) : "";
+        externalEnv = originDetectionEnabled ? Utf8.sanitize(env.get("DD_EXTERNAL_ENV")) : "";
 
         try {
             clientChannel =
@@ -1521,11 +1519,11 @@ public boolean writeTo(StringBuilder sb, int capacity) {
      * @param entityID the entityID string provided by argument
      * @return true if tags was modified
      */
-    private static boolean updateTagsWithEntityID(final List<String> tags, String entityID) {
+    private boolean updateTagsWithEntityID(final List<String> tags, String entityID) {
         // Support "dd.internal.entity_id" internal tag.
         if (entityID == null || entityID.trim().isEmpty()) {
             // if the entityID parameter is null, default to the environment variable
-            entityID = System.getenv(DD_ENTITY_ID_ENV_VAR);
+            entityID = env.get(DD_ENTITY_ID_ENV_VAR);
         }
         if (entityID != null && !entityID.trim().isEmpty()) {
             final String entityTag = ENTITY_ID_TAG_NAME + ":" + entityID;
@@ -1591,14 +1589,14 @@ protected boolean isInvalidSample(double sampleRate) {
         return sampleRate != 1 && ThreadLocalRandom.current().nextDouble() > sampleRate;
     }
 
-    static boolean isOriginDetectionEnabled(boolean originDetectionEnabled) {
+    boolean isOriginDetectionEnabled(boolean originDetectionEnabled) {
         if (!originDetectionEnabled) {
             // origin detection is explicitly disabled
             // or a user-defined container ID was provided
             return false;
         }
 
-        String value = System.getenv(ORIGIN_DETECTION_ENABLED_ENV_VAR);
+        String value = env.get(ORIGIN_DETECTION_ENABLED_ENV_VAR);
         value = value != null ? value.trim() : null;
         if (value != null && !value.isEmpty()) {
             return !Arrays.asList("no", "false", "0", "n", "off").contains(value.toLowerCase());

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

@@ -5,6 +5,7 @@
 import java.net.SocketAddress;
 import java.net.URI;
 import java.net.UnknownHostException;
+import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ThreadFactory;
 import jnr.unixsocket.UnixSocketAddress;
@@ -122,6 +123,8 @@ public class NonBlockingStatsDClientBuilder implements Cloneable {
     public ThreadFactory threadFactory;
     public TagsCardinality tagsCardinality = null;
 
+    EnvMap env = new EnvMap();
+
     public NonBlockingStatsDClientBuilder() {}
 
     public NonBlockingStatsDClientBuilder port(int val) {
@@ -334,6 +337,22 @@ public NonBlockingStatsDClientBuilder tagsCardinality(TagsCardinality cardinalit
         return this;
     }
 
+    /**
+     * Overrides the environment variables the client reads configuration from.
+     *
+     * <p>By default the client reads from the process environment via {@link
+     * System#getenv(String)}. Supplying a map makes the client read configuration (for example,
+     * {@code DD_ENV}, {@code DD_SERVICE}, {@code DD_VERSION}, {@code DD_ENTITY_ID}) exclusively
+     * from that map, so it is unaffected by the ambient process environment.
+     *
+     * @param env the map to read environment variables from, or null to use the process environment
+     * @return this builder
+     */
+    public NonBlockingStatsDClientBuilder withEnvironmentVariables(Map<String, String> env) {
+        this.env = new EnvMap(env);
+        return this;
+    }
+
     /**
      * NonBlockingStatsDClient factory method.
      *
@@ -386,11 +405,10 @@ protected NonBlockingStatsDClientBuilder resolve() {
 
         resolved.tagsCardinality = this.tagsCardinality;
         if (resolved.tagsCardinality == null) {
-            resolved.tagsCardinality = TagsCardinality.fromString(System.getenv("DD_CARDINALITY"));
+            resolved.tagsCardinality = TagsCardinality.fromString(env.get("DD_CARDINALITY"));
         }
         if (resolved.tagsCardinality == null) {
-            resolved.tagsCardinality =
-                    TagsCardinality.fromString(System.getenv("DATADOG_CARDINALITY"));
+            resolved.tagsCardinality = TagsCardinality.fromString(env.get("DATADOG_CARDINALITY"));
         }
         if (resolved.tagsCardinality == null) {
             resolved.tagsCardinality = TagsCardinality.DEFAULT;
@@ -414,12 +432,12 @@ private Callable<SocketAddress> getAddressLookup() {
         }
 
         // Next, try various environment variables.
-        String url = System.getenv(NonBlockingStatsDClient.DD_DOGSTATSD_URL_ENV_VAR);
+        String url = env.get(NonBlockingStatsDClient.DD_DOGSTATSD_URL_ENV_VAR);
         if (url != null) {
             return getAddressLookupFromUrl(url);
         }
 
-        String namedPipeFromEnv = System.getenv(NonBlockingStatsDClient.DD_NAMED_PIPE_ENV_VAR);
+        String namedPipeFromEnv = env.get(NonBlockingStatsDClient.DD_NAMED_PIPE_ENV_VAR);
         if (namedPipeFromEnv != null) {
             return staticNamedPipeResolution(namedPipeFromEnv);
         }
@@ -543,8 +561,8 @@ private static Callable<SocketAddress> staticAddress(final String hostname, fina
      * @return host name from the environment variable "DD_AGENT_HOST"
      * @throws StatsDClientException if the environment variable is not set
      */
-    private static String getHostnameFromEnvVar() {
-        final String hostname = System.getenv(NonBlockingStatsDClient.DD_AGENT_HOST_ENV_VAR);
+    private String getHostnameFromEnvVar() {
+        final String hostname = env.get(NonBlockingStatsDClient.DD_AGENT_HOST_ENV_VAR);
         if (hostname == null) {
             throw new StatsDClientException(
                     "Failed to retrieve agent hostname from environment variable", null);
@@ -558,9 +576,8 @@ private static String getHostnameFromEnvVar() {
      * @return dogstatsd port from the environment variable "DD_DOGSTATSD_PORT"
      * @throws StatsDClientException if the environment variable is an integer
      */
-    private static int getPortFromEnvVar(final int defaultPort) {
-        final String statsDPortString =
-                System.getenv(NonBlockingStatsDClient.DD_DOGSTATSD_PORT_ENV_VAR);
+    private int getPortFromEnvVar(final int defaultPort) {
+        final String statsDPortString = env.get(NonBlockingStatsDClient.DD_DOGSTATSD_PORT_ENV_VAR);
         if (statsDPortString == null) {
             return defaultPort;
         } else {

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

@@ -6,6 +6,7 @@
 import static org.hamcrest.Matchers.startsWith;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.List;
 import org.junit.After;
 import org.junit.Before;
@@ -20,6 +21,7 @@ public void start() throws IOException {
         server = new UDPDummyStatsDServer(0);
         testClient =
                 new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(new HashMap<String, String>())
                         .prefix("my.prefix")
                         .hostname("localhost")
                         .port(server.getPort())

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

@@ -7,18 +7,20 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
 import jnr.unixsocket.UnixSocketAddress;
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.contrib.java.lang.system.EnvironmentVariables;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
 
 @RunWith(Parameterized.class)
 public class BuilderAddressTest {
-    @Rule public final EnvironmentVariables environmentVariables = new EnvironmentVariables();
+
+    // Controlled environment so address resolution only sees the variables this test sets.
+    Map<String, String> env;
 
     final String url;
     final String host;
@@ -187,6 +189,7 @@ public String getPath() {
 
     @Before
     public void set() {
+        env = new HashMap<>();
         set(NonBlockingStatsDClient.DD_DOGSTATSD_URL_ENV_VAR, url);
         set(NonBlockingStatsDClient.DD_AGENT_HOST_ENV_VAR, host);
         set(NonBlockingStatsDClient.DD_DOGSTATSD_PORT_ENV_VAR, port);
@@ -195,9 +198,7 @@ public void set() {
 
     void set(String name, String val) {
         if (val != null) {
-            environmentVariables.set(name, val);
-        } else {
-            environmentVariables.clear(name);
+            env.put(name, val);
         }
     }
 
@@ -206,7 +207,7 @@ public void address_resolution() throws Exception {
         NonBlockingStatsDClientBuilder b;
 
         // Default configuration matches env vars
-        b = new NonBlockingStatsDClientBuilder().resolve();
+        b = new NonBlockingStatsDClientBuilder().withEnvironmentVariables(env).resolve();
         SocketAddress actual = b.addressLookup.call();
 
         // Make it possible to run this code even if we don't have jnr-unixsocket.
@@ -222,13 +223,26 @@ public void address_resolution() throws Exception {
         }
 
         // Explicit configuration is used regardless of environment variables.
-        b = new NonBlockingStatsDClientBuilder().hostname("2.2.2.2").resolve();
+        b =
+                new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(env)
+                        .hostname("2.2.2.2")
+                        .resolve();
         assertEquals(new InetSocketAddress("2.2.2.2", defaultPort), b.addressLookup.call());
 
-        b = new NonBlockingStatsDClientBuilder().hostname("2.2.2.2").port(2222).resolve();
+        b =
+                new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(env)
+                        .hostname("2.2.2.2")
+                        .port(2222)
+                        .resolve();
         assertEquals(new InetSocketAddress("2.2.2.2", 2222), b.addressLookup.call());
 
-        b = new NonBlockingStatsDClientBuilder().namedPipe("ook").resolve();
+        b =
+                new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(env)
+                        .namedPipe("ook")
+                        .resolve();
         assertEquals(new NamedPipeSocketAddress("ook"), b.addressLookup.call());
     }
 }

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

@@ -5,12 +5,12 @@
 import static org.hamcrest.Matchers.hasItem;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.logging.Logger;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.contrib.java.lang.system.EnvironmentVariables;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
@@ -45,13 +45,14 @@ public static Object[][] parameters() {
                 });
     }
 
-    @Rule public final EnvironmentVariables environmentVariables = new EnvironmentVariables();
-
     String clientTagsCardinality;
     Case msgTagsCardinality;
     UDPDummyStatsDServer server;
     NonBlockingStatsDClient client;
 
+    // Controlled environment so the client is unaffected by ambient DD_* variables.
+    Map<String, String> clientEnv;
+
     public CardinalityOverrideTest(String clientTagsCardinality, Case msgTagsCardinality) {
         log.info(String.format("%s %s", clientTagsCardinality, msgTagsCardinality.value));
         this.clientTagsCardinality = clientTagsCardinality;
@@ -75,12 +76,14 @@ private void assertPayload(final String payloadHead) {
 
     @Before
     public void start() throws IOException {
+        clientEnv = new HashMap<>();
         if (clientTagsCardinality != null) {
-            environmentVariables.set("DD_CARDINALITY", clientTagsCardinality);
+            clientEnv.put("DD_CARDINALITY", clientTagsCardinality);
         }
         server = new UDPDummyStatsDServer(0);
         client =
                 new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(clientEnv)
                         .prefix("my.prefix")
                         .hostname("localhost")
                         .port(server.getPort())

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

@@ -5,6 +5,7 @@
 import static org.hamcrest.Matchers.nullValue;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.Random;
 import java.util.logging.Logger;
 import org.junit.After;
@@ -42,6 +43,7 @@ public void start() {
         server = new NamedPipeDummyStatsDServer(pipeName);
         client =
                 new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(new HashMap<String, String>())
                         .prefix("my.prefix")
                         .namedPipe(pipeName)
                         .queueSize(1)

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

@@ -5,13 +5,13 @@
 import static org.hamcrest.Matchers.hasItem;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.FixMethodOrder;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.contrib.java.lang.system.EnvironmentVariables;
 import org.junit.runners.MethodSorters;
 
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
@@ -22,13 +22,15 @@ public class NonBlockingDirectStatsDClientTest {
     private static DirectStatsDClient client;
     private static DummyStatsDServer server;
 
-    @Rule public final EnvironmentVariables environmentVariables = new EnvironmentVariables();
+    // Controlled (empty) environment so the client is unaffected by ambient DD_* variables.
+    private static final Map<String, String> CLIENT_ENV = new HashMap<>();
 
     @BeforeClass
     public static void start() throws IOException {
         server = new UDPDummyStatsDServer(STATSD_SERVER_PORT);
         client =
                 new NonBlockingStatsDClientBuilder()
+                        .withEnvironmentVariables(CLIENT_ENV)
                         .prefix("my.prefix")
                         .hostname("localhost")
                         .port(STATSD_SERVER_PORT)