From 2b66505b0336077d627e013cb34c5438579701c2 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 3 Jun 2025 17:59:21 +0200 Subject: [PATCH 1/5] Use resolved address for peer.hostname when available without hitting the cache --- .../decorator/BaseDecorator.java | 12 +--- .../java/net/HostNameResolver.java | 72 +++++++++++++++++++ .../java/net/HostNameResolverTest.groovy | 45 ++++++++++++ 3 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java create mode 100644 dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index d5bc9b56550..bb575d92080 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -2,13 +2,12 @@ import static datadog.trace.api.cache.RadixTreeCache.PORTS; import static datadog.trace.api.cache.RadixTreeCache.UNSET_PORT; +import static datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver.hostName; import datadog.context.ContextScope; import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.Functions; -import datadog.trace.api.cache.DDCache; -import datadog.trace.api.cache.DDCaches; import datadog.trace.api.cache.QualifiedClassNameCache; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -40,8 +39,6 @@ public String apply(Class clazz) { }, Functions.PrefixJoin.of(".")); - private static final DDCache HOSTNAME_CACHE = DDCaches.newFixedSizeCache(64); - protected final boolean traceAnalyticsEnabled; protected final Double traceAnalyticsSampleRate; @@ -200,11 +197,4 @@ public CharSequence className(final Class clazz) { String simpleName = clazz.getSimpleName(); return simpleName.isEmpty() ? CLASS_NAMES.getClassName(clazz) : simpleName; } - - private static String hostName(InetAddress remoteAddress, String ip) { - if (null != ip) { - return HOSTNAME_CACHE.computeIfAbsent(ip, _ip -> remoteAddress.getHostName()); - } - return remoteAddress.getHostName(); - } } 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 new file mode 100644 index 00000000000..8d50222a59c --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java @@ -0,0 +1,72 @@ +package datadog.trace.bootstrap.instrumentation.java.net; + +import datadog.trace.api.cache.DDCache; +import datadog.trace.api.cache.DDCaches; +import datadog.trace.util.MethodHandles; +import java.lang.invoke.MethodHandle; +import java.net.InetAddress; + +public final class HostNameResolver { + private static final MethodHandle HOLDER_GET; + private static final MethodHandle HOSTNAME_GET; + + private static final DDCache HOSTNAME_CACHE = DDCaches.newFixedSizeCache(64); + + static { + MethodHandle holderTmp = null, hostnameTmp = null; + try { + final ClassLoader cl = HostNameResolver.class.getClassLoader(); + final MethodHandles methodHandles = new MethodHandles(cl); + + final Class holderClass = + Class.forName("java.net.InetAddress$InetAddressHolder", false, cl); + holderTmp = methodHandles.method(InetAddress.class, "holder"); + if (holderTmp != null) { + hostnameTmp = methodHandles.method(holderClass, "getHostName"); + } + } catch (Throwable ignored) { + holderTmp = null; + } finally { + if (holderTmp != null && hostnameTmp != null) { + HOLDER_GET = holderTmp; + HOSTNAME_GET = hostnameTmp; + } else { + HOLDER_GET = null; + HOSTNAME_GET = null; + } + } + } + + private HostNameResolver() {} + + static String getAlreadyResolvedHostName(InetAddress address) { + if (HOLDER_GET == null) { + return null; + } + try { + final Object holder = HOLDER_GET.invoke(address); + return (String) HOSTNAME_GET.invoke(holder); + } catch (final Throwable ignored) { + } + return null; + } + + private static String fromCache(InetAddress remoteAddress, String ip) { + final String alreadyResolved = HostNameResolver.getAlreadyResolvedHostName(remoteAddress); + if (alreadyResolved != null) { + return alreadyResolved; + } + if (null != ip) { + return HOSTNAME_CACHE.computeIfAbsent(ip, _ip -> remoteAddress.getHostName()); + } + return remoteAddress.getHostName(); + } + + public static String hostName(InetAddress address, String ip) { + final String alreadyResolved = getAlreadyResolvedHostName(address); + if (alreadyResolved != null) { + return alreadyResolved; + } + return fromCache(address, ip); + } +} diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy new file mode 100644 index 00000000000..58f7a417c7a --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy @@ -0,0 +1,45 @@ +package datadog.trace.bootstrap.instrumentation.java.net + +import datadog.trace.test.util.DDSpecification + +class HostNameResolverTest extends DDSpecification { + def "should directly get the hostname for already resolved address #address"() { + given: + def host = HostNameResolver.getAlreadyResolvedHostName(address) + expect: + host == expected + where: + address | expected + new Inet4Address("test", InetAddress.getLocalHost().getAddress()) | "test" + new Inet6Address("test", InetAddress.getLocalHost().getAddress()) | "test" + } + + def "should return null when directly get the address for unresolved #address"() { + given: + def host = HostNameResolver.getAlreadyResolvedHostName(address) + expect: + host == null + where: + address | _ + InetAddress.getByAddress(InetAddress.getLocalHost().getAddress()) | _ + new Inet6Address(null, InetAddress.getLocalHost().getAddress()) | _ + } + + def "should use the cache for unresolved addresses"() { + setup: + def inet1 = Mock(InetAddress) + def inet2 = Mock(InetAddress) + when: + def address = new InetSocketAddress(inet1, 666) + def host = HostNameResolver.hostName(address.getAddress(), "127.0.0.1") + then: + host == "somehost" + 1 * inet1.getHostName() >> "somehost" + when: + address = new InetSocketAddress(inet2, 666) + host = HostNameResolver.hostName(address.getAddress(), "127.0.0.1") + then: + 0 * inet2.getHostName() + host == "somehost" + } +} From e45ff3972ccb373a029ec91c695c6974cd90a08f Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 3 Jun 2025 18:43:17 +0200 Subject: [PATCH 2/5] Update dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java Co-authored-by: Stuart McCulloch --- .../bootstrap/instrumentation/java/net/HostNameResolver.java | 4 ---- 1 file changed, 4 deletions(-) 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 8d50222a59c..d16e663da3e 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 @@ -52,10 +52,6 @@ static String getAlreadyResolvedHostName(InetAddress address) { } private static String fromCache(InetAddress remoteAddress, String ip) { - final String alreadyResolved = HostNameResolver.getAlreadyResolvedHostName(remoteAddress); - if (alreadyResolved != null) { - return alreadyResolved; - } if (null != ip) { return HOSTNAME_CACHE.computeIfAbsent(ip, _ip -> remoteAddress.getHostName()); } From 5bcf24104d896397bab0ce5f373923e37fd78d12 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 3 Jun 2025 19:08:44 +0200 Subject: [PATCH 3/5] try to fork the test --- ...ameResolverTest.groovy => HostNameResolverForkedTest.groovy} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/{HostNameResolverTest.groovy => HostNameResolverForkedTest.groovy} (96%) diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy similarity index 96% rename from dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy rename to dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy index 58f7a417c7a..b8ad8b64a22 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy @@ -2,7 +2,7 @@ package datadog.trace.bootstrap.instrumentation.java.net import datadog.trace.test.util.DDSpecification -class HostNameResolverTest extends DDSpecification { +class HostNameResolverForkedTest extends DDSpecification { def "should directly get the hostname for already resolved address #address"() { given: def host = HostNameResolver.getAlreadyResolvedHostName(address) From c9b7da4fc886dc3cd543d78afe5cb9544c1d81d3 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 3 Jun 2025 22:53:10 +0200 Subject: [PATCH 4/5] Open java.net for hostnameresolver tests --- dd-java-agent/agent-bootstrap/build.gradle | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dd-java-agent/agent-bootstrap/build.gradle b/dd-java-agent/agent-bootstrap/build.gradle index bf9f1742443..47fcf1b0717 100644 --- a/dd-java-agent/agent-bootstrap/build.gradle +++ b/dd-java-agent/agent-bootstrap/build.gradle @@ -65,3 +65,11 @@ jmh { jmhVersion = '1.32' duplicateClassesStrategy = DuplicatesStrategy.EXCLUDE } + +project.afterEvaluate { + tasks.withType(Test).configureEach { + if (javaLauncher.get().metadata.languageVersion.asInt() >= 16) { + jvmArgs += ['--add-opens', 'java.base/java.net=ALL-UNNAMED'] // for HostNameResolverForkedTest + } + } +} From fef3bcd449dfe4c54dedb3d439ab67ae40a92c82 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 4 Jun 2025 00:03:19 +0200 Subject: [PATCH 5/5] Do not mock sealed classes with java 21 --- .../java/net/HostNameResolverForkedTest.groovy | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy index b8ad8b64a22..131094c70ed 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolverForkedTest.groovy @@ -26,20 +26,18 @@ class HostNameResolverForkedTest extends DDSpecification { } def "should use the cache for unresolved addresses"() { - setup: - def inet1 = Mock(InetAddress) - def inet2 = Mock(InetAddress) + given: + def inet1 = new Inet4Address(null, InetAddress.getLocalHost().getAddress()) + def inet2 = new Inet4Address(null, 0) // this will fail if a resolution will happen when: def address = new InetSocketAddress(inet1, 666) def host = HostNameResolver.hostName(address.getAddress(), "127.0.0.1") then: - host == "somehost" - 1 * inet1.getHostName() >> "somehost" + host != null when: address = new InetSocketAddress(inet2, 666) - host = HostNameResolver.hostName(address.getAddress(), "127.0.0.1") + def host2 = HostNameResolver.hostName(address.getAddress(), "127.0.0.1") then: - 0 * inet2.getHostName() - host == "somehost" + host == host2 } }