Skip to content

Commit 3ba8d13

Browse files
amarzialidevflow.devflow-routing-intake
andauthored
Add a jpms opener for HostNameResolver cache (#11095)
# What Does This Do Enable using the HostNameResolver cache also under jpms enforcement. This PR instrument java.net specific classes in order to open the module from inside. The same kind of trick has been previously done with mule instrumentation Solves #11088 # Motivation # Additional Notes # Contributor Checklist - Format the title according to [the contribution guidelines](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#title-format) - Assign the `type:` and (`comp:` or `inst:`) labels in addition to [any other useful labels](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#labels) - Avoid using `close`, `fix`, or [any linking keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) when referencing an issue Use `solves` instead, and assign the PR [milestone](https://github.com/DataDog/dd-trace-java/milestones) to the issue - Update the [CODEOWNERS](https://github.com/DataDog/dd-trace-java/blob/master/.github/CODEOWNERS) file on source file addition, migration, or deletion - Update [public documentation](https://docs.datadoghq.com/tracing/trace_collection/library_config/java/) with any new configuration flags or behaviors Jira ticket: [PROJ-IDENT] ***Note:*** **Once your PR is ready to merge, add it to the merge queue by commenting `/merge`.** `/merge -c` cancels the queue request. `/merge -f --reason "reason"` skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see [this doc](https://datadoghq.atlassian.net/wiki/spaces/DEVX/pages/3121612126/MergeQueue). <!-- # Opening vs Drafting a PR: When opening a pull request, please open it as a draft to not auto assign reviewers before you feel the pull request is in a reviewable state. # Linking a JIRA ticket: Please link your JIRA ticket by adding its identifier between brackets (ex [PROJ-IDENT]) in the PR description, not the title. This requirement only applies to Datadog employees. --> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent e2d98c0 commit 3ba8d13

8 files changed

Lines changed: 160 additions & 0 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/net/HostNameResolver.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ public final class HostNameResolver {
1717
try {
1818
final ClassLoader cl = HostNameResolver.class.getClassLoader();
1919
final MethodHandles methodHandles = new MethodHandles(cl);
20+
// forces the JPMS opener for this class to get executed. More efficient than getLocalHost
21+
InetAddress.getLoopbackAddress();
2022

2123
final Class<?> holderClass =
2224
Class.forName("java.net.InetAddress$InetAddressHolder", false, cl);
@@ -59,6 +61,9 @@ private static String fromCache(InetAddress remoteAddress, String ip) {
5961
}
6062

6163
public static String hostName(InetAddress address, String ip) {
64+
if (address == null) {
65+
return null;
66+
}
6267
final String alreadyResolved = getAlreadyResolvedHostName(address);
6368
if (alreadyResolved != null) {
6469
return alreadyResolved;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package datadog.trace.bootstrap.instrumentation.java.net;
2+
3+
import java.util.concurrent.atomic.AtomicBoolean;
4+
5+
public class JpmsInetAddressHelper {
6+
public static final AtomicBoolean OPENED = new AtomicBoolean(false);
7+
8+
private JpmsInetAddressHelper() {}
9+
}

dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
0 java.lang.VirtualThread
6060
0 java.net.http.*
6161
0 java.net.HttpURLConnection
62+
0 java.net.InetAddress
6263
0 java.net.Socket
6364
0 java.net.URL
6465
0 java.nio.DirectByteBuffer
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package datadog.trace.instrumentation.httpclient;
2+
3+
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
4+
5+
import com.google.auto.service.AutoService;
6+
import datadog.environment.JavaVirtualMachine;
7+
import datadog.trace.agent.tooling.Instrumenter;
8+
import datadog.trace.agent.tooling.InstrumenterModule;
9+
10+
@AutoService(InstrumenterModule.class)
11+
public class JpmsInetAddressInstrumentation extends InstrumenterModule.Tracing
12+
implements Instrumenter.HasMethodAdvice, Instrumenter.ForSingleType {
13+
14+
public JpmsInetAddressInstrumentation() {
15+
super("java-net");
16+
}
17+
18+
@Override
19+
public boolean isEnabled() {
20+
return super.isEnabled() && JavaVirtualMachine.isJavaVersionAtLeast(9);
21+
}
22+
23+
@Override
24+
public String instrumentedType() {
25+
return "java.net.InetAddress";
26+
}
27+
28+
@Override
29+
public void methodAdvice(MethodTransformer transformer) {
30+
// it does not work with typeInitializer()
31+
transformer.applyAdvice(isConstructor(), packageName + ".JpmsInetAddressClearanceAdvice");
32+
}
33+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package datadog.trace.instrumentation.httpclient;
2+
3+
import datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver;
4+
import datadog.trace.bootstrap.instrumentation.java.net.JpmsInetAddressHelper;
5+
import java.net.InetAddress;
6+
import net.bytebuddy.asm.Advice;
7+
8+
public class JpmsInetAddressClearanceAdvice {
9+
@Advice.OnMethodExit(suppress = Throwable.class)
10+
public static void openOnReturn() {
11+
if (JpmsInetAddressHelper.OPENED.compareAndSet(false, true)) {
12+
// This call needs imperatively to be done from the same module we're adding opens to,
13+
// because the JDK checks that the caller belongs to the same module.
14+
// The code of this advice is inlined into the constructor of InetAddress (java.base),
15+
// so it will work. Moving the same call to a helper class won't.
16+
InetAddress.class.getModule().addOpens("java.net", HostNameResolver.class.getModule());
17+
}
18+
}
19+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package datadog.trace.instrumentation.httpclient
2+
3+
import datadog.environment.JavaVirtualMachine
4+
import datadog.trace.agent.test.InstrumentationSpecification
5+
import datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver
6+
import spock.lang.IgnoreIf
7+
8+
@IgnoreIf(reason = "--illegal-access=deny is only enforced from java 16", value = {
9+
!JavaVirtualMachine.isJavaVersionAtLeast(16)
10+
})
11+
class JpmsInetAddressDisabledForkedTest extends InstrumentationSpecification {
12+
13+
@Override
14+
protected void configurePreAgent() {
15+
super.configurePreAgent()
16+
// Disable the JPMS instrumentation so java.net is NOT opened for deep reflection.
17+
// HostNameResolver will be unable to bypass the IP→hostname cache and will fall back
18+
// to the cache keyed by IP address.
19+
injectSysConfig("dd.trace.java-net.enabled", "false")
20+
}
21+
22+
/**
23+
* Verifies the fallback behaviour when the JPMS instrumentation is disabled:
24+
* HostNameResolver cannot reflectively read the pre-set hostname from InetAddress and
25+
* falls back to a cache keyed by IP address. As a result, once a hostname has been
26+
* cached for a given IP, every subsequent lookup for that IP returns the first cached
27+
* value, even when the InetAddress object carries a different hostname.
28+
*
29+
* This is the broken behaviour that the JPMS instrumentation is designed to fix.
30+
*/
31+
def "without JPMS instrumentation, IP cache causes stale hostname to be returned"() {
32+
given:
33+
def ip = [192, 0, 2, 2] as byte[] // different subnet from the enabled-test to avoid cross-test cache pollution
34+
def addr1 = InetAddress.getByAddress("service1.example.com", ip)
35+
// Prime the IP→hostname cache with service1's hostname
36+
HostNameResolver.hostName(addr1, "192.0.2.2")
37+
38+
when: "a second service with the same IP but a different hostname is resolved"
39+
def addr2 = InetAddress.getByAddress("service2.example.com", ip)
40+
def result = HostNameResolver.hostName(addr2, "192.0.2.2")
41+
42+
then: "the stale cached hostname of service1 is returned instead of service2's"
43+
result == "service1.example.com"
44+
}
45+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package datadog.trace.instrumentation.httpclient
2+
3+
import datadog.environment.JavaVirtualMachine
4+
import datadog.trace.agent.test.InstrumentationSpecification
5+
import datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver
6+
import spock.lang.IgnoreIf
7+
8+
@IgnoreIf(reason = 'Does not work on J9', value = {
9+
JavaVirtualMachine.isJ9()
10+
})
11+
class JpmsInetAddressForkedTest extends InstrumentationSpecification {
12+
13+
/**
14+
* Verifies that the JPMS instrumentation opens java.base/java.net so that
15+
* HostNameResolver can bypass its IP→hostname cache and return the correct
16+
* peer.hostname even when multiple services share a single IP address
17+
* (e.g. services behind a reverse proxy).
18+
*
19+
* Without the fix, HostNameResolver cannot reflectively access
20+
* InetAddress$InetAddressHolder on Java 9+ and falls back to a cache keyed
21+
* by IP, causing the first service's hostname to be returned for all
22+
* subsequent services on the same IP.
23+
*/
24+
def "instrumentation opens java.net so hostname is resolved correctly when IP is shared"() {
25+
given:
26+
// emulate an early initialisation
27+
HostNameResolver.hostName(null, "192.0.2.1")
28+
def ip = [192, 0, 2, 1] as byte[] // TEST-NET, will never appear in real DNS cache
29+
def addr1 = InetAddress.getByAddress("service1.example.com", ip)
30+
// Warm the IP→hostname cache with service1's hostname
31+
HostNameResolver.hostName(addr1, "192.0.2.1")
32+
33+
when: "a second service with the same IP but different hostname is resolved"
34+
def addr2 = InetAddress.getByAddress("service2.example.com", ip)
35+
def result = HostNameResolver.hostName(addr2, "192.0.2.1")
36+
37+
then: "the hostname of addr2 is returned, not the cached hostname of addr1"
38+
result == "service2.example.com"
39+
}
40+
}

metadata/supported-configurations.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6745,6 +6745,14 @@
67456745
"aliases": ["DD_TRACE_INTEGRATION_JAVA_HTTP_CLIENT_ENABLED", "DD_INTEGRATION_JAVA_HTTP_CLIENT_ENABLED"]
67466746
}
67476747
],
6748+
"DD_TRACE_JAVA_NET_ENABLED": [
6749+
{
6750+
"version": "B",
6751+
"type": "boolean",
6752+
"default": "true",
6753+
"aliases": ["DD_TRACE_INTEGRATION_JAVA_NET_ENABLED", "DD_INTEGRATION_JAVA_NET_ENABLED"]
6754+
}
6755+
],
67486756
"DD_TRACE_TRACE_FFM_ANALYTICS_ENABLED": [
67496757
{
67506758
"version": "A",

0 commit comments

Comments
 (0)