diff --git a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java index 24379500836..7c24dccad4b 100644 --- a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java +++ b/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()); diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java index f91b20798eb..c1e01b13fc7 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java +++ b/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(); } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java index c0fea244428..d7521591efb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java +++ b/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, diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/MetricsAggregatorFactoryTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/MetricsAggregatorFactoryTest.groovy index cb6b9e6d6a1..fc47ba0ecfa 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/MetricsAggregatorFactoryTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/metrics/MetricsAggregatorFactoryTest.groovy @@ -3,6 +3,7 @@ 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 { @@ -10,8 +11,10 @@ class MetricsAggregatorFactoryTest extends DDSpecification { 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 } @@ -19,8 +22,10 @@ class MetricsAggregatorFactoryTest extends DDSpecification { 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 } } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 6ba0232ada5..764c8db1dc0 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/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) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 544b1f52584..232352b40cb 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/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 }