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 2628e9416cf..96c70a91de4 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 @@ -1,7 +1,5 @@ package datadog.trace.bootstrap.instrumentation.decorator; -import static datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver.hostName; - import datadog.context.Context; import datadog.context.ContextScope; import datadog.trace.api.Config; @@ -14,8 +12,6 @@ import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.lang.reflect.Method; -import java.net.Inet4Address; -import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.concurrent.ExecutionException; @@ -150,10 +146,7 @@ public ContextScope onError(final ContextScope scope, final Throwable throwable) public AgentSpan onPeerConnection( final AgentSpan span, final InetSocketAddress remoteConnection) { - if (remoteConnection != null) { - onPeerConnection(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); - setPeerPort(span, remoteConnection.getPort()); - } + span.setTags(remoteConnection, PeerConnectionExtractor.INSTANCE); return span; } @@ -162,17 +155,7 @@ public AgentSpan onPeerConnection(final AgentSpan span, final InetAddress remote } public AgentSpan onPeerConnection(AgentSpan span, InetAddress remoteAddress, boolean resolved) { - if (remoteAddress != null) { - String ip = remoteAddress.getHostAddress(); - if (resolved && Config.get().isPeerHostNameEnabled()) { - span.setTag(Tags.PEER_HOSTNAME, hostName(remoteAddress, ip)); - } - if (remoteAddress instanceof Inet4Address) { - span.setTag(Tags.PEER_HOST_IPV4, ip); - } else if (remoteAddress instanceof Inet6Address) { - span.setTag(Tags.PEER_HOST_IPV6, ip); - } - } + PeerConnectionExtractor.setPeerAddress(span, remoteAddress, resolved); return span; } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java new file mode 100644 index 00000000000..f7767054fe6 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java @@ -0,0 +1,57 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import static datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver.hostName; + +import datadog.trace.api.Config; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.net.Inet4Address; +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.InetSocketAddress; + +/** + * Named singleton {@link TagExtractor} for peer-connection tags (hostname / IPv4 / IPv6 / port) + * from a remote {@link InetSocketAddress}. + * + *

Promoted from an inline lambda in {@link BaseDecorator} to a named class so it can be: + * referenced by name at call sites ({@code span.setTags(addr, PeerConnectionExtractor.INSTANCE)}), + * composed with other extractors (the axis the Decorator inheritance chain lacked), and + * given a home for the pure statics and the {@code hostName} resolver cache it consults. + * Non-capturing and stateless — the single {@link #INSTANCE} is effectively a static function + * object, so a monomorphic call site inlines it away. + */ +public final class PeerConnectionExtractor implements TagExtractor { + public static final PeerConnectionExtractor INSTANCE = new PeerConnectionExtractor(); + + private static final int UNSET_PORT = 0; + + private PeerConnectionExtractor() {} + + @Override + public void extract(final InetSocketAddress remoteConnection, final AgentSpan span) { + // Non-null guaranteed by AgentSpan.setTags (the sole entry point); extractors skip + // null-handling. + setPeerAddress(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); + final int port = remoteConnection.getPort(); + if (port > UNSET_PORT) { + span.setTag(Tags.PEER_PORT, port); + } + } + + static void setPeerAddress( + final AgentSpan span, final InetAddress remoteAddress, final boolean resolved) { + if (remoteAddress != null) { + String ip = remoteAddress.getHostAddress(); + if (resolved && Config.get().isPeerHostNameEnabled()) { + span.setTag(Tags.PEER_HOSTNAME, hostName(remoteAddress, ip)); + } + if (remoteAddress instanceof Inet4Address) { + span.setTag(Tags.PEER_HOST_IPV4, ip); + } else if (remoteAddress instanceof Inet6Address) { + span.setTag(Tags.PEER_HOST_IPV6, ip); + } + } + } +} diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy index 5b70cba2085..1a8b0c3d66b 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy @@ -46,29 +46,8 @@ class BaseDecoratorTest extends DDSpecification { 0 * _ } - def "test onPeerConnection"() { - when: - decorator.onPeerConnection(span, connection) - - then: - if (!connection.isUnresolved()) { - 1 * span.setTag(Tags.PEER_HOSTNAME, connection.hostName) - } - 1 * span.setTag(Tags.PEER_PORT, connection.port) - if (connection.address instanceof Inet4Address) { - 1 * span.setTag(Tags.PEER_HOST_IPV4, connection.address.hostAddress) - } - if (connection.address instanceof Inet6Address) { - 1 * span.setTag(Tags.PEER_HOST_IPV6, connection.address.hostAddress) - } - 0 * _ - - where: - connection | _ - new InetSocketAddress("localhost", 888) | _ - new InetSocketAddress("ipv6.google.com", 999) | _ - InetSocketAddress.createUnresolved("bad.address.local", 999) | _ - } + // "test onPeerConnection" migrated to PeerConnectionExtractorTest (JUnit 5): onPeerConnection now + // routes through AgentSpan.setTags, a default method a Spock mock won't dispatch to the extractor. def "test onError"() { when: diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java new file mode 100644 index 00000000000..ac0ccd09255 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java @@ -0,0 +1,62 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link PeerConnectionExtractor#extract}. Covers the deterministic IP-family and + * port routing; the config/resolver-dependent {@code PEER_HOSTNAME} tag is intentionally not + * asserted (that behaviour belongs to the host-name resolver, and peer-hostname is on by default, + * so it would couple this test to reverse-DNS). Null source handling is the responsibility of + * {@code AgentSpan.setTags} (the extractor's sole entry point), not the extractor. + * + *

Literal IPs are used so no DNS lookup happens. + */ +class PeerConnectionExtractorTest { + + @Test + void ipv4AddressSetsIpv4TagAndPort() throws UnknownHostException { + final AgentSpan span = mock(AgentSpan.class); + final InetAddress addr = InetAddress.getByName("1.2.3.4"); // literal -> no DNS + + PeerConnectionExtractor.INSTANCE.extract(new InetSocketAddress(addr, 8080), span); + + verify(span).setTag(Tags.PEER_HOST_IPV4, addr.getHostAddress()); + verify(span).setTag(Tags.PEER_PORT, 8080); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV6), anyString()); + } + + @Test + void ipv6AddressSetsIpv6TagAndPort() throws UnknownHostException { + final AgentSpan span = mock(AgentSpan.class); + final InetAddress addr = InetAddress.getByName("2001:db8::1"); // literal -> no DNS + + PeerConnectionExtractor.INSTANCE.extract(new InetSocketAddress(addr, 9090), span); + + verify(span).setTag(Tags.PEER_HOST_IPV6, addr.getHostAddress()); + verify(span).setTag(Tags.PEER_PORT, 9090); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV4), anyString()); + } + + @Test + void unresolvedAddressSetsOnlyPort() { + final AgentSpan span = mock(AgentSpan.class); + + PeerConnectionExtractor.INSTANCE.extract( + InetSocketAddress.createUnresolved("no.dns.local", 999), span); + + verify(span).setTag(Tags.PEER_PORT, 999); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV4), anyString()); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV6), anyString()); + } +}