Skip to content

Commit 2a1428e

Browse files
committed
Move network.client.ip out of AppSec into HttpServerDecorator
network.client.ip now shares the same activation logic as http.client_ip (DD_APPSEC_ENABLED or DD_TRACE_CLIENT_IP_ENABLED) and is no longer exclusive to AppSec events. Move it from GatewayBridge (where it was set only when security events fired) into HttpServerDecorator alongside http.client_ip, using the raw peer/socket IP. actor.ip remains in AppSec as a deprecated backward-compatibility tag.
1 parent 34333bb commit 2a1428e

7 files changed

Lines changed: 17 additions & 4 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,9 @@ public AgentSpan onRequest(
370370
} else {
371371
span.setTag(Tags.PEER_HOST_IPV4, peerIp);
372372
}
373+
if (clientIpResolverEnabled) {
374+
span.setTag(Tags.NETWORK_CLIENT_IP, peerIp);
375+
}
373376
}
374377
setPeerPort(span, peerPort);
375378
Flow<Void> flow = callIGCallbackAddressAndPort(span, peerIp, peerPort, inferredAddressStr);

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
218218
1 * this.span.setTag(Tags.HTTP_FORWARDED_HOST, "somehost")
219219
if (conn?.peerIp) {
220220
1 * this.span.setTag(ipv4 ? Tags.PEER_HOST_IPV4 : Tags.PEER_HOST_IPV6, conn.peerIp)
221+
1 * this.span.setTag(Tags.NETWORK_CLIENT_IP, conn.peerIp)
221222
}
222223
if (conn?.ip) {
223224
1 * this.span.setTag(Tags.HTTP_CLIENT_IP, conn.ip)
@@ -253,6 +254,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
253254

254255
then:
255256
1 * this.span.setTag(Tags.HTTP_CLIENT_IP, result)
257+
1 * this.span.setTag(Tags.NETWORK_CLIENT_IP, peerIpAddr)
256258

257259
where:
258260
headerIpAddr | peerIpAddr | result
@@ -277,6 +279,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
277279
0 * extracted.getForwarded()
278280
0 * span.setTag(Tags.HTTP_FORWARDED, _)
279281
0 * this.span.setTag(Tags.HTTP_CLIENT_IP, _)
282+
0 * this.span.setTag(Tags.NETWORK_CLIENT_IP, _)
280283
}
281284

282285
void 'disabling appsec disables header collection and ip address resolution'() {
@@ -294,6 +297,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
294297
0 * extracted.getForwarded()
295298
0 * span.setTag(Tags.HTTP_FORWARDED, _)
296299
0 * this.span.setTag(Tags.HTTP_CLIENT_IP, _)
300+
0 * this.span.setTag(Tags.NETWORK_CLIENT_IP, _)
297301
}
298302

299303
void 'disabling appsec but enabling client_ip_without_appsec enables header collection and ip address resolution'() {
@@ -313,6 +317,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
313317
2 * extracted.getXForwardedFor() >> '2.3.4.5'
314318
1 * this.span.setTag(Tags.HTTP_CLIENT_IP, '2.3.4.5')
315319
1 * this.span.setTag(Tags.HTTP_FORWARDED_IP, '2.3.4.5')
320+
1 * this.span.setTag(Tags.NETWORK_CLIENT_IP, '4.4.4.4')
316321

317322
// Forwarded doesn't participate in client ip resolution anymore
318323
1 * extracted.getForwarded() >> 'for=9.9.9.9'
@@ -333,6 +338,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
333338
then:
334339
1 * extracted.getCustomIpHeader() >> '5.5.5.5'
335340
1 * this.span.setTag(Tags.HTTP_CLIENT_IP, '5.5.5.5')
341+
1 * this.span.setTag(Tags.NETWORK_CLIENT_IP, '4.4.4.4')
336342
}
337343

338344
def "test onResponse"() {

dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -986,9 +986,6 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
986986

987987
span.setTag("appsec.event", true);
988988

989-
String peerAddress = ctx.getPeerAddress();
990-
span.setTag("network.client.ip", peerAddress);
991-
992989
// Reflect client_ip as actor.ip for backward compatibility
993990
Object clientIp = tags.get(Tags.HTTP_CLIENT_IP);
994991
if (clientIp != null) {

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ class GatewayBridgeSpecification extends DDSpecification {
194194
1 * spanInfo.setMetric("_dd.appsec.enabled", 1)
195195
1 * spanInfo.setTag("_dd.runtime_family", "jvm")
196196
1 * spanInfo.setTag('appsec.event', true)
197-
1 * spanInfo.setTag('network.client.ip', '2001::1')
198197
1 * spanInfo.setTag('actor.ip', '1.1.1.1')
199198
1 * traceSegment.setDataTop('appsec', new AppSecEventWrapper([event]))
200199
1 * mockAppSecCtx.isWafBlocked()

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2497,13 +2497,15 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
24972497
"$Tags.HTTP_CLIENT_IP" {
24982498
it == "0:0:0:0:0:0:0:1" || (endpoint == FORWARDED && it == endpoint.body)
24992499
}
2500+
"$Tags.NETWORK_CLIENT_IP" "0:0:0:0:0:0:0:1"
25002501
} else {
25012502
"$Tags.PEER_HOST_IPV4" {
25022503
it == "127.0.0.1" || (endpoint == FORWARDED && it == endpoint.body)
25032504
}
25042505
"$Tags.HTTP_CLIENT_IP" {
25052506
it == "127.0.0.1" || (endpoint == FORWARDED && it == endpoint.body)
25062507
}
2508+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
25072509
}
25082510
} else {
25092511
"$Tags.HTTP_CLIENT_IP" clientIp

dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,11 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest {
486486
assert it['rule']['tags']['type'] == 'attack_tool'
487487
}
488488
rootSpans.each { assert it.meta['actor.ip'] == '1.2.3.4' }
489+
rootSpans.each {
490+
// network.client.ip is the raw peer (socket) address, not the header-spoofed value
491+
assert it.meta['network.client.ip'] != null
492+
assert it.meta['network.client.ip'] != '1.2.3.4'
493+
}
489494
rootSpans.each {
490495
assert it.meta['http.response.headers.content-type'] == 'text/plain;charset=UTF-8'
491496
assert it.meta['http.response.headers.content-length'] == '15'

internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public class Tags {
2727
public static final String HTTP_FORWARDED_PORT = "http.forwarded.port";
2828
public static final String HTTP_USER_AGENT = "http.useragent";
2929
public static final String HTTP_CLIENT_IP = "http.client_ip";
30+
public static final String NETWORK_CLIENT_IP = "network.client.ip";
3031
public static final String HTTP_REQUEST_CONTENT_LENGTH = "http.request_content_length";
3132
public static final String HTTP_RESPONSE_CONTENT_LENGTH = "http.response_content_length";
3233
public static final String PEER_HOST_IPV4 = "peer.ipv4";

0 commit comments

Comments
 (0)