From 9cfe33f296191ae05e66c52a373671c62a92ed97 Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Wed, 4 Nov 2026 17:40:39 +0100 Subject: [PATCH 1/7] Resolve client IPs for Java Lambda AppSec events --- dd-java-agent/testing/gradle.lockfile | 2 +- dd-trace-core/build.gradle | 4 + dd-trace-core/gradle.lockfile | 4 +- .../trace/lambda/LambdaAppSecHandler.java | 69 +++- .../trace/lambda/LambdaAppSecHandlerTest.java | 343 ++++++++++++++---- 5 files changed, 353 insertions(+), 69 deletions(-) diff --git a/dd-java-agent/testing/gradle.lockfile b/dd-java-agent/testing/gradle.lockfile index b1d6fd3c2dd..b9641077f64 100644 --- a/dd-java-agent/testing/gradle.lockfile +++ b/dd-java-agent/testing/gradle.lockfile @@ -60,7 +60,7 @@ org.apache.commons:commons-lang3:3.20.0=spotbugs org.apache.commons:commons-text:1.15.0=spotbugs org.apache.logging.log4j:log4j-api:2.26.1=spotbugs org.apache.logging.log4j:log4j-core:2.26.1=spotbugs -org.apiguardian:apiguardian-api:1.1.2=compileClasspath,testCompileClasspath +org.apiguardian:apiguardian-api:1.1.2=compileClasspath,runtimeClasspath,testCompileClasspath org.codehaus.groovy:groovy-ant:3.0.23=codenarc org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc org.codehaus.groovy:groovy-groovydoc:3.0.23=codenarc diff --git a/dd-trace-core/build.gradle b/dd-trace-core/build.gradle index 7161ba9676d..4008698db7d 100644 --- a/dd-trace-core/build.gradle +++ b/dd-trace-core/build.gradle @@ -81,6 +81,10 @@ dependencies { api project(':dd-trace-api') api project(':communication') api project(':internal-api') + // Lambda reuses the HTTP client-IP resolver already supplied by the agent bootstrap jar. + compileOnly project(':dd-java-agent:agent-bootstrap') + testImplementation project(':dd-java-agent:agent-bootstrap') + jmhImplementation project(':dd-java-agent:agent-bootstrap') api project(':products:metrics:metrics-agent') // Note: dd-trace-core should NOT depend on metrics-lib directly // metrics-lib (with sketches-java) is bundled in metrics-agent shadowJar diff --git a/dd-trace-core/gradle.lockfile b/dd-trace-core/gradle.lockfile index e0bffcb8c85..afb1a90cc78 100644 --- a/dd-trace-core/gradle.lockfile +++ b/dd-trace-core/gradle.lockfile @@ -10,7 +10,7 @@ com.amazonaws:aws-lambda-java-events:3.11.0=jmhRuntimeClasspath,testCompileClass com.blogspot.mydailyjava:weak-lock-free:0.17=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq.okhttp3:okhttp:3.12.15=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq.okio:okio:1.17.6=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath -com.datadoghq:dd-instrument-java:0.0.6=jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath +com.datadoghq:dd-instrument-java:0.0.6=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq:dd-javac-plugin-client:0.2.2=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq:java-dogstatsd-client:4.4.5=jmhRuntimeClasspath,runtimeClasspath,testRuntimeClasspath,traceAgentTestRuntimeClasspath com.datadoghq:sketches-java:0.8.3=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testRuntimeClasspath,traceAgentTestRuntimeClasspath @@ -81,7 +81,7 @@ org.apache.commons:commons-math3:3.6.1=jmh,jmhCompileClasspath,jmhRuntimeClasspa org.apache.commons:commons-text:1.15.0=spotbugs org.apache.logging.log4j:log4j-api:2.26.1=spotbugs org.apache.logging.log4j:log4j-core:2.26.1=spotbugs -org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath,traceAgentTestCompileClasspath +org.apiguardian:apiguardian-api:1.1.2=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath org.checkerframework:checker-qual:3.33.0=testAnnotationProcessor,traceAgentTestAnnotationProcessor org.codehaus.groovy:groovy-ant:3.0.23=codenarc org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java index aa229a9f95f..5370c82627a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java @@ -10,9 +10,11 @@ import datadog.logging.RatelimitedLogger; import datadog.trace.api.Config; import datadog.trace.api.DDTags; +import datadog.trace.api.DDTraceId; import datadog.trace.api.KnownTags; import datadog.trace.api.ProductTraceSource; import datadog.trace.api.TagMap; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.appsec.AppSecContext; import datadog.trace.api.function.TriConsumer; import datadog.trace.api.gateway.BlockResponseFunction; @@ -22,6 +24,7 @@ import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.internal.TraceSegment; +import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.bootstrap.ActiveSubsystems; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; @@ -32,11 +35,13 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.URIUtils; +import datadog.trace.bootstrap.instrumentation.decorator.http.ClientIpAddressResolver; import datadog.trace.lambda.LambdaEventParser.LambdaRequestData; import datadog.trace.lambda.LambdaEventParser.LambdaResponseData; import datadog.trace.lambda.LambdaEventParser.LambdaTriggerType; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.net.InetAddress; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; @@ -118,7 +123,8 @@ public static AgentSpanContext processRequestStart(Object event) { } LambdaURIDataAdapter uriAdapter = new LambdaURIDataAdapter(fullPath, eventData.headers, eventData.host); - AgentSpanContext context = processAppSecRequestData(eventData, uriAdapter); + String clientIp = resolveClientIp(eventData); + AgentSpanContext context = processAppSecRequestData(eventData, uriAdapter, clientIp); if (context instanceof TagContext) { applyHttpTags((TagContext) context, eventData, uriAdapter); } @@ -385,7 +391,7 @@ static void applyHttpTags(TagContext ctx, LambdaRequestData req, LambdaURIDataAd * requestStarted} callback */ private static AgentSpanContext processAppSecRequestData( - LambdaRequestData eventData, LambdaURIDataAdapter uriAdapter) { + LambdaRequestData eventData, LambdaURIDataAdapter uriAdapter, String clientIp) { AgentTracer.TracerAPI tracer = AgentTracer.get(); Supplier> requestStartedCallback = tracer.getCallbackProvider(RequestContextSlot.APPSEC).getCallback(EVENTS.requestStarted()); @@ -400,6 +406,9 @@ private static AgentSpanContext processAppSecRequestData( // Call requestStarted appSecRequestContext = requestStartedCallback.get().getResult(); tagContext.withRequestContextDataAppSec(appSecRequestContext); + if (clientIp != null) { + tagContext.putTag(Tags.HTTP_CLIENT_IP, clientIp); + } if (appSecRequestContext != null) { TemporaryRequestContext requestContext = new TemporaryRequestContext(appSecRequestContext); @@ -433,6 +442,17 @@ private static AgentSpanContext processAppSecRequestData( } } + // Publish the inferred address before the socket address can trigger WAF evaluation. + if (clientIp != null) { + BiFunction> inferredAddrCallback = + tracer + .getCallbackProvider(RequestContextSlot.APPSEC) + .getCallback(EVENTS.requestInferredClientAddress()); + if (inferredAddrCallback != null) { + inferredAddrCallback.apply(requestContext, clientIp); + } + } + // Call requestClientSocketAddress even when no source IP is available. datadog.trace.api.function.TriFunction> socketAddrCallback = @@ -504,6 +524,51 @@ private static AgentSpanContext processAppSecRequestData( return tagContext; } + /** Resolves the client address using the same configuration and precedence as HTTP servers. */ + private static String resolveClientIp(LambdaRequestData eventData) { + Config config = Config.get(); + if (!config.isClientIpEnabled() && !config.isTraceClientIpResolverEnabled()) { + return null; + } + TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); + headers.xForwardedFor = findHeader(eventData.headers, "x-forwarded-for"); + headers.xRealIp = findHeader(eventData.headers, "x-real-ip"); + headers.trueClientIp = findHeader(eventData.headers, "true-client-ip"); + headers.xClientIp = findHeader(eventData.headers, "x-client-ip"); + headers.forwarded = findHeader(eventData.headers, "forwarded"); + headers.forwardedFor = findHeader(eventData.headers, "forwarded-for"); + headers.xClusterClientIp = findHeader(eventData.headers, "x-cluster-client-ip"); + headers.fastlyClientIp = findHeader(eventData.headers, "fastly-client-ip"); + headers.cfConnectingIp = findHeader(eventData.headers, "cf-connecting-ip"); + headers.cfConnectingIpv6 = findHeader(eventData.headers, "cf-connecting-ipv6"); + String customHeader = config.getTraceClientIpHeader(); + if (customHeader != null) { + headers.customIpHeader = findHeader(eventData.headers, customHeader); + } + TagContext extracted = + new TagContext( + null, + null, + headers, + null, + PrioritySampling.UNSET, + null, + TracePropagationStyle.NONE, + DDTraceId.ZERO); + InetAddress inferred = ClientIpAddressResolver.resolve(extracted, null); + if (eventData.sourceIp != null) { + if (inferred == null) { + inferred = ClientIpAddressResolver.parseIpAddress(eventData.sourceIp); + } else if (ClientIpAddressResolver.isIpAddrPrivate(inferred)) { + InetAddress source = ClientIpAddressResolver.parseIpAddress(eventData.sourceIp); + if (source != null && !ClientIpAddressResolver.isIpAddrPrivate(source)) { + inferred = source; + } + } + } + return inferred == null ? null : inferred.getHostAddress(); + } + /** Sets the current trigger type thread-local. Package-private for use in tests only. */ static void setCurrentTriggerType(LambdaTriggerType type) { if (type == null) { diff --git a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java index cce4e9e237c..59069ea9a0a 100644 --- a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java @@ -1,9 +1,12 @@ package datadog.trace.lambda; import static datadog.json.JsonMapper.toJson; +import static datadog.trace.api.config.TracerConfig.TRACE_CLIENT_IP_HEADER; import static datadog.trace.api.gateway.Events.EVENTS; import static datadog.trace.lambda.LambdaEventParser.detectTriggerType; import static datadog.trace.lambda.LambdaEventParser.parseResponse; +import static datadog.trace.test.junit.utils.config.WithConfigExtension.injectSysConfig; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -77,6 +80,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.tabletest.junit.TableTest; @@ -489,50 +493,54 @@ void extractsAlbDataCorrectly() { } @TableTest({ - "Scenario | Header | Value | Expected Ip ", - "xff | X-Forwarded-For | '1.1.1.1' | '1.1.1.1' ", - "xff last entry | X-Forwarded-For | '10.0.0.1, 1.1.1.1' | '1.1.1.1' ", - "xff multiple prefixes | X-Forwarded-For | '10.0.0.1, 1.1.1.1, 8.8.8.8' | '8.8.8.8' ", - "xff invalid prefix | X-Forwarded-For | 'invalid, 1.1.1.1' | '1.1.1.1' ", - "xff ipv6 | X-Forwarded-For | '2001:4860:4860::8888' | '2001:4860:4860::8888'", - "xff ipv6 last entry | X-Forwarded-For | 'fd00::1, 2001:4860:4860::8888' | '2001:4860:4860::8888'", - "xff private ipv6 | X-Forwarded-For | 'fd00::1' | 'fd00::1' ", - "xff private last entry | X-Forwarded-For | '1.1.1.1, fd00::1' | 'fd00::1' ", - "xff padded last entry | X-Forwarded-For | '10.0.0.1, 1.1.1.1 ' | '1.1.1.1' ", - "xff empty last entry | X-Forwarded-For | '1.1.1.1, ' | ", - "ignored forwarded | Forwarded | 'for=1.1.1.1' | ", - "ignored real ip | X-Real-IP | '1.1.1.1' | ", - "ignored fastly | Fastly-Client-IP | '1.1.1.1' | ", - "ignored cf | CF-Connecting-IP | '1.1.1.1' | ", - "empty ip | X-Forwarded-For | '' | ", - "missing ip | | | " + "Scenario | Header | Value | Expected Ip | Expected Client Ip ", + "xff | X-Forwarded-For | '1.1.1.1' | '1.1.1.1' | '1.1.1.1' ", + "xff last entry | X-Forwarded-For | '10.0.0.1, 1.1.1.1' | '1.1.1.1' | '1.1.1.1' ", + "xff multiple prefixes | X-Forwarded-For | '10.0.0.1, 1.1.1.1, 8.8.8.8' | '8.8.8.8' | '1.1.1.1' ", + "xff invalid prefix | X-Forwarded-For | 'invalid, 1.1.1.1' | '1.1.1.1' | '1.1.1.1' ", + "xff ipv6 | X-Forwarded-For | '2001:4860:4860::8888' | '2001:4860:4860::8888' | '2001:4860:4860:0:0:0:0:8888'", + "xff ipv6 last entry | X-Forwarded-For | 'fd00::1, 2001:4860:4860::8888' | '2001:4860:4860::8888' | '2001:4860:4860:0:0:0:0:8888'", + "xff private ipv6 | X-Forwarded-For | 'fd00::1' | 'fd00::1' | ", + "xff private last entry | X-Forwarded-For | '1.1.1.1, fd00::1' | 'fd00::1' | '1.1.1.1' ", + "xff padded last entry | X-Forwarded-For | '10.0.0.1, 1.1.1.1 ' | '1.1.1.1' | '1.1.1.1' ", + "xff empty last entry | X-Forwarded-For | '1.1.1.1, ' | | '1.1.1.1' ", + "ignored forwarded | Forwarded | 'for=1.1.1.1' | | '1.1.1.1' ", + "ignored real ip | X-Real-IP | '1.1.1.1' | | '1.1.1.1' ", + "ignored fastly | Fastly-Client-IP | '1.1.1.1' | | '1.1.1.1' ", + "ignored cf | CF-Connecting-IP | '1.1.1.1' | | '1.1.1.1' ", + "empty ip | X-Forwarded-For | '' | | ", + "missing ip | | | | " }) - void albReadsSourceIpFromXffOnly(String header, String value, String expectedIp) { + // The shared resolver ignores fd00::1; it still remains the unmodified source address. + void albReadsSourceIpFromXffOnly( + String header, String value, String expectedIp, String expectedClientIp) { for (boolean multiValue : new boolean[] {false, true}) { String jsonValue = value == null || value.isEmpty() ? "\"\"" : toJson(value); String headers = header == null ? "{}" : "{\"" + header + "\": " + (multiValue ? "[" + jsonValue + "]" : jsonValue) + "}"; - assertAlbClientIp(headers, multiValue, expectedIp); + assertAlbSourceAndClientIp(headers, multiValue, expectedIp, expectedClientIp); } } @TableTest({ - "Scenario | Xff | Forwarded | Expected Ip", - "xff wins | '1.1.1.1' | 'for=8.8.8.8' | '1.1.1.1' ", - "private xff wins | '10.0.0.1' | 'for=8.8.8.8' | '10.0.0.1' ", - "empty xff fallback | '' | 'for=8.8.8.8' | " + "Scenario | Xff | Forwarded | Expected Ip | Expected Client Ip", + "xff wins | '1.1.1.1' | 'for=8.8.8.8' | '1.1.1.1' | '1.1.1.1' ", + "private xff wins | '10.0.0.1' | 'for=8.8.8.8' | '10.0.0.1' | '8.8.8.8' ", + "empty xff fallback | '' | 'for=8.8.8.8' | | '8.8.8.8' " }) - void albIgnoresForwardedWhenResolvingSourceIp(String xff, String forwarded, String expectedIp) { - assertAlbClientIp( + void albIgnoresForwardedWhenResolvingSourceIp( + String xff, String forwarded, String expectedIp, String expectedClientIp) { + assertAlbSourceAndClientIp( "{\"X-Forwarded-For\": " + (xff.isEmpty() ? "\"\"" : toJson(xff)) + ", \"Forwarded\": " + toJson(forwarded) + "}", false, - expectedIp); + expectedIp, + expectedClientIp); } @TableTest({ @@ -543,25 +551,39 @@ void albIgnoresForwardedWhenResolvingSourceIp(String xff, String forwarded, Stri "websocket | '{\"headers\":%s,\"requestContext\":{\"connectionId\":\"c1\",\"routeKey\":\"$connect\",\"identity\":{\"sourceIp\":\"9.9.9.9\"}}}' | '9.9.9.9' " }) void otherTriggersKeepTheirExplicitEventSourceIp(String eventJson, String expectedIp) { - for (String headersJson : - new String[] { - "{}", - "{\"x-forwarded-for\":\"invalid\",\"forwarded\":\"for=unknown\"}", - "{\"x-forwarded-for\":\"1.1.1.1\"}", - "{\"x-forwarded-for\":\"10.0.0.1\"}", - "{\"x-forwarded-for\":\"invalid\",\"forwarded\":\"for=1.1.1.1\"}" - }) { - String[] peerIp = {null}; - setupMockCallbacks(new Callbacks().onSocketAddress((ip, port) -> peerIp[0] = ip)); - - AgentSpanContext context = - LambdaAppSecHandler.processRequestStart( - createInputStream(String.format(eventJson, headersJson))); - - assertNotNull(context, headersJson); - assertEquals(expectedIp, peerIp[0], headersJson); - assertNull(tagsOf(context).get(Tags.HTTP_CLIENT_IP), headersJson); - assertNull(tagsOf(context).get(Tags.NETWORK_CLIENT_IP), headersJson); + String[][] headerCases = { + {"{}", expectedIp}, + {"{\"x-forwarded-for\":\"invalid\",\"forwarded\":\"for=unknown\"}", expectedIp}, + {"{\"x-forwarded-for\":\"1.1.1.1\"}", "1.1.1.1"}, + {"{\"x-forwarded-for\":\"10.0.0.1\"}", expectedIp}, + {"{\"x-forwarded-for\":\"invalid\",\"forwarded\":\"for=1.1.1.1\"}", "1.1.1.1"} + }; + for (boolean resolverEnabled : new boolean[] {true, false}) { + injectSysConfig("trace.client-ip.resolver.enabled", String.valueOf(resolverEnabled)); + injectSysConfig("trace.client-ip.enabled", "false"); + for (String[] headerCase : headerCases) { + String headersJson = headerCase[0]; + String expectedClientIp = resolverEnabled ? headerCase[1] : null; + String[] peerIp = {null}; + List inferredAddresses = new ArrayList<>(); + setupMockCallbacks( + new Callbacks() + .onSocketAddress((ip, port) -> peerIp[0] = ip) + .onInferredAddress(inferredAddresses::add)); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart( + createInputStream(String.format(eventJson, headersJson))); + + assertNotNull(context, headersJson); + assertEquals(expectedIp, peerIp[0], headersJson); + assertEquals(expectedClientIp, tagsOf(context).get(Tags.HTTP_CLIENT_IP), headersJson); + assertEquals( + expectedClientIp == null ? emptyList() : singletonList(expectedClientIp), + inferredAddresses, + headersJson); + assertNull(tagsOf(context).get(Tags.NETWORK_CLIENT_IP), headersJson); + } } } @@ -2844,12 +2866,171 @@ void omitsRouteTagForAlbEvent() { assertEquals("alb-agent", tags.get(Tags.HTTP_USER_AGENT)); } + @ParameterizedTest + @MethodSource("clientIpResolutionCases") + void processRequestStartTagsResolvedClientIp( + String sourceIp, String headerName, String headerValue, String expectedIp) { + setupMockCallbacks(new Callbacks()); + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart(clientIpEvent(sourceIp, headerName, headerValue)); + assertNotNull(context); + assertEquals(expectedIp, tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + } + + @ParameterizedTest + @MethodSource("clientIpResolutionCases") + void processRequestStartPublishesResolvedClientIpToAppSec( + String sourceIp, String headerName, String headerValue, String expectedIp) { + List inferredAddresses = new ArrayList<>(); + setupMockCallbacks(new Callbacks().onInferredAddress(inferredAddresses::add)); + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart(clientIpEvent(sourceIp, headerName, headerValue)); + assertNotNull(context); + assertEquals(singletonList(expectedIp), inferredAddresses); + } + + @ParameterizedTest + @MethodSource("clientIpResolutionCases") + void processRequestStartSkipsClientIpResolutionWhenDisabled( + String sourceIp, String headerName, String headerValue, String expectedIp) { + injectSysConfig("trace.client-ip.resolver.enabled", "false"); + injectSysConfig("trace.client-ip.enabled", "false"); + assertFalse(Config.get().isTraceClientIpResolverEnabled()); + assertFalse(Config.get().isClientIpEnabled()); + + List inferredAddresses = new ArrayList<>(); + List sourceAddresses = new ArrayList<>(); + setupMockCallbacks( + new Callbacks() + .onInferredAddress(inferredAddresses::add) + .onSocketAddress((ip, port) -> sourceAddresses.add(ip))); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart(clientIpEvent(sourceIp, headerName, headerValue)); + + assertNotNull(context); + assertNotNull(((TagContext) context).getRequestContextDataAppSec()); + assertNull(tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + assertTrue(inferredAddresses.isEmpty()); + assertEquals(singletonList(sourceIp), sourceAddresses); + } + + @Test + void processRequestStartHonorsExplicitClientIpOptInWhenResolverIsDisabled() { + injectSysConfig("trace.client-ip.resolver.enabled", "false"); + injectSysConfig("trace.client-ip.enabled", "true"); + setupMockCallbacks(new Callbacks()); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart( + clientIpEvent("10.0.0.5", "x-forwarded-for", "1.2.3.4")); + + assertEquals("1.2.3.4", tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + } + + @Test + void processRequestStartResolvesConfiguredClientIpHeader() { + injectSysConfig(TRACE_CLIENT_IP_HEADER, "X-My-Client"); + setupMockCallbacks(new Callbacks()); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart( + clientIpEvent("10.0.0.5", "X-My-Client", "1.2.3.4")); + + assertEquals("1.2.3.4", tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + } + + @ParameterizedTest + @ValueSource( + strings = { + "x-real-ip", + "true-client-ip", + "x-client-ip", + "forwarded-for", + "x-cluster-client-ip", + "fastly-client-ip", + "cf-connecting-ip", + "cf-connecting-ipv6" + }) + void processRequestStartResolvesOtherSupportedClientIpHeaders(String header) { + setupMockCallbacks(new Callbacks()); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart(clientIpEvent(null, header, "1.2.3.4")); + + assertEquals("1.2.3.4", tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + } + + @Test + void processRequestStartResolvesForwardedHeader() { + setupMockCallbacks(new Callbacks()); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart( + clientIpEvent("10.0.0.5", "Forwarded", "for=1.2.3.4;proto=https")); + + assertEquals("1.2.3.4", tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + } + + @Test + void processRequestStartPublishesInferredAddressBeforeSourceAddress() { + List addresses = new ArrayList<>(); + setupMockCallbacks( + new Callbacks() + .onInferredAddress(ip -> addresses.add("inferred:" + ip)) + .onSocketAddress((ip, port) -> addresses.add("source:" + ip))); + + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart( + clientIpEvent("10.0.0.5", "x-forwarded-for", "1.2.3.4")); + + assertNotNull(context); + assertEquals(Arrays.asList("inferred:1.2.3.4", "source:10.0.0.5"), addresses); + } + + private static Object[][] clientIpResolutionCases() { + // Match HTTP instrumentation: prefer a public forwarded address, fall back to the event + // source, and prefer a public source over a private forwarded address. + return new Object[][] { + {"10.0.0.5", "X-Forwarded-For", "1.2.3.4, 10.0.0.2", "1.2.3.4"}, + {"10.0.0.5", "x-forwarded-for", "1.2.3.4", "1.2.3.4"}, + {"1.2.3.4", null, null, "1.2.3.4"}, + {"1.2.3.4", "x-forwarded-for", "invalid", "1.2.3.4"}, + {"1.2.3.4", "x-forwarded-for", "10.0.0.2", "1.2.3.4"}, + {"10.0.0.5", "x-forwarded-for", "2001:4860:4860::8888", "2001:4860:4860:0:0:0:0:8888"} + }; + } + + @Test + void processRequestStartOmitsClientIpWhenNoAddressIsAvailable() { + List inferredAddresses = new ArrayList<>(); + setupMockCallbacks(new Callbacks().onInferredAddress(inferredAddresses::add)); + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart(clientIpEvent(null, null, null)); + assertNotNull(context); + assertNull(tagsOf(context).get(Tags.HTTP_CLIENT_IP)); + assertTrue(inferredAddresses.isEmpty()); + } + + private static ByteArrayInputStream clientIpEvent( + String sourceIp, String headerName, String headerValue) { + String headers = headerName == null ? "" : ",\"" + headerName + "\":\"" + headerValue + "\""; + String identity = sourceIp == null ? "{}" : "{\"sourceIp\":\"" + sourceIp + "\"}"; + return createInputStream( + "{\"httpMethod\":\"POST\",\"path\":\"/login\",\"headers\":{\"host\":\"api.example.com\"" + + headers + + "},\"requestContext\":{\"httpMethod\":\"POST\",\"identity\":" + + identity + + "}}"); + } + @Test void mergeContextsCopiesHttpTagsIntoExtensionContext() { TagContext appSecContext = new TagContext(); appSecContext.putTag(Tags.COMPONENT, "aws-lambda"); appSecContext.putTag(Tags.HTTP_URL, "https://api.example.com/users/42"); appSecContext.putTag(Tags.HTTP_ROUTE, "/users/{id}"); + appSecContext.putTag(Tags.HTTP_CLIENT_IP, "1.2.3.4"); TagContext extensionContext = new TagContext(); AgentSpanContext merged = LambdaAppSecHandler.mergeContexts(extensionContext, appSecContext); @@ -2859,6 +3040,7 @@ void mergeContextsCopiesHttpTagsIntoExtensionContext() { assertEquals("aws-lambda", tags.get(Tags.COMPONENT)); assertEquals("https://api.example.com/users/42", tags.get(Tags.HTTP_URL)); assertEquals("/users/{id}", tags.get(Tags.HTTP_ROUTE)); + assertEquals("1.2.3.4", tags.get(Tags.HTTP_CLIENT_IP)); ((TagContext) merged).putTag(Tags.COMPONENT, "spring-web-controller"); assertEquals("spring-web-controller", tagsOf(merged).get(Tags.COMPONENT)); @@ -2916,28 +3098,40 @@ private static Map tagsOf(AgentSpanContext context) { return new HashMap<>(((TagContext) context).getTags()); } - private void assertAlbClientIp(String headersJson, boolean multiValue, String expectedIp) { + private void assertAlbSourceAndClientIp( + String headersJson, boolean multiValue, String expectedIp, String expectedClientIp) { String headerName = multiValue ? "multiValueHeaders" : "headers"; - String[] peerIp = {null}; String expectedSourceIp = expectedIp != null ? expectedIp : "0.0.0.0"; - setupMockCallbacks( - new Callbacks() - .onSocketAddress((ip, port) -> peerIp[0] = ip) - .onHeadersDone(() -> assertEquals(expectedSourceIp, peerIp[0], headerName))); - String eventJson = - "{\"path\":\"/alb\",\"httpMethod\":\"GET\",\"" - + headerName - + "\":" - + headersJson - + ",\"requestContext\":{\"elb\":{\"targetGroupArn\":\"arn\"}}}"; - - AgentSpanContext context = - LambdaAppSecHandler.processRequestStart(createInputStream(eventJson)); + for (boolean resolverEnabled : new boolean[] {true, false}) { + injectSysConfig("trace.client-ip.resolver.enabled", String.valueOf(resolverEnabled)); + injectSysConfig("trace.client-ip.enabled", "false"); + String expectedResolvedIp = resolverEnabled ? expectedClientIp : null; + String[] peerIp = {null}; + List inferredAddresses = new ArrayList<>(); + setupMockCallbacks( + new Callbacks() + .onSocketAddress((ip, port) -> peerIp[0] = ip) + .onInferredAddress(inferredAddresses::add) + .onHeadersDone(() -> assertEquals(expectedSourceIp, peerIp[0], headerName))); + String eventJson = + "{\"path\":\"/alb\",\"httpMethod\":\"GET\",\"" + + headerName + + "\":" + + headersJson + + ",\"requestContext\":{\"elb\":{\"targetGroupArn\":\"arn\"}}}"; - assertNotNull(context, headerName); - assertEquals(expectedSourceIp, peerIp[0], headerName); - assertNull(tagsOf(context).get(Tags.HTTP_CLIENT_IP), headerName); - assertNull(tagsOf(context).get(Tags.NETWORK_CLIENT_IP), headerName); + AgentSpanContext context = + LambdaAppSecHandler.processRequestStart(createInputStream(eventJson)); + + assertNotNull(context, headerName); + assertEquals(expectedSourceIp, peerIp[0], headerName); + assertEquals(expectedResolvedIp, tagsOf(context).get(Tags.HTTP_CLIENT_IP), headerName); + assertEquals( + expectedResolvedIp == null ? emptyList() : singletonList(expectedResolvedIp), + inferredAddresses, + headerName); + assertNull(tagsOf(context).get(Tags.NETWORK_CLIENT_IP), headerName); + } } private static ByteArrayInputStream createInputStream(String json) { @@ -2958,6 +3152,7 @@ private static class Callbacks { BiConsumer onMethodUri; BiConsumer onHeader; BiConsumer onSocketAddress; + Consumer onInferredAddress; Runnable onHeadersDone; Consumer> onPathParams; Consumer onBody; @@ -2983,6 +3178,11 @@ Callbacks onHeadersDone(Runnable cb) { return this; } + Callbacks onInferredAddress(Consumer cb) { + this.onInferredAddress = cb; + return this; + } + Callbacks onPathParams(Consumer> cb) { this.onPathParams = cb; return this; @@ -3044,6 +3244,19 @@ private void setupMockCallbacks(Callbacks callbacks) { .apply(any(), anyString(), anyInt()); } + BiFunction> inferredAddressCallback = null; + if (callbacks.onInferredAddress != null) { + inferredAddressCallback = mock(BiFunction.class); + Consumer capture = callbacks.onInferredAddress; + doAnswer( + inv -> { + capture.accept(inv.getArgument(1)); + return Flow.ResultFlow.empty(); + }) + .when(inferredAddressCallback) + .apply(any(), anyString()); + } + Function> headerDoneCallback = mock(Function.class); when(headerDoneCallback.apply(any())) .thenAnswer( @@ -3101,6 +3314,8 @@ private void setupMockCallbacks(Callbacks callbacks) { when(mockCallbackProvider.getCallback(EVENTS.requestHeader())).thenReturn(headerCallback); when(mockCallbackProvider.getCallback(EVENTS.requestClientSocketAddress())) .thenReturn(socketAddressCallback); + when(mockCallbackProvider.getCallback(EVENTS.requestInferredClientAddress())) + .thenReturn(inferredAddressCallback); when(mockCallbackProvider.getCallback(EVENTS.requestHeaderDone())) .thenReturn(headerDoneCallback); when(mockCallbackProvider.getCallback(EVENTS.requestPathParams())) From 2138f7ab3cc2712cd94b90069c29c08491f6a7ea Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Wed, 4 Nov 2026 17:40:41 +0100 Subject: [PATCH 2/7] Add Lambda request-start client IP benchmark --- .../core/LambdaRequestStartBenchmark.java | 144 ++++++++++++++++++ .../resources/lambda-benchmark-logback.xml | 3 + 2 files changed, 147 insertions(+) create mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java create mode 100644 dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java new file mode 100644 index 00000000000..da21721bd45 --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java @@ -0,0 +1,144 @@ +package datadog.trace.core; + +import static datadog.trace.api.gateway.Events.EVENTS; +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +import datadog.trace.api.Config; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.gateway.SubscriptionService; +import datadog.trace.bootstrap.ActiveSubsystems; +import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.lambda.LambdaAppSecHandler; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** Lambda request parsing and context creation, with lightweight callbacks and no WAF or I/O. */ +@State(Scope.Benchmark) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(NANOSECONDS) +@Threads(1) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@Fork( + value = 2, + jvmArgsAppend = { + "-Dlogback.configurationFile=lambda-benchmark-logback.xml", + "-Ddd.trace.startup.logs=false", + "-Ddd.trace.health.metrics.enabled=false" + }) +public class LambdaRequestStartBenchmark { + @Param({"true", "false"}) + public boolean resolverEnabled; + + @Param({"source", "forwarded", "ipv6", "malformed", "longChain"}) + public String payload; + + private CoreTracer tracer; + private AgentTracer.TracerAPI originalTracer; + private ByteArrayInputStream event; + + @Setup + public void setup(Blackhole blackhole) { + // Each parameter combination runs in its own JVM. Set flags before Config is initialized. + System.setProperty("dd.trace.client-ip.resolver.enabled", String.valueOf(resolverEnabled)); + System.setProperty("dd.trace.client-ip.enabled", "false"); + System.setProperty("dd.appsec.enabled", "true"); + System.setProperty("dd.remote_config.enabled", "false"); + System.setProperty("dd.telemetry.enabled", "false"); + if (Config.get().isTraceClientIpResolverEnabled() != resolverEnabled) { + throw new IllegalStateException( + "Resolver configuration was initialized before benchmark setup"); + } + originalTracer = AgentTracer.get(); + tracer = CoreTracer.builder().writer(new DropWriter(blackhole)).build(); + AgentTracer.forceRegister(tracer); + ActiveSubsystems.APPSEC_ACTIVE = true; + SubscriptionService callbacks = tracer.getSubscriptionService(RequestContextSlot.APPSEC); + callbacks.registerCallback(EVENTS.requestStarted(), () -> new Flow.ResultFlow<>(new Object())); + callbacks.registerCallback( + EVENTS.requestMethodUriRaw(), + (ctx, method, uri) -> { + blackhole.consume(uri); + return Flow.ResultFlow.empty(); + }); + callbacks.registerCallback( + EVENTS.requestHeader(), (ctx, name, value) -> blackhole.consume(value)); + callbacks.registerCallback( + EVENTS.requestClientSocketAddress(), + (ctx, ip, port) -> { + blackhole.consume(ip); + return Flow.ResultFlow.empty(); + }); + callbacks.registerCallback( + EVENTS.requestInferredClientAddress(), + (ctx, ip) -> { + blackhole.consume(ip); + return Flow.ResultFlow.empty(); + }); + callbacks.registerCallback(EVENTS.requestHeaderDone(), ctx -> Flow.ResultFlow.empty()); + String header; + switch (payload) { + case "source": + header = ""; + break; + case "forwarded": + header = "1.2.3.4, 10.0.0.2"; + break; + case "ipv6": + header = "2001:4860:4860::8888"; + break; + case "malformed": + header = "invalid, unknown, [broken"; + break; + case "longChain": + StringBuilder chain = new StringBuilder(); + for (int i = 0; i < 32; i++) { + chain.append("10.0.0.2, "); + } + header = chain.append("1.2.3.4").toString(); + break; + default: + throw new IllegalArgumentException(payload); + } + String json = + "{\"httpMethod\":\"POST\",\"path\":\"/login\",\"headers\":{\"host\":\"api.example.com\"" + + (header.isEmpty() ? "" : ",\"x-forwarded-for\":\"" + header + "\"") + + "},\"requestContext\":{\"httpMethod\":\"POST\",\"identity\":{\"sourceIp\":\"10.0.0.5\"}}}"; + event = new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)); + if (LambdaAppSecHandler.processRequestStart(event) == null) { + throw new IllegalStateException("Benchmark payload did not create an AppSec context"); + } + LambdaAppSecHandler.processRequestEnd(null); + } + + @Benchmark + public AgentSpanContext requestStart() { + AgentSpanContext context = LambdaAppSecHandler.processRequestStart(event); + // Clears per-invocation state without adding span creation or response processing costs. + LambdaAppSecHandler.processRequestEnd(null); + return context; + } + + @TearDown + public void tearDown() { + ActiveSubsystems.APPSEC_ACTIVE = false; + AgentTracer.forceRegister(originalTracer); + tracer.close(); + } +} diff --git a/dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml b/dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml new file mode 100644 index 00000000000..34a2e3aac50 --- /dev/null +++ b/dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml @@ -0,0 +1,3 @@ + + + From ce4a74748eadc7bf4b48d20c9968a7124fc1fe39 Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Thu, 5 Nov 2026 11:27:16 +0100 Subject: [PATCH 3/7] Expect resolved client IP in Lambda instrumentation test --- .../src/test/java/LambdaHandlerInstrumentationTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java index 39794288b21..c118dd8b341 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/java/LambdaHandlerInstrumentationTest.java @@ -515,6 +515,7 @@ void invocationSpanCarriesHttpTags() throws IOException { tag("request_id", is(REQUEST_ID)), tag("_dd.appsec.serverless.tracer", is(1)), tag(Tags.HTTP_METHOD, is("GET")), + tag(Tags.HTTP_CLIENT_IP, is("127.0.0.1")), // The tracer tags http.url without the query string; QueryObfuscator // obfuscates http.query.string and re-appends it as the trace is serialised tag(Tags.HTTP_URL, is("https://api.example.com/api/users/123?q=hello")), From 4506543cf9877d5847afc213074696d9e5db1a28 Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Fri, 6 Nov 2026 10:31:19 +0100 Subject: [PATCH 4/7] Share client IP resolution between HTTP and Lambda --- .../decorator/HttpServerDecorator.java | 17 +-- ...lientIpAddressResolverSpecification.groovy | 1 + dd-java-agent/testing/gradle.lockfile | 2 +- dd-trace-core/build.gradle | 4 - dd-trace-core/gradle.lockfile | 4 +- .../core/LambdaRequestStartBenchmark.java | 144 ------------------ .../resources/lambda-benchmark-logback.xml | 3 - .../trace/lambda/LambdaAppSecHandler.java | 14 +- .../opentracing/ShadowJarSmokeTest.java | 8 + .../api}/http/ClientIpAddressResolver.java | 21 ++- .../api}/http/utils/IPAddressUtil.java | 2 +- .../api/http/ClientIpAddressResolverTest.java | 72 +++++++++ 12 files changed, 108 insertions(+), 184 deletions(-) delete mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java delete mode 100644 dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml rename {dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator => internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api}/http/ClientIpAddressResolver.java (94%) rename {dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator => internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api}/http/utils/IPAddressUtil.java (99%) create mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java index 60ebcf8a1f9..4047f9c832e 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java @@ -41,10 +41,10 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.bootstrap.instrumentation.api.http.ClientIpAddressResolver; import datadog.trace.bootstrap.instrumentation.api.http.DatadogServerUrl; import datadog.trace.bootstrap.instrumentation.api.http.HttpServerUrl; import datadog.trace.bootstrap.instrumentation.api.http.OtelServerUrl; -import datadog.trace.bootstrap.instrumentation.decorator.http.ClientIpAddressResolver; import java.net.InetAddress; import java.util.BitSet; import java.util.Locale; @@ -386,20 +386,7 @@ protected void doOnRequest( String inferredAddressStr = null; boolean clientIpTagged = false; if (shouldResolveIp && extracted != null) { - InetAddress inferredAddress = ClientIpAddressResolver.resolve(extracted, span); - // the peer address should be used if: - // 1. the headers yield nothing, regardless of whether it is public or not - // 2. it is public and the headers yield a private address - if (peerIp != null) { - if (inferredAddress == null) { - inferredAddress = ClientIpAddressResolver.parseIpAddress(peerIp); - } else if (ClientIpAddressResolver.isIpAddrPrivate(inferredAddress)) { - InetAddress peerAddress = ClientIpAddressResolver.parseIpAddress(peerIp); - if (!ClientIpAddressResolver.isIpAddrPrivate(peerAddress)) { - inferredAddress = peerAddress; - } - } - } + InetAddress inferredAddress = ClientIpAddressResolver.resolve(extracted, span, peerIp); if (inferredAddress != null) { inferredAddressStr = inferredAddress.getHostAddress(); if (shouldTagIps) { diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy index fbc26ab4cd8..9f3f5be06ae 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy @@ -2,6 +2,7 @@ package datadog.trace.bootstrap.instrumentation.decorator.http import datadog.trace.api.interceptor.MutableSpan import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext +import datadog.trace.bootstrap.instrumentation.api.http.ClientIpAddressResolver import spock.lang.Specification class ClientIpAddressResolverSpecification extends Specification { diff --git a/dd-java-agent/testing/gradle.lockfile b/dd-java-agent/testing/gradle.lockfile index b9641077f64..b1d6fd3c2dd 100644 --- a/dd-java-agent/testing/gradle.lockfile +++ b/dd-java-agent/testing/gradle.lockfile @@ -60,7 +60,7 @@ org.apache.commons:commons-lang3:3.20.0=spotbugs org.apache.commons:commons-text:1.15.0=spotbugs org.apache.logging.log4j:log4j-api:2.26.1=spotbugs org.apache.logging.log4j:log4j-core:2.26.1=spotbugs -org.apiguardian:apiguardian-api:1.1.2=compileClasspath,runtimeClasspath,testCompileClasspath +org.apiguardian:apiguardian-api:1.1.2=compileClasspath,testCompileClasspath org.codehaus.groovy:groovy-ant:3.0.23=codenarc org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc org.codehaus.groovy:groovy-groovydoc:3.0.23=codenarc diff --git a/dd-trace-core/build.gradle b/dd-trace-core/build.gradle index 4008698db7d..7161ba9676d 100644 --- a/dd-trace-core/build.gradle +++ b/dd-trace-core/build.gradle @@ -81,10 +81,6 @@ dependencies { api project(':dd-trace-api') api project(':communication') api project(':internal-api') - // Lambda reuses the HTTP client-IP resolver already supplied by the agent bootstrap jar. - compileOnly project(':dd-java-agent:agent-bootstrap') - testImplementation project(':dd-java-agent:agent-bootstrap') - jmhImplementation project(':dd-java-agent:agent-bootstrap') api project(':products:metrics:metrics-agent') // Note: dd-trace-core should NOT depend on metrics-lib directly // metrics-lib (with sketches-java) is bundled in metrics-agent shadowJar diff --git a/dd-trace-core/gradle.lockfile b/dd-trace-core/gradle.lockfile index afb1a90cc78..e0bffcb8c85 100644 --- a/dd-trace-core/gradle.lockfile +++ b/dd-trace-core/gradle.lockfile @@ -10,7 +10,7 @@ com.amazonaws:aws-lambda-java-events:3.11.0=jmhRuntimeClasspath,testCompileClass com.blogspot.mydailyjava:weak-lock-free:0.17=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq.okhttp3:okhttp:3.12.15=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq.okio:okio:1.17.6=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath -com.datadoghq:dd-instrument-java:0.0.6=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath +com.datadoghq:dd-instrument-java:0.0.6=jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq:dd-javac-plugin-client:0.2.2=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath com.datadoghq:java-dogstatsd-client:4.4.5=jmhRuntimeClasspath,runtimeClasspath,testRuntimeClasspath,traceAgentTestRuntimeClasspath com.datadoghq:sketches-java:0.8.3=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testRuntimeClasspath,traceAgentTestRuntimeClasspath @@ -81,7 +81,7 @@ org.apache.commons:commons-math3:3.6.1=jmh,jmhCompileClasspath,jmhRuntimeClasspa org.apache.commons:commons-text:1.15.0=spotbugs org.apache.logging.log4j:log4j-api:2.26.1=spotbugs org.apache.logging.log4j:log4j-core:2.26.1=spotbugs -org.apiguardian:apiguardian-api:1.1.2=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath,traceAgentTestCompileClasspath,traceAgentTestRuntimeClasspath +org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath,traceAgentTestCompileClasspath org.checkerframework:checker-qual:3.33.0=testAnnotationProcessor,traceAgentTestAnnotationProcessor org.codehaus.groovy:groovy-ant:3.0.23=codenarc org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java deleted file mode 100644 index da21721bd45..00000000000 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/LambdaRequestStartBenchmark.java +++ /dev/null @@ -1,144 +0,0 @@ -package datadog.trace.core; - -import static datadog.trace.api.gateway.Events.EVENTS; -import static java.util.concurrent.TimeUnit.NANOSECONDS; - -import datadog.trace.api.Config; -import datadog.trace.api.gateway.Flow; -import datadog.trace.api.gateway.RequestContextSlot; -import datadog.trace.api.gateway.SubscriptionService; -import datadog.trace.bootstrap.ActiveSubsystems; -import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import datadog.trace.lambda.LambdaAppSecHandler; -import java.io.ByteArrayInputStream; -import java.nio.charset.StandardCharsets; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Measurement; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Param; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.Setup; -import org.openjdk.jmh.annotations.State; -import org.openjdk.jmh.annotations.TearDown; -import org.openjdk.jmh.annotations.Threads; -import org.openjdk.jmh.annotations.Warmup; -import org.openjdk.jmh.infra.Blackhole; - -/** Lambda request parsing and context creation, with lightweight callbacks and no WAF or I/O. */ -@State(Scope.Benchmark) -@BenchmarkMode(Mode.AverageTime) -@OutputTimeUnit(NANOSECONDS) -@Threads(1) -@Warmup(iterations = 3, time = 1) -@Measurement(iterations = 5, time = 1) -@Fork( - value = 2, - jvmArgsAppend = { - "-Dlogback.configurationFile=lambda-benchmark-logback.xml", - "-Ddd.trace.startup.logs=false", - "-Ddd.trace.health.metrics.enabled=false" - }) -public class LambdaRequestStartBenchmark { - @Param({"true", "false"}) - public boolean resolverEnabled; - - @Param({"source", "forwarded", "ipv6", "malformed", "longChain"}) - public String payload; - - private CoreTracer tracer; - private AgentTracer.TracerAPI originalTracer; - private ByteArrayInputStream event; - - @Setup - public void setup(Blackhole blackhole) { - // Each parameter combination runs in its own JVM. Set flags before Config is initialized. - System.setProperty("dd.trace.client-ip.resolver.enabled", String.valueOf(resolverEnabled)); - System.setProperty("dd.trace.client-ip.enabled", "false"); - System.setProperty("dd.appsec.enabled", "true"); - System.setProperty("dd.remote_config.enabled", "false"); - System.setProperty("dd.telemetry.enabled", "false"); - if (Config.get().isTraceClientIpResolverEnabled() != resolverEnabled) { - throw new IllegalStateException( - "Resolver configuration was initialized before benchmark setup"); - } - originalTracer = AgentTracer.get(); - tracer = CoreTracer.builder().writer(new DropWriter(blackhole)).build(); - AgentTracer.forceRegister(tracer); - ActiveSubsystems.APPSEC_ACTIVE = true; - SubscriptionService callbacks = tracer.getSubscriptionService(RequestContextSlot.APPSEC); - callbacks.registerCallback(EVENTS.requestStarted(), () -> new Flow.ResultFlow<>(new Object())); - callbacks.registerCallback( - EVENTS.requestMethodUriRaw(), - (ctx, method, uri) -> { - blackhole.consume(uri); - return Flow.ResultFlow.empty(); - }); - callbacks.registerCallback( - EVENTS.requestHeader(), (ctx, name, value) -> blackhole.consume(value)); - callbacks.registerCallback( - EVENTS.requestClientSocketAddress(), - (ctx, ip, port) -> { - blackhole.consume(ip); - return Flow.ResultFlow.empty(); - }); - callbacks.registerCallback( - EVENTS.requestInferredClientAddress(), - (ctx, ip) -> { - blackhole.consume(ip); - return Flow.ResultFlow.empty(); - }); - callbacks.registerCallback(EVENTS.requestHeaderDone(), ctx -> Flow.ResultFlow.empty()); - String header; - switch (payload) { - case "source": - header = ""; - break; - case "forwarded": - header = "1.2.3.4, 10.0.0.2"; - break; - case "ipv6": - header = "2001:4860:4860::8888"; - break; - case "malformed": - header = "invalid, unknown, [broken"; - break; - case "longChain": - StringBuilder chain = new StringBuilder(); - for (int i = 0; i < 32; i++) { - chain.append("10.0.0.2, "); - } - header = chain.append("1.2.3.4").toString(); - break; - default: - throw new IllegalArgumentException(payload); - } - String json = - "{\"httpMethod\":\"POST\",\"path\":\"/login\",\"headers\":{\"host\":\"api.example.com\"" - + (header.isEmpty() ? "" : ",\"x-forwarded-for\":\"" + header + "\"") - + "},\"requestContext\":{\"httpMethod\":\"POST\",\"identity\":{\"sourceIp\":\"10.0.0.5\"}}}"; - event = new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)); - if (LambdaAppSecHandler.processRequestStart(event) == null) { - throw new IllegalStateException("Benchmark payload did not create an AppSec context"); - } - LambdaAppSecHandler.processRequestEnd(null); - } - - @Benchmark - public AgentSpanContext requestStart() { - AgentSpanContext context = LambdaAppSecHandler.processRequestStart(event); - // Clears per-invocation state without adding span creation or response processing costs. - LambdaAppSecHandler.processRequestEnd(null); - return context; - } - - @TearDown - public void tearDown() { - ActiveSubsystems.APPSEC_ACTIVE = false; - AgentTracer.forceRegister(originalTracer); - tracer.close(); - } -} diff --git a/dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml b/dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml deleted file mode 100644 index 34a2e3aac50..00000000000 --- a/dd-trace-core/src/jmh/resources/lambda-benchmark-logback.xml +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java index 5370c82627a..171782ffda5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java @@ -35,7 +35,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.URIUtils; -import datadog.trace.bootstrap.instrumentation.decorator.http.ClientIpAddressResolver; +import datadog.trace.bootstrap.instrumentation.api.http.ClientIpAddressResolver; import datadog.trace.lambda.LambdaEventParser.LambdaRequestData; import datadog.trace.lambda.LambdaEventParser.LambdaResponseData; import datadog.trace.lambda.LambdaEventParser.LambdaTriggerType; @@ -555,17 +555,7 @@ private static String resolveClientIp(LambdaRequestData eventData) { null, TracePropagationStyle.NONE, DDTraceId.ZERO); - InetAddress inferred = ClientIpAddressResolver.resolve(extracted, null); - if (eventData.sourceIp != null) { - if (inferred == null) { - inferred = ClientIpAddressResolver.parseIpAddress(eventData.sourceIp); - } else if (ClientIpAddressResolver.isIpAddrPrivate(inferred)) { - InetAddress source = ClientIpAddressResolver.parseIpAddress(eventData.sourceIp); - if (source != null && !ClientIpAddressResolver.isIpAddrPrivate(source)) { - inferred = source; - } - } - } + InetAddress inferred = ClientIpAddressResolver.resolve(extracted, null, eventData.sourceIp); return inferred == null ? null : inferred.getHostAddress(); } diff --git a/dd-trace-ot/src/shadowJarSmokeTest/java/datadog/opentracing/ShadowJarSmokeTest.java b/dd-trace-ot/src/shadowJarSmokeTest/java/datadog/opentracing/ShadowJarSmokeTest.java index 10cc8218dc3..3745bc0fe10 100644 --- a/dd-trace-ot/src/shadowJarSmokeTest/java/datadog/opentracing/ShadowJarSmokeTest.java +++ b/dd-trace-ot/src/shadowJarSmokeTest/java/datadog/opentracing/ShadowJarSmokeTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import ddtrot.dd.trace.bootstrap.instrumentation.api.http.ClientIpAddressResolver; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.propagation.Format; @@ -10,11 +11,18 @@ import java.util.HashMap; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; /** Smoke tests for the shaded OpenTracing distribution and its public dependencies. */ class ShadowJarSmokeTest { + @Test + void resolvesClientIpWithoutAgentBootstrap() { + assertEquals( + "1.2.3.4", ClientIpAddressResolver.resolve(null, null, "1.2.3.4").getHostAddress()); + } + @ParameterizedTest @ValueSource(strings = {"datadog", "tracecontext", "datadog,tracecontext"}) void injectAndExtractWithoutLlmObservability(String propagationStyle) throws Exception { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolver.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java similarity index 94% rename from dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolver.java rename to internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java index 6a4535ba9ac..14ad40a72ff 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolver.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java @@ -1,8 +1,8 @@ -package datadog.trace.bootstrap.instrumentation.decorator.http; +package datadog.trace.bootstrap.instrumentation.api.http; import datadog.trace.api.interceptor.MutableSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; -import datadog.trace.bootstrap.instrumentation.decorator.http.utils.IPAddressUtil; +import datadog.trace.bootstrap.instrumentation.api.http.utils.IPAddressUtil; import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; @@ -37,6 +37,23 @@ public static InetAddress resolve(AgentSpanContext.Extracted context, MutableSpa } } + /** + * Resolves the client address from headers, falling back to the peer when headers yield no + * address or a private address and the peer is public. An invalid peer never replaces a valid + * header address. + */ + public static InetAddress resolve( + AgentSpanContext.Extracted context, MutableSpan span, String peerIp) { + InetAddress inferred = resolve(context, span); + if (peerIp != null && (inferred == null || isIpAddrPrivate(inferred))) { + InetAddress peer = parseIpAddress(peerIp); + if (peer != null && (inferred == null || !isIpAddrPrivate(peer))) { + return peer; + } + } + return inferred; + } + private static InetAddress doResolve(AgentSpanContext.Extracted context, MutableSpan span) { if (context == null) { return null; diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/utils/IPAddressUtil.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtil.java similarity index 99% rename from dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/utils/IPAddressUtil.java rename to internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtil.java index 6db6182acdf..cb78c6beaea 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/utils/IPAddressUtil.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtil.java @@ -1,4 +1,4 @@ -package datadog.trace.bootstrap.instrumentation.decorator.http.utils; +package datadog.trace.bootstrap.instrumentation.api.http.utils; /** * This is exact copy of sun.net.util.IPAddressUtil Used copy because class diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java new file mode 100644 index 00000000000..5bb905413bc --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java @@ -0,0 +1,72 @@ +package datadog.trace.bootstrap.instrumentation.api.http; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.TracePropagationStyle; +import datadog.trace.api.sampling.PrioritySampling; +import datadog.trace.bootstrap.instrumentation.api.TagContext; +import java.net.InetAddress; +import java.net.UnknownHostException; +import org.junit.jupiter.api.Test; +import org.tabletest.junit.TableTest; + +class ClientIpAddressResolverTest { + @TableTest({ + "Scenario | Header | Peer | Expected ", + "no addresses | | | ", + "public peer fallback | | 1.2.3.4 | 1.2.3.4 ", + "private peer fallback | | 10.0.0.5 | 10.0.0.5 ", + "invalid peer | | invalid | ", + "invalid header fallback | invalid | 1.2.3.4 | 1.2.3.4 ", + "public header beats peer | 1.2.3.4 | 8.8.8.8 | 1.2.3.4 ", + "public header without peer | 1.2.3.4 | | 1.2.3.4 ", + "public peer beats private | 10.0.0.2 | 1.2.3.4 | 1.2.3.4 ", + "private header beats private | 10.0.0.2 | 10.0.0.5 | 10.0.0.2 ", + "private header without peer | 10.0.0.2 | | 10.0.0.2 ", + "invalid peer preserves header | 10.0.0.2 | invalid | 10.0.0.2 ", + "forwarded chain | '10.0.0.2, 1.2.3.4' | 8.8.8.8 | 1.2.3.4 ", + "public IPv6 header | 2001:4860:4860::8888 | 1.2.3.4 | 2001:4860:4860::8888", + "public peer beats loopback | ::1 | 1.2.3.4 | 1.2.3.4 " + }) + void resolvesHeadersAndPeer(String header, String peer, String expected) + throws UnknownHostException { + assertEquals( + address(expected), ClientIpAddressResolver.resolve(context(header, null), null, peer)); + } + + @TableTest({ + "Scenario | Custom | Peer | Expected", + "custom overrides standard | 8.8.8.8 | 1.2.3.4 | 8.8.8.8 ", + "custom forwarded syntax | 'for=8.8.8.8;proto=https' | 1.2.3.4 | 8.8.8.8 ", + "invalid custom uses peer | invalid | 1.2.3.4 | 1.2.3.4 " + }) + void honorsCustomHeader(String custom, String peer, String expected) throws UnknownHostException { + assertEquals( + address(expected), ClientIpAddressResolver.resolve(context("9.9.9.9", custom), null, peer)); + } + + @Test + void resolvesPeerWithoutExtractedContext() throws UnknownHostException { + assertEquals(address("1.2.3.4"), ClientIpAddressResolver.resolve(null, null, "1.2.3.4")); + } + + private static InetAddress address(String ip) throws UnknownHostException { + return ip == null ? null : InetAddress.getByName(ip); + } + + private static TagContext context(String forwarded, String custom) { + TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); + headers.xForwardedFor = forwarded; + headers.customIpHeader = custom; + return new TagContext( + null, + null, + headers, + null, + PrioritySampling.UNSET, + null, + TracePropagationStyle.NONE, + DDTraceId.ZERO); + } +} From a963f63fe7bdf791916b324bf082802c17815544 Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Fri, 6 Nov 2026 11:02:21 +0100 Subject: [PATCH 5/7] Move client IP resolver tests into internal-api --- ...lientIpAddressResolverSpecification.groovy | 195 ------------------ .../api/http/ClientIpAddressResolverTest.java | 187 +++++++++++++++++ .../api/http/utils/IPAddressUtilTest.java | 93 +++++++++ 3 files changed, 280 insertions(+), 195 deletions(-) delete mode 100644 dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy create mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtilTest.java diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy deleted file mode 100644 index 9f3f5be06ae..00000000000 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy +++ /dev/null @@ -1,195 +0,0 @@ -package datadog.trace.bootstrap.instrumentation.decorator.http - -import datadog.trace.api.interceptor.MutableSpan -import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext -import datadog.trace.bootstrap.instrumentation.api.http.ClientIpAddressResolver -import spock.lang.Specification - -class ClientIpAddressResolverSpecification extends Specification { - - void 'test with custom header value=#headerValue'() { - setup: - MutableSpan span = Stub() - def context = Mock(AgentSpanContext.Extracted) - 1 * context.getCustomIpHeader() >> headerValue - - expect: - InetAddress resultInetAddr = result ? InetAddress.getByName(result) : null - ClientIpAddressResolver.resolve(context, span) == resultInetAddr - - where: - headerValue | result - '127.0.0.1, 8.8.8.8' | '8.8.8.8' - 'for="[::1]", for=8.8.8.8' | '8.8.8.8' - 'for="8.8.8.8:8888"' | '8.8.8.8' - 'for="[::1]", for=[::ffff:1.1.1.1]:8888' | '1.1.1.1' // though we allow this, the correct form is the next - 'for="[::1]", for="[::ffff:1.1.1.1]:8888"' | '1.1.1.1' - '10.0.0.1' | '10.0.0.1' - } - - private static String headerToCamelCase(String headerName) { - headerName.split('-').collect {it.capitalize()}.join() - } - - void 'test with standard header #header=#headerValue'() { - setup: - MutableSpan span = Stub() - def method = "get${headerToCamelCase(header)}" - def context = Mock(AgentSpanContext.Extracted) - 1 * context."$method"() >> headerValue - - expect: - InetAddress resultInetAddr = result ? InetAddress.getByName(result) : null - ClientIpAddressResolver.resolve(context, span) == resultInetAddr - - where: - header | headerValue | result - 'x-forwarded-for' | '2001:0::1' | '2001::1' - 'x-forwarded-for' | '::1, febf::1, fc00::1, fd00::1,2001:0000::1' | '2001::1' - 'x-forwarded-for' | 'fec0::,feff::ffff,fd00::,fdff::ffff,2001::1' | '2001::1' - 'x-forwarded-for' | '172.16.0.1' | '172.16.0.1' - 'x-forwarded-for' | '172.16.0.1, 172.31.255.254, 172.32.255.1, 8.8.8.8' | '172.32.255.1' - 'x-forwarded-for' | '169.254.0.1, 127.1.1.1, 10.255.255.254,' | '169.254.0.1' - 'x-forwarded-for' | '127.1.1.1,, ' | '127.1.1.1' - 'x-forwarded-for' | 'bad_value, 1.1.1.1' | '1.1.1.1' - - 'x-real-ip' | '2.2.2.2' | '2.2.2.2' - 'x-real-ip' | '2.2.2.2, 3.3.3.3' | '2.2.2.2' - 'x-real-ip' | '127.0.0.1' | '127.0.0.1' - 'x-real-ip' | '::ffff:4.4.4.4' | '4.4.4.4' - 'x-real-ip' | '::ffff:127.0.0.1' | '127.0.0.1' - 'x-real-ip' | '42' | '0.0.0.42' - - 'x-client-ip' | '2.2.2.2' | '2.2.2.2' - - 'x-cluster-client-ip' | '2.2.2.2' | '2.2.2.2' - - 'forwarded-for' | '::1, 127.0.0.1, 2001::1' | '2001::1' - - 'true-client-ip' | '8.8.8.8' | '8.8.8.8' - - 'fastly-client-ip' | '3.3.3.3' | '3.3.3.3' - 'cf-connecting-ip' | '4.4.4.4' | '4.4.4.4' - 'cf-connecting-ipv6' | '2001::2' | '2001::2' - - 'forwarded' | 'for="[2001::1]:1111"' | '2001::1' - 'forwarded' | 'fOr="[2001::1]:1111"' | '2001::1' - 'forwarded' | 'for=some_host' | null - 'forwarded' | 'for=127.0.0.1, FOR=1.1.1.1' | '1.1.1.1' - 'forwarded' |'for="\"foobar";proto=http,FOR="1.1.1.1"' | '1.1.1.1' - 'forwarded' | 'for="8.8.8.8:2222",' | '8.8.8.8' - 'forwarded' | 'for="8.8.8.8' | null // quote not closed - 'forwarded' | 'far="8.8.8.8",for=4.4.4.4;' | '4.4.4.4' - 'forwarded' | ' for=127.0.0.1,for= for=,for=;"for = for="" ,; for=8.8.8.8;' | '8.8.8.8' - 'forwarded' | 'for=192.0.2.60;proto=http;by=203.0.113.43' | '192.0.2.60' - 'forwarded' | 'For="[2001:db8:cafe::17]:4711"' | '2001:db8:cafe::17' - 'forwarded' | 'for=192.0.2.43;proto=https;by=203.0.113.43' | '192.0.2.43' - 'forwarded' | 'for="_gazonk"' | null - 'forwarded' | 'for=unknown, for=8.8.8.8' | '8.8.8.8' - 'forwarded' | 'for="[::ffff:192.0.2.128]";proto=http' | '192.0.2.128' - } - - void 'test recognition strategy with custom header'() { - setup: - MutableSpan span = Stub() - def context = Mock(AgentSpanContext.Extracted) - - when: - def ip = ClientIpAddressResolver.resolve(context, span) - - then: - 1 * context.getCustomIpHeader() >> '8.8.8.8' - 0 * _ - - ip == InetAddress.getByName('8.8.8.8') - } - - void 'test recognition strategy without custom header'() { - setup: - MutableSpan span = Mock() - def context = Mock(AgentSpanContext.Extracted) - - when: - def ip = ClientIpAddressResolver.resolve(context, span) - - then: - 1 * context.getCustomIpHeader() >> null - - then: - 1 * context.getXForwardedFor() >> null - - then: - 1 * context.getXRealIp() >> '127.0.0.1' - - then: - 1 * context.getTrueClientIp() >> null - - then: - 1 * context.getXClientIp() >> null - - then: - 1 * context.getForwarded() >> null - - then: - 1 * context.getForwardedFor() >> null - - then: - 1 * context.getXClusterClientIp() >> null - - then: - 1 * context.getFastlyClientIp() >> null - - then: - 1 * context.getCfConnectingIp() >> null - - then: - 1 * context.getCfConnectingIpv6() >> '2001::1' - 0 * _ - - ip == InetAddress.getByName('2001::1') - } - - void 'no custom header public IP address is preferred'() { - setup: - MutableSpan span = Mock() - def context = Mock(AgentSpanContext.Extracted) - - when: - def ip = ClientIpAddressResolver.resolve(context, span) - - then: - 1 * context.getCustomIpHeader() >> null - - then: - 1 * context.getXRealIp() >> '127.0.0.1' - 1 * context.getXClientIp() >> '8.8.8.8' - - ip == InetAddress.getByName('8.8.8.8') - } - - void 'no custom header all headers are reported'() { - setup: - MutableSpan span = Mock() - def context = Mock(AgentSpanContext.Extracted) - - when: - def ip = ClientIpAddressResolver.resolve(context, span) - - then: - 1 * context.getCustomIpHeader() >> null - - then: - 1 * context.getXForwardedFor() >> '127.0.0.1' - 1 * context.getXRealIp() >> '127.0.0.2' - 1 * context.getXClientIp() >> '127.0.0.3' - 1 * context.getForwarded() >> 'for=127.0.0.4' - 1 * context.getXClusterClientIp() >> '127.0.0.5' - 1 * context.getForwardedFor() >> '127.0.0.6' - 1 * context.getTrueClientIp() >> '127.0.0.9' - 1 * context.getFastlyClientIp() >> '127.0.0.10' - 1 * context.getCfConnectingIp() >> '127.0.0.11' - 1 * context.getCfConnectingIpv6() >> '::1' - - ip == InetAddress.getByName('127.0.0.1') - } -} diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java index 5bb905413bc..613f37c940b 100644 --- a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java @@ -1,14 +1,22 @@ package datadog.trace.bootstrap.instrumentation.api.http; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import datadog.trace.api.DDTraceId; import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; +import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; import datadog.trace.bootstrap.instrumentation.api.TagContext; import java.net.InetAddress; import java.net.UnknownHostException; import org.junit.jupiter.api.Test; +import org.mockito.InOrder; import org.tabletest.junit.TableTest; class ClientIpAddressResolverTest { @@ -51,6 +59,181 @@ void resolvesPeerWithoutExtractedContext() throws UnknownHostException { assertEquals(address("1.2.3.4"), ClientIpAddressResolver.resolve(null, null, "1.2.3.4")); } + @TableTest({ + "Scenario | Value | Expected ", + "case 1 | '127.0.0.1, 8.8.8.8' | '8.8.8.8' ", + "case 2 | 'for=\"[::1]\", for=8.8.8.8' | '8.8.8.8' ", + "case 3 | 'for=\"8.8.8.8:8888\"' | '8.8.8.8' ", + "case 4 | 'for=\"[::1]\", for=[::ffff:1.1.1.1]:8888' | '1.1.1.1' ", + "case 5 | 'for=\"[::1]\", for=\"[::ffff:1.1.1.1]:8888\"' | '1.1.1.1' ", + "case 6 | '10.0.0.1' | '10.0.0.1'" + }) + void resolvesCustomHeaders(String value, String expected) throws UnknownHostException { + assertEquals(address(expected), ClientIpAddressResolver.resolve(context(null, value), null)); + } + + @TableTest({ + "Scenario | Header | Value | Expected ", + "case 1 | 'x-forwarded-for' | '2001:0::1' | '2001::1' ", + "case 2 | 'x-forwarded-for' | '::1, febf::1, fc00::1, fd00::1,2001:0000::1' | '2001::1' ", + "case 3 | 'x-forwarded-for' | 'fec0::,feff::ffff,fd00::,fdff::ffff,2001::1' | '2001::1' ", + "case 4 | 'x-forwarded-for' | '172.16.0.1' | '172.16.0.1' ", + "case 5 | 'x-forwarded-for' | '172.16.0.1, 172.31.255.254, 172.32.255.1, 8.8.8.8' | '172.32.255.1' ", + "case 6 | 'x-forwarded-for' | '169.254.0.1, 127.1.1.1, 10.255.255.254,' | '169.254.0.1' ", + "case 7 | 'x-forwarded-for' | '127.1.1.1,, ' | '127.1.1.1' ", + "case 8 | 'x-forwarded-for' | 'bad_value, 1.1.1.1' | '1.1.1.1' ", + "case 9 | 'x-real-ip' | '2.2.2.2' | '2.2.2.2' ", + "case 10 | 'x-real-ip' | '2.2.2.2, 3.3.3.3' | '2.2.2.2' ", + "case 11 | 'x-real-ip' | '127.0.0.1' | '127.0.0.1' ", + "case 12 | 'x-real-ip' | '::ffff:4.4.4.4' | '4.4.4.4' ", + "case 13 | 'x-real-ip' | '::ffff:127.0.0.1' | '127.0.0.1' ", + "case 14 | 'x-real-ip' | '42' | '0.0.0.42' ", + "case 15 | 'x-client-ip' | '2.2.2.2' | '2.2.2.2' ", + "case 16 | 'x-cluster-client-ip' | '2.2.2.2' | '2.2.2.2' ", + "case 17 | 'forwarded-for' | '::1, 127.0.0.1, 2001::1' | '2001::1' ", + "case 18 | 'true-client-ip' | '8.8.8.8' | '8.8.8.8' ", + "case 19 | 'fastly-client-ip' | '3.3.3.3' | '3.3.3.3' ", + "case 20 | 'cf-connecting-ip' | '4.4.4.4' | '4.4.4.4' ", + "case 21 | 'cf-connecting-ipv6' | '2001::2' | '2001::2' ", + "case 22 | 'forwarded' | 'for=\"[2001::1]:1111\"' | '2001::1' ", + "case 23 | 'forwarded' | 'fOr=\"[2001::1]:1111\"' | '2001::1' ", + "case 24 | 'forwarded' | 'for=some_host' | ", + "case 25 | 'forwarded' | 'for=127.0.0.1, FOR=1.1.1.1' | '1.1.1.1' ", + "case 26 | 'forwarded' | 'for=\"\"foobar\";proto=http,FOR=\"1.1.1.1\"' | '1.1.1.1' ", + "case 27 | 'forwarded' | 'for=\"8.8.8.8:2222\",' | '8.8.8.8' ", + "case 28 | 'forwarded' | 'for=\"8.8.8.8' | ", + "case 29 | 'forwarded' | 'far=\"8.8.8.8\",for=4.4.4.4;' | '4.4.4.4' ", + "case 30 | 'forwarded' | ' for=127.0.0.1,for= for=,for=;\"for = for=\"\" ,; for=8.8.8.8;' | '8.8.8.8' ", + "case 31 | 'forwarded' | 'for=192.0.2.60;proto=http;by=203.0.113.43' | '192.0.2.60' ", + "case 32 | 'forwarded' | 'For=\"[2001:db8:cafe::17]:4711\"' | '2001:db8:cafe::17'", + "case 33 | 'forwarded' | 'for=192.0.2.43;proto=https;by=203.0.113.43' | '192.0.2.43' ", + "case 34 | 'forwarded' | 'for=\"_gazonk\"' | ", + "case 35 | 'forwarded' | 'for=unknown, for=8.8.8.8' | '8.8.8.8' ", + "case 36 | 'forwarded' | 'for=\"[::ffff:192.0.2.128]\";proto=http' | '192.0.2.128' " + }) + void resolvesStandardHeaders(String header, String value, String expected) + throws UnknownHostException { + assertEquals( + address(expected), ClientIpAddressResolver.resolve(headerContext(header, value), null)); + } + + @Test + void customHeaderIsExclusive() throws UnknownHostException { + AgentSpanContext.Extracted context = mock(AgentSpanContext.Extracted.class); + when(context.getCustomIpHeader()).thenReturn("8.8.8.8"); + assertEquals(address("8.8.8.8"), ClientIpAddressResolver.resolve(context, null)); + verify(context).getCustomIpHeader(); + verifyNoMoreInteractions(context); + } + + @Test + void readsStandardHeadersInPriorityOrder() throws UnknownHostException { + AgentSpanContext.Extracted context = mock(AgentSpanContext.Extracted.class); + when(context.getXRealIp()).thenReturn("127.0.0.1"); + when(context.getCfConnectingIpv6()).thenReturn("2001::1"); + assertEquals(address("2001::1"), ClientIpAddressResolver.resolve(context, null)); + verifyAllHeaderReads(context); + } + + @Test + void publicHeaderIsPreferredAndStopsFurtherReads() throws UnknownHostException { + AgentSpanContext.Extracted context = mock(AgentSpanContext.Extracted.class); + when(context.getXRealIp()).thenReturn("127.0.0.1"); + when(context.getXClientIp()).thenReturn("8.8.8.8"); + assertEquals(address("8.8.8.8"), ClientIpAddressResolver.resolve(context, null)); + InOrder order = inOrder(context); + order.verify(context).getCustomIpHeader(); + order.verify(context).getXForwardedFor(); + order.verify(context).getXRealIp(); + order.verify(context).getTrueClientIp(); + order.verify(context).getXClientIp(); + verifyNoMoreInteractions(context); + } + + @Test + void firstPrivateHeaderIsPreservedAfterReadingAllHeaders() throws UnknownHostException { + AgentSpanContext.Extracted context = mock(AgentSpanContext.Extracted.class); + when(context.getXForwardedFor()).thenReturn("127.0.0.1"); + when(context.getXRealIp()).thenReturn("127.0.0.2"); + when(context.getXClientIp()).thenReturn("127.0.0.3"); + when(context.getForwarded()).thenReturn("for=127.0.0.4"); + when(context.getXClusterClientIp()).thenReturn("127.0.0.5"); + when(context.getForwardedFor()).thenReturn("127.0.0.6"); + when(context.getTrueClientIp()).thenReturn("127.0.0.9"); + when(context.getFastlyClientIp()).thenReturn("127.0.0.10"); + when(context.getCfConnectingIp()).thenReturn("127.0.0.11"); + when(context.getCfConnectingIpv6()).thenReturn("::1"); + assertEquals(address("127.0.0.1"), ClientIpAddressResolver.resolve(context, null)); + verifyAllHeaderReads(context); + } + + @Test + void unexpectedHeaderReadFailureDoesNotEscape() { + AgentSpanContext.Extracted context = mock(AgentSpanContext.Extracted.class); + when(context.getCustomIpHeader()).thenThrow(new IllegalStateException("unreadable carrier")); + assertNull(ClientIpAddressResolver.resolve(context, null)); + } + + @Test + void nullContextHasNoHeaderAddress() { + assertNull(ClientIpAddressResolver.resolve(null, null)); + } + + private static void verifyAllHeaderReads(AgentSpanContext.Extracted context) { + InOrder order = inOrder(context); + order.verify(context).getCustomIpHeader(); + order.verify(context).getXForwardedFor(); + order.verify(context).getXRealIp(); + order.verify(context).getTrueClientIp(); + order.verify(context).getXClientIp(); + order.verify(context).getForwarded(); + order.verify(context).getForwardedFor(); + order.verify(context).getXClusterClientIp(); + order.verify(context).getFastlyClientIp(); + order.verify(context).getCfConnectingIp(); + order.verify(context).getCfConnectingIpv6(); + verifyNoMoreInteractions(context); + } + + private static TagContext headerContext(String name, String value) { + TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); + switch (name) { + case "x-forwarded-for": + headers.xForwardedFor = value; + break; + case "x-real-ip": + headers.xRealIp = value; + break; + case "true-client-ip": + headers.trueClientIp = value; + break; + case "x-client-ip": + headers.xClientIp = value; + break; + case "forwarded": + headers.forwarded = value; + break; + case "forwarded-for": + headers.forwardedFor = value; + break; + case "x-cluster-client-ip": + headers.xClusterClientIp = value; + break; + case "fastly-client-ip": + headers.fastlyClientIp = value; + break; + case "cf-connecting-ip": + headers.cfConnectingIp = value; + break; + case "cf-connecting-ipv6": + headers.cfConnectingIpv6 = value; + break; + default: + throw new IllegalArgumentException(name); + } + return context(headers); + } + private static InetAddress address(String ip) throws UnknownHostException { return ip == null ? null : InetAddress.getByName(ip); } @@ -59,6 +242,10 @@ private static TagContext context(String forwarded, String custom) { TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); headers.xForwardedFor = forwarded; headers.customIpHeader = custom; + return context(headers); + } + + private static TagContext context(TagContext.HttpHeaders headers) { return new TagContext( null, null, diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtilTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtilTest.java new file mode 100644 index 00000000000..31d8f50eb05 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/utils/IPAddressUtilTest.java @@ -0,0 +1,93 @@ +package datadog.trace.bootstrap.instrumentation.api.http.utils; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import org.junit.jupiter.api.Test; +import org.tabletest.junit.TableTest; + +class IPAddressUtilTest { + @TableTest({ + "Input | Expected ", + "0.0.0.0 | 0.0.0.0 ", + "255.255.255.255 | 255.255.255.255", + "127.1 | 127.0.0.1 ", + "127.0.1 | 127.0.0.1 ", + "2130706433 | 127.0.0.1 ", + "001.002.003.004 | 1.2.3.4 ", + "'' | ", + "1234567890123456 | ", + ".1.2.3 | ", + "1..2.3 | ", + "1.2.3. | ", + "1.2.3.4.5 | ", + "256.1.2.3 | ", + "1.2.65536 | ", + "4294967296 | ", + "-1.2.3.4 | ", + "a.2.3.4 | ", + "' 1.2.3.4' | " + }) + void parsesIPv4Literals(String input, String expected) throws UnknownHostException { + assertArrayEquals(bytes(expected), IPAddressUtil.textToNumericFormatV4(input)); + assertEquals(expected != null, IPAddressUtil.isIPv4LiteralAddress(input)); + } + + @TableTest({ + "Input | Expected ", + ":: | :: ", + "::1 | ::1 ", + "2001:db8::1 | 2001:db8::1 ", + "2001:db8:0:0:1:2:3:4 | 2001:db8:0:0:1:2:3:4", + "1:2:3:4:5:6:7:8 | 1:2:3:4:5:6:7:8 ", + "1:2:3:4:5:6:7:: | 1:2:3:4:5:6:7:: ", + "::ffff:192.0.2.1 | 192.0.2.1 ", + "2001:db8::192.0.2.1 | 2001:db8::192.0.2.1 ", + "fe80::1%eth0 | fe80::1 ", + "'' | ", + "1 | ", + ": | ", + "1: | ", + "::: | ", + "1::2::3 | ", + "12345:: | ", + "1:2:3:4:5:6:7:8:9 | ", + "1:2:3:4:5:6:7 | ", + "1:2:3:4:5:6:7:8:: | ", + "1:2:3:4:5:6:7:8:9: | ", + "1:2:3:4:5:6:7:192.0.2.1 | ", + "::ffff:192.0.1 | ", + "::ffff:192.0.2.256 | ", + "2001:db8::g | ", + "fe80::1% | " + }) + void parsesIPv6Literals(String input, String expected) throws UnknownHostException { + assertArrayEquals(bytes(expected), IPAddressUtil.textToNumericFormatV6(input)); + assertEquals(expected != null, IPAddressUtil.isIPv6LiteralAddress(input)); + } + + @Test + void convertsOnlyTheCompleteIPv4MappedPrefix() { + byte[] mapped = new byte[16]; + mapped[10] = (byte) 0xff; + mapped[11] = (byte) 0xff; + mapped[12] = 1; + mapped[13] = 2; + mapped[14] = 3; + mapped[15] = 4; + assertArrayEquals(new byte[] {1, 2, 3, 4}, IPAddressUtil.convertFromIPv4MappedAddress(mapped)); + assertNull(IPAddressUtil.convertFromIPv4MappedAddress(new byte[4])); + for (int i = 0; i < 12; i++) { + byte[] invalid = mapped.clone(); + invalid[i] ^= 1; + assertNull(IPAddressUtil.convertFromIPv4MappedAddress(invalid), "prefix byte " + i); + } + } + + private static byte[] bytes(String address) throws UnknownHostException { + return address == null ? null : InetAddress.getByName(address).getAddress(); + } +} From 50a0a28bab5205862bf91dba816db12336f6ae53 Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Fri, 6 Nov 2026 11:31:08 +0100 Subject: [PATCH 6/7] Update SpotBugs exclusion for relocated IP parser --- gradle/spotbugFilters/exclude.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/spotbugFilters/exclude.xml b/gradle/spotbugFilters/exclude.xml index 22bde5b0ef0..f7a29090d8b 100644 --- a/gradle/spotbugFilters/exclude.xml +++ b/gradle/spotbugFilters/exclude.xml @@ -3,7 +3,7 @@ - + From c00dd95b1b694c458f0bc2ab5dd839d97f00543f Mon Sep 17 00:00:00 2001 From: Clara Poncet Date: Fri, 6 Nov 2026 19:24:39 +0100 Subject: [PATCH 7/7] Reuse Lambda request context and skip invalid IP exceptions --- .../trace/lambda/LambdaAppSecHandler.java | 25 +++++++------- .../trace/lambda/LambdaAppSecHandlerTest.java | 32 +++++++++++++++++ .../api/http/ClientIpAddressResolver.java | 3 ++ .../api/http/ClientIpAddressResolverTest.java | 34 +++++++++++-------- 4 files changed, 66 insertions(+), 28 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java index 171782ffda5..3c29b53b9eb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaAppSecHandler.java @@ -123,8 +123,7 @@ public static AgentSpanContext processRequestStart(Object event) { } LambdaURIDataAdapter uriAdapter = new LambdaURIDataAdapter(fullPath, eventData.headers, eventData.host); - String clientIp = resolveClientIp(eventData); - AgentSpanContext context = processAppSecRequestData(eventData, uriAdapter, clientIp); + AgentSpanContext context = processAppSecRequestData(eventData, uriAdapter); if (context instanceof TagContext) { applyHttpTags((TagContext) context, eventData, uriAdapter); } @@ -391,7 +390,7 @@ static void applyHttpTags(TagContext ctx, LambdaRequestData req, LambdaURIDataAd * requestStarted} callback */ private static AgentSpanContext processAppSecRequestData( - LambdaRequestData eventData, LambdaURIDataAdapter uriAdapter, String clientIp) { + LambdaRequestData eventData, LambdaURIDataAdapter uriAdapter) { AgentTracer.TracerAPI tracer = AgentTracer.get(); Supplier> requestStartedCallback = tracer.getCallbackProvider(RequestContextSlot.APPSEC).getCallback(EVENTS.requestStarted()); @@ -400,15 +399,12 @@ private static AgentSpanContext processAppSecRequestData( return null; } - TagContext tagContext = new TagContext(); - Object appSecRequestContext; + TagContext tagContext = createRequestTagContext(eventData); + String clientIp = tagContext.getTags().getString(Tags.HTTP_CLIENT_IP); // Call requestStarted - appSecRequestContext = requestStartedCallback.get().getResult(); + Object appSecRequestContext = requestStartedCallback.get().getResult(); tagContext.withRequestContextDataAppSec(appSecRequestContext); - if (clientIp != null) { - tagContext.putTag(Tags.HTTP_CLIENT_IP, clientIp); - } if (appSecRequestContext != null) { TemporaryRequestContext requestContext = new TemporaryRequestContext(appSecRequestContext); @@ -524,11 +520,11 @@ private static AgentSpanContext processAppSecRequestData( return tagContext; } - /** Resolves the client address using the same configuration and precedence as HTTP servers. */ - private static String resolveClientIp(LambdaRequestData eventData) { + /** Creates the request context, resolving the client address with HTTP server precedence. */ + private static TagContext createRequestTagContext(LambdaRequestData eventData) { Config config = Config.get(); if (!config.isClientIpEnabled() && !config.isTraceClientIpResolverEnabled()) { - return null; + return new TagContext(); } TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); headers.xForwardedFor = findHeader(eventData.headers, "x-forwarded-for"); @@ -556,7 +552,10 @@ private static String resolveClientIp(LambdaRequestData eventData) { TracePropagationStyle.NONE, DDTraceId.ZERO); InetAddress inferred = ClientIpAddressResolver.resolve(extracted, null, eventData.sourceIp); - return inferred == null ? null : inferred.getHostAddress(); + if (inferred != null) { + extracted.putTag(Tags.HTTP_CLIENT_IP, inferred.getHostAddress()); + } + return extracted; } /** Sets the current trigger type thread-local. Package-private for use in tests only. */ diff --git a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java index 59069ea9a0a..dce069e3813 100644 --- a/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/lambda/LambdaAppSecHandlerTest.java @@ -14,6 +14,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -3012,6 +3013,37 @@ void processRequestStartOmitsClientIpWhenNoAddressIsAvailable() { assertTrue(inferredAddresses.isEmpty()); } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void processRequestStartKeepsContextAndTagsSeparateAcrossInvocations(boolean resolverEnabled) { + injectSysConfig("trace.client-ip.resolver.enabled", Boolean.toString(resolverEnabled)); + injectSysConfig("trace.client-ip.enabled", "false"); + setupMockCallbacks(new Callbacks()); + + TagContext first = + (TagContext) + LambdaAppSecHandler.processRequestStart( + clientIpEvent("10.0.0.5", "x-forwarded-for", "1.2.3.4")); + TagContext second = + (TagContext) + LambdaAppSecHandler.processRequestStart( + clientIpEvent("10.0.0.6", "x-forwarded-for", "8.8.8.8")); + TagContext withoutAddress = + (TagContext) LambdaAppSecHandler.processRequestStart(clientIpEvent(null, null, null)); + + assertNotSame(first, second); + assertNotSame(second, withoutAddress); + assertNotSame(first.getTags(), second.getTags()); + assertEquals( + resolverEnabled ? "1.2.3.4" : null, first.getTags().getString(Tags.HTTP_CLIENT_IP)); + assertEquals( + resolverEnabled ? "8.8.8.8" : null, second.getTags().getString(Tags.HTTP_CLIENT_IP)); + assertNull(withoutAddress.getTags().getString(Tags.HTTP_CLIENT_IP)); + assertNotNull(first.getRequestContextDataAppSec()); + assertNotNull(second.getRequestContextDataAppSec()); + assertNotNull(withoutAddress.getRequestContextDataAppSec()); + } + private static ByteArrayInputStream clientIpEvent( String sourceIp, String headerName, String headerValue) { String headers = headerName == null ? "" : ",\"" + headerName + "\":\"" + headerValue + "\""; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java index 14ad40a72ff..2de55f974e5 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolver.java @@ -452,6 +452,9 @@ public static InetAddress parseIpAddress(String str) { if (addr == null) { addr = IPAddressUtil.textToNumericFormatV6(str); } + if (addr == null) { + return null; + } return InetAddress.getByAddress(str, addr); } catch (UnknownHostException e) { return null; // should not happen diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java index 613f37c940b..db93eedd305 100644 --- a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/http/ClientIpAddressResolverTest.java @@ -21,21 +21,25 @@ class ClientIpAddressResolverTest { @TableTest({ - "Scenario | Header | Peer | Expected ", - "no addresses | | | ", - "public peer fallback | | 1.2.3.4 | 1.2.3.4 ", - "private peer fallback | | 10.0.0.5 | 10.0.0.5 ", - "invalid peer | | invalid | ", - "invalid header fallback | invalid | 1.2.3.4 | 1.2.3.4 ", - "public header beats peer | 1.2.3.4 | 8.8.8.8 | 1.2.3.4 ", - "public header without peer | 1.2.3.4 | | 1.2.3.4 ", - "public peer beats private | 10.0.0.2 | 1.2.3.4 | 1.2.3.4 ", - "private header beats private | 10.0.0.2 | 10.0.0.5 | 10.0.0.2 ", - "private header without peer | 10.0.0.2 | | 10.0.0.2 ", - "invalid peer preserves header | 10.0.0.2 | invalid | 10.0.0.2 ", - "forwarded chain | '10.0.0.2, 1.2.3.4' | 8.8.8.8 | 1.2.3.4 ", - "public IPv6 header | 2001:4860:4860::8888 | 1.2.3.4 | 2001:4860:4860::8888", - "public peer beats loopback | ::1 | 1.2.3.4 | 1.2.3.4 " + "Scenario | Header | Peer | Expected ", + "no addresses | | | ", + "public peer fallback | | 1.2.3.4 | 1.2.3.4 ", + "private peer fallback | | 10.0.0.5 | 10.0.0.5 ", + "invalid peer | | invalid | ", + "invalid header fallback | invalid | 1.2.3.4 | 1.2.3.4 ", + "public header beats peer | 1.2.3.4 | 8.8.8.8 | 1.2.3.4 ", + "public header without peer | 1.2.3.4 | | 1.2.3.4 ", + "public peer beats private | 10.0.0.2 | 1.2.3.4 | 1.2.3.4 ", + "private header beats private | 10.0.0.2 | 10.0.0.5 | 10.0.0.2 ", + "private header without peer | 10.0.0.2 | | 10.0.0.2 ", + "invalid peer preserves header | 10.0.0.2 | invalid | 10.0.0.2 ", + "invalid numeric IPv4 | 999.999.999.999 | | ", + "invalid numeric IPv6 | 2001:db8:::1 | | ", + "skip invalid numeric chain | '999.999.999.999, 300.1.2.3, 1.2.3.4' | 8.8.8.8 | 1.2.3.4 ", + "invalid numeric peer | 10.0.0.2 | 999.999.999.999 | 10.0.0.2 ", + "forwarded chain | '10.0.0.2, 1.2.3.4' | 8.8.8.8 | 1.2.3.4 ", + "public IPv6 header | 2001:4860:4860::8888 | 1.2.3.4 | 2001:4860:4860::8888", + "public peer beats loopback | ::1 | 1.2.3.4 | 1.2.3.4 " }) void resolvesHeadersAndPeer(String header, String peer, String expected) throws UnknownHostException {