diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java index d16e663da3e..6ac51e70e9c 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java @@ -17,6 +17,8 @@ public final class HostNameResolver { try { final ClassLoader cl = HostNameResolver.class.getClassLoader(); final MethodHandles methodHandles = new MethodHandles(cl); + // forces the JPMS opener for this class to get executed. More efficient than getLocalHost + InetAddress.getLoopbackAddress(); final Class holderClass = Class.forName("java.net.InetAddress$InetAddressHolder", false, cl); @@ -59,6 +61,9 @@ private static String fromCache(InetAddress remoteAddress, String ip) { } public static String hostName(InetAddress address, String ip) { + if (address == null) { + return null; + } final String alreadyResolved = getAlreadyResolvedHostName(address); if (alreadyResolved != null) { return alreadyResolved; diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/JpmsInetAddressHelper.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/JpmsInetAddressHelper.java new file mode 100644 index 00000000000..62b937b824d --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/JpmsInetAddressHelper.java @@ -0,0 +1,9 @@ +package datadog.trace.bootstrap.instrumentation.java.net; + +import java.util.concurrent.atomic.AtomicBoolean; + +public class JpmsInetAddressHelper { + public static final AtomicBoolean OPENED = new AtomicBoolean(false); + + private JpmsInetAddressHelper() {} +} diff --git a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie index daae12566f1..36cedaf6081 100644 --- a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie +++ b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie @@ -59,6 +59,7 @@ 0 java.lang.VirtualThread 0 java.net.http.* 0 java.net.HttpURLConnection +0 java.net.InetAddress 0 java.net.Socket 0 java.net.URL 0 java.nio.DirectByteBuffer diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java/datadog/trace/instrumentation/httpclient/JpmsInetAddressInstrumentation.java b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java/datadog/trace/instrumentation/httpclient/JpmsInetAddressInstrumentation.java new file mode 100644 index 00000000000..0f971d7a3b0 --- /dev/null +++ b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java/datadog/trace/instrumentation/httpclient/JpmsInetAddressInstrumentation.java @@ -0,0 +1,33 @@ +package datadog.trace.instrumentation.httpclient; + +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import datadog.environment.JavaVirtualMachine; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; + +@AutoService(InstrumenterModule.class) +public class JpmsInetAddressInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.HasMethodAdvice, Instrumenter.ForSingleType { + + public JpmsInetAddressInstrumentation() { + super("java-net"); + } + + @Override + public boolean isEnabled() { + return super.isEnabled() && JavaVirtualMachine.isJavaVersionAtLeast(9); + } + + @Override + public String instrumentedType() { + return "java.net.InetAddress"; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + // it does not work with typeInitializer() + transformer.applyAdvice(isConstructor(), packageName + ".JpmsInetAddressClearanceAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java11/datadog/trace/instrumentation/httpclient/JpmsInetAddressClearanceAdvice.java b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java11/datadog/trace/instrumentation/httpclient/JpmsInetAddressClearanceAdvice.java new file mode 100644 index 00000000000..201b13ba3f5 --- /dev/null +++ b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java11/datadog/trace/instrumentation/httpclient/JpmsInetAddressClearanceAdvice.java @@ -0,0 +1,19 @@ +package datadog.trace.instrumentation.httpclient; + +import datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver; +import datadog.trace.bootstrap.instrumentation.java.net.JpmsInetAddressHelper; +import java.net.InetAddress; +import net.bytebuddy.asm.Advice; + +public class JpmsInetAddressClearanceAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void openOnReturn() { + if (JpmsInetAddressHelper.OPENED.compareAndSet(false, true)) { + // This call needs imperatively to be done from the same module we're adding opens to, + // because the JDK checks that the caller belongs to the same module. + // The code of this advice is inlined into the constructor of InetAddress (java.base), + // so it will work. Moving the same call to a helper class won't. + InetAddress.class.getModule().addOpens("java.net", HostNameResolver.class.getModule()); + } + } +} diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JpmsInetAddressDisabledForkedTest.groovy b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JpmsInetAddressDisabledForkedTest.groovy new file mode 100644 index 00000000000..bcf7fd11042 --- /dev/null +++ b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JpmsInetAddressDisabledForkedTest.groovy @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.httpclient + +import datadog.environment.JavaVirtualMachine +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver +import spock.lang.IgnoreIf + +@IgnoreIf(reason = "--illegal-access=deny is only enforced from java 16", value = { + !JavaVirtualMachine.isJavaVersionAtLeast(16) +}) +class JpmsInetAddressDisabledForkedTest extends InstrumentationSpecification { + + @Override + protected void configurePreAgent() { + super.configurePreAgent() + // Disable the JPMS instrumentation so java.net is NOT opened for deep reflection. + // HostNameResolver will be unable to bypass the IP→hostname cache and will fall back + // to the cache keyed by IP address. + injectSysConfig("dd.trace.java-net.enabled", "false") + } + + /** + * Verifies the fallback behaviour when the JPMS instrumentation is disabled: + * HostNameResolver cannot reflectively read the pre-set hostname from InetAddress and + * falls back to a cache keyed by IP address. As a result, once a hostname has been + * cached for a given IP, every subsequent lookup for that IP returns the first cached + * value, even when the InetAddress object carries a different hostname. + * + * This is the broken behaviour that the JPMS instrumentation is designed to fix. + */ + def "without JPMS instrumentation, IP cache causes stale hostname to be returned"() { + given: + def ip = [192, 0, 2, 2] as byte[] // different subnet from the enabled-test to avoid cross-test cache pollution + def addr1 = InetAddress.getByAddress("service1.example.com", ip) + // Prime the IP→hostname cache with service1's hostname + HostNameResolver.hostName(addr1, "192.0.2.2") + + when: "a second service with the same IP but a different hostname is resolved" + def addr2 = InetAddress.getByAddress("service2.example.com", ip) + def result = HostNameResolver.hostName(addr2, "192.0.2.2") + + then: "the stale cached hostname of service1 is returned instead of service2's" + result == "service1.example.com" + } +} diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JpmsInetAddressForkedTest.groovy b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JpmsInetAddressForkedTest.groovy new file mode 100644 index 00000000000..1dde353c3ee --- /dev/null +++ b/dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JpmsInetAddressForkedTest.groovy @@ -0,0 +1,35 @@ +package datadog.trace.instrumentation.httpclient + +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver + +class JpmsInetAddressForkedTest extends InstrumentationSpecification { + + /** + * Verifies that the JPMS instrumentation opens java.base/java.net so that + * HostNameResolver can bypass its IP→hostname cache and return the correct + * peer.hostname even when multiple services share a single IP address + * (e.g. services behind a reverse proxy). + * + * Without the fix, HostNameResolver cannot reflectively access + * InetAddress$InetAddressHolder on Java 9+ and falls back to a cache keyed + * by IP, causing the first service's hostname to be returned for all + * subsequent services on the same IP. + */ + def "instrumentation opens java.net so hostname is resolved correctly when IP is shared"() { + given: + // emulate an early initialisation + HostNameResolver.hostName(null, "192.0.2.1") + def ip = [192, 0, 2, 1] as byte[] // TEST-NET, will never appear in real DNS cache + def addr1 = InetAddress.getByAddress("service1.example.com", ip) + // Warm the IP→hostname cache with service1's hostname + HostNameResolver.hostName(addr1, "192.0.2.1") + + when: "a second service with the same IP but different hostname is resolved" + def addr2 = InetAddress.getByAddress("service2.example.com", ip) + def result = HostNameResolver.hostName(addr2, "192.0.2.1") + + then: "the hostname of addr2 is returned, not the cached hostname of addr1" + result == "service2.example.com" + } +} diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index f0fa49b0dd8..8c3e2f59509 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -6745,6 +6745,14 @@ "aliases": ["DD_TRACE_INTEGRATION_JAVA_HTTP_CLIENT_ENABLED", "DD_INTEGRATION_JAVA_HTTP_CLIENT_ENABLED"] } ], + "DD_TRACE_JAVA_NET_ENABLED": [ + { + "version": "B", + "type": "boolean", + "default": "true", + "aliases": ["DD_TRACE_INTEGRATION_JAVA_NET_ENABLED", "DD_INTEGRATION_JAVA_NET_ENABLED"] + } + ], "DD_TRACE_TRACE_FFM_ANALYTICS_ENABLED": [ { "version": "A",