Preserve unix: agent URLs

communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java

@@ -29,7 +29,7 @@ public void createRemaining(Config config) {
       monitoring = Monitoring.DISABLED;
     }
     if (agentUrl == null) {
-      agentUrl = HttpUrl.parse(config.getAgentUrl());
+      agentUrl = parseAgentUrl(config);
       if (agentUrl == null) {
         throw new IllegalArgumentException("Bad agent URL: " + config.getAgentUrl());
       }
@@ -43,6 +43,15 @@ public void createRemaining(Config config) {
     }
   }
 
+  private static HttpUrl parseAgentUrl(Config config) {
+    String agentUrl = config.getAgentUrl();
+    if (agentUrl.startsWith("unix:")) {
+      // provide placeholder agent URL, in practice we'll be tunnelling over UDS
+      agentUrl = "http://" + config.getAgentHost() + ":" + config.getAgentPort();
+    }
+    return HttpUrl.parse(agentUrl);
+  }
+
   private static long getHttpClientTimeout(Config config) {
     if (!config.isCiVisibilityEnabled()) {
       return TimeUnit.SECONDS.toMillis(config.getAgentTimeout());

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java

@@ -142,7 +142,9 @@ public static synchronized void run(
   private static String getDiagnosticEndpoint(
       Config config, DDAgentFeaturesDiscovery ddAgentFeaturesDiscovery) {
     if (ddAgentFeaturesDiscovery.supportsDebuggerDiagnostics()) {
-      return config.getAgentUrl() + "/" + DDAgentFeaturesDiscovery.DEBUGGER_DIAGNOSTICS_ENDPOINT;
+      return ddAgentFeaturesDiscovery
+          .buildUrl(DDAgentFeaturesDiscovery.DEBUGGER_DIAGNOSTICS_ENDPOINT)
+          .toString();
     }
     return config.getFinalDebuggerSnapshotUrl();
   }

dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java

@@ -71,7 +71,7 @@ public ConflatingMetricsAggregator(
         sharedCommunicationObjects.featuresDiscovery(config),
         new OkHttpSink(
             sharedCommunicationObjects.okHttpClient,
-            config.getAgentUrl(),
+            sharedCommunicationObjects.agentUrl.toString(),
             V6_METRICS_ENDPOINT,
             config.isTracerMetricsBufferingEnabled(),
             false,

dd-trace-core/src/test/groovy/datadog/trace/common/metrics/MetricsAggregatorFactoryTest.groovy

@@ -3,24 +3,29 @@ package datadog.trace.common.metrics
 import datadog.communication.ddagent.SharedCommunicationObjects
 import datadog.trace.api.Config
 import datadog.trace.test.util.DDSpecification
+import okhttp3.HttpUrl
 
 class MetricsAggregatorFactoryTest extends DDSpecification {
 
   def "when metrics disabled no-op aggregator created"() {
     setup:
     Config config = Mock(Config)
     config.isTracerMetricsEnabled() >> false
+    def sco = Mock(SharedCommunicationObjects)
+    sco.agentUrl = HttpUrl.parse("http://localhost:8126")
     expect:
-    def aggregator = MetricsAggregatorFactory.createMetricsAggregator(config, Mock(SharedCommunicationObjects))
+    def aggregator = MetricsAggregatorFactory.createMetricsAggregator(config, sco)
     assert aggregator instanceof NoOpMetricsAggregator
   }
 
   def "when metrics enabled conflating aggregator created"() {
     setup:
     Config config = Spy(Config.get())
     config.isTracerMetricsEnabled() >> true
+    def sco = Mock(SharedCommunicationObjects)
+    sco.agentUrl = HttpUrl.parse("http://localhost:8126")
     expect:
-    def aggregator = MetricsAggregatorFactory.createMetricsAggregator(config, Mock(SharedCommunicationObjects))
+    def aggregator = MetricsAggregatorFactory.createMetricsAggregator(config, sco)
     assert aggregator instanceof ConflatingMetricsAggregator
   }
 }

internal-api/src/main/java/datadog/trace/api/Config.java

@@ -1090,14 +1090,17 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
       }
     }
 
-    if (agentHostFromEnvironment == null) {
-      agentHostFromEnvironment = configProvider.getString(AGENT_HOST);
-      rebuildAgentUrl = true;
-    }
-
-    if (agentPortFromEnvironment < 0) {
-      agentPortFromEnvironment = configProvider.getInteger(TRACE_AGENT_PORT, -1, AGENT_PORT_LEGACY);
-      rebuildAgentUrl = true;
+    // avoid merging in supplementary host/port settings when dealing with unix: URLs
+    if (unixSocketFromEnvironment == null) {
+      if (agentHostFromEnvironment == null) {
+        agentHostFromEnvironment = configProvider.getString(AGENT_HOST);
+        rebuildAgentUrl = true;
+      }
+      if (agentPortFromEnvironment < 0) {
+        agentPortFromEnvironment =
+            configProvider.getInteger(TRACE_AGENT_PORT, -1, AGENT_PORT_LEGACY);
+        rebuildAgentUrl = true;
+      }
     }
 
     if (agentHostFromEnvironment == null) {

internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy

@@ -2036,8 +2036,45 @@ class ConfigTest extends DDSpecification {
     !config.perfMetricsEnabled
   }
 
-  def "trace_agent_url overrides either host and port or unix domain"() {
+  def "trace_agent_url overrides default host and port or unix domain"() {
     setup:
+    if (configuredUrl != null) {
+      System.setProperty(PREFIX + TRACE_AGENT_URL, configuredUrl)
+    } else {
+      System.clearProperty(PREFIX + TRACE_AGENT_URL)
+    }
+
+    when:
+    def config = new Config()
+
+    then:
+    config.agentUrl == expectedUrl
+    config.agentHost == expectedHost
+    config.agentPort == expectedPort
+    config.agentUnixDomainSocket == expectedUnixDomainSocket
+
+    where:
+    // spotless:off
+    configuredUrl                     | expectedUrl                       | expectedHost | expectedPort | expectedUnixDomainSocket
+    null                              | "http://localhost:8126"           | "localhost"  | 8126         | null
+    ""                                | "http://localhost:8126"           | "localhost"  | 8126         | null
+    "http://localhost:1234"           | "http://localhost:1234"           | "localhost"  | 1234         | null
+    "http://somehost"                 | "http://somehost:8126"            | "somehost"   | 8126         | null
+    "http://somehost:80"              | "http://somehost:80"              | "somehost"   | 80           | null
+    "https://somehost:8143"           | "https://somehost:8143"           | "somehost"   | 8143         | null
+    "unix:///another/socket/path"     | "unix:///another/socket/path"     | "localhost"  | 8126         | "/another/socket/path"
+    "unix:///another%2Fsocket%2Fpath" | "unix:///another%2Fsocket%2Fpath" | "localhost"  | 8126         | "/another/socket/path"
+    "http:"                           | "http://localhost:8126"           | "localhost"  | 8126         | null
+    "unix:"                           | "http://localhost:8126"           | "localhost"  | 8126         | null
+    "1234"                            | "http://localhost:8126"           | "localhost"  | 8126         | null
+    ":1234"                           | "http://localhost:8126"           | "localhost"  | 8126         | null
+    // spotless:on
+  }
+
+  def "trace_agent_url overrides configured host and port or unix domain"() {
+    setup:
+    System.setProperty(PREFIX + AGENT_HOST, "test-host")
+    System.setProperty(PREFIX + TRACE_AGENT_PORT, "8888")
     System.setProperty(PREFIX + AGENT_UNIX_DOMAIN_SOCKET, "/path/to/socket")
     if (configuredUrl != null) {
       System.setProperty(PREFIX + TRACE_AGENT_URL, configuredUrl)
@@ -2056,19 +2093,19 @@ class ConfigTest extends DDSpecification {
 
     where:
     // spotless:off
-    configuredUrl                     | expectedUrl             | expectedHost | expectedPort | expectedUnixDomainSocket
-    null                              | "http://localhost:8126" | "localhost"  | 8126         | "/path/to/socket"
-    ""                                | "http://localhost:8126" | "localhost"  | 8126         | "/path/to/socket"
-    "http://localhost:1234"           | "http://localhost:1234" | "localhost"  | 1234         | "/path/to/socket"
-    "http://somehost"                 | "http://somehost:8126"  | "somehost"   | 8126         | "/path/to/socket"
-    "http://somehost:80"              | "http://somehost:80"    | "somehost"   | 80           | "/path/to/socket"
-    "https://somehost:8143"           | "https://somehost:8143" | "somehost"   | 8143         | "/path/to/socket"
-    "unix:///another/socket/path"     | "http://localhost:8126" | "localhost"  | 8126         | "/another/socket/path"
-    "unix:///another%2Fsocket%2Fpath" | "http://localhost:8126" | "localhost"  | 8126         | "/another/socket/path"
-    "http:"                           | "http://localhost:8126" | "localhost"  | 8126         | "/path/to/socket"
-    "unix:"                           | "http://localhost:8126" | "localhost"  | 8126         | "/path/to/socket"
-    "1234"                            | "http://localhost:8126" | "localhost"  | 8126         | "/path/to/socket"
-    ":1234"                           | "http://localhost:8126" | "localhost"  | 8126         | "/path/to/socket"
+    configuredUrl                     | expectedUrl                       | expectedHost | expectedPort | expectedUnixDomainSocket
+    null                              | "http://test-host:8888"           | "test-host"  | 8888         | "/path/to/socket"
+    ""                                | "http://test-host:8888"           | "test-host"  | 8888         | "/path/to/socket"
+    "http://localhost:1234"           | "http://localhost:1234"           | "localhost"  | 1234         | "/path/to/socket"
+    "http://somehost"                 | "http://somehost:8888"            | "somehost"   | 8888         | "/path/to/socket"
+    "http://somehost:80"              | "http://somehost:80"              | "somehost"   | 80           | "/path/to/socket"
+    "https://somehost:8143"           | "https://somehost:8143"           | "somehost"   | 8143         | "/path/to/socket"
+    "unix:///another/socket/path"     | "unix:///another/socket/path"     | "localhost"  | 8126         | "/another/socket/path"
+    "unix:///another%2Fsocket%2Fpath" | "unix:///another%2Fsocket%2Fpath" | "localhost"  | 8126         | "/another/socket/path"
+    "http:"                           | "http://test-host:8888"           | "test-host"  | 8888         | "/path/to/socket"
+    "unix:"                           | "http://test-host:8888"           | "test-host"  | 8888         | "/path/to/socket"
+    "1234"                            | "http://test-host:8888"           | "test-host"  | 8888         | "/path/to/socket"
+    ":1234"                           | "http://test-host:8888"           | "test-host"  | 8888         | "/path/to/socket"
     // spotless:on
   }