Skip to content

Commit 21b2fc1

Browse files
smolasmola
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 75fe2b3 commit 21b2fc1

29 files changed

Lines changed: 85 additions & 26 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 & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,10 @@ class GatewayBridgeSpecification extends DDSpecification {
189189
then:
190190
1 * spanInfo.getTags() >> TagMap.fromMap(['http.client_ip': '1.1.1.1'])
191191
1 * mockAppSecCtx.transferCollectedEvents() >> [event]
192-
1 * mockAppSecCtx.peerAddress >> '2001::1'
193192
1 * mockAppSecCtx.close()
194193
1 * spanInfo.setMetric("_dd.appsec.enabled", 1)
195194
1 * spanInfo.setTag("_dd.runtime_family", "jvm")
196195
1 * spanInfo.setTag('appsec.event', true)
197-
1 * spanInfo.setTag('network.client.ip', '2001::1')
198196
1 * spanInfo.setTag('actor.ip', '1.1.1.1')
199197
1 * traceSegment.setDataTop('appsec', new AppSecEventWrapper([event]))
200198
1 * mockAppSecCtx.isWafBlocked()

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,22 +2490,15 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
24902490
if (hasPeerPort) {
24912491
"$Tags.PEER_PORT" Integer
24922492
}
2493-
if (span.getTag(Tags.PEER_HOST_IPV6) != null) {
2494-
"$Tags.PEER_HOST_IPV6" {
2495-
it == "0:0:0:0:0:0:0:1" || (endpoint == FORWARDED && it == endpoint.body)
2496-
}
2497-
"$Tags.HTTP_CLIENT_IP" {
2498-
it == "0:0:0:0:0:0:0:1" || (endpoint == FORWARDED && it == endpoint.body)
2499-
}
2500-
} else {
2501-
"$Tags.PEER_HOST_IPV4" {
2502-
it == "127.0.0.1" || (endpoint == FORWARDED && it == endpoint.body)
2503-
}
2504-
"$Tags.HTTP_CLIENT_IP" {
2505-
it == "127.0.0.1" || (endpoint == FORWARDED && it == endpoint.body)
2506-
}
2507-
}
2493+
def peerHostTag = span.getTag(Tags.PEER_HOST_IPV6) != null ?
2494+
Tags.PEER_HOST_IPV6 : Tags.PEER_HOST_IPV4
2495+
def expectedPeerIp = peerHostTag == Tags.PEER_HOST_IPV6 ? "0:0:0:0:0:0:0:1" : "127.0.0.1"
2496+
"$peerHostTag" expectedPeerIp
2497+
"$Tags.HTTP_CLIENT_IP" clientIp ?: expectedPeerIp
2498+
"$Tags.NETWORK_CLIENT_IP" expectedPeerIp
25082499
} else {
2500+
// http.client_ip is inferred from forwarded headers; network.client.ip requires peerIp.
2501+
"$Tags.NETWORK_CLIENT_IP" null
25092502
"$Tags.HTTP_CLIENT_IP" clientIp
25102503
}
25112504
"$Tags.HTTP_HOSTNAME" address.host

‎dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class LagomTest extends InstrumentationSpecification {
7474
"$Tags.PEER_HOST_IPV4" '127.0.0.1'
7575
"$Tags.PEER_PORT" Integer
7676
"$Tags.HTTP_CLIENT_IP" '127.0.0.1'
77+
"$Tags.NETWORK_CLIENT_IP" '127.0.0.1'
7778
defaultTags()
7879
}
7980
}
@@ -121,6 +122,7 @@ class LagomTest extends InstrumentationSpecification {
121122
"$Tags.PEER_HOST_IPV4" '127.0.0.1'
122123
"$Tags.PEER_PORT" Integer
123124
"$Tags.HTTP_CLIENT_IP" '127.0.0.1'
125+
"$Tags.NETWORK_CLIENT_IP" '127.0.0.1'
124126
defaultTags()
125127
}
126128
}

‎dd-java-agent/instrumentation/cxf-2.1/src/latestDepTest/groovy/CxfContextPropagationTest.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class CxfContextPropagationTest extends InstrumentationSpecification {
7575
"$InstrumentationTags.SERVLET_PATH" "/test"
7676
"$Tags.HTTP_USER_AGENT" String
7777
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
78+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
7879
withCustomIntegrationName("jetty-server")
7980
defaultTags()
8081
}

‎dd-java-agent/instrumentation/cxf-2.1/src/test/groovy/CxfContextPropagationTest.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class CxfContextPropagationTest extends InstrumentationSpecification {
7373
"servlet.path" { it == null || it == "/test" }
7474
"$Tags.HTTP_USER_AGENT" String
7575
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
76+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
7677
withCustomIntegrationName("jetty-server")
7778
defaultTags()
7879
}

‎dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
3737
"$Tags.HTTP_STATUS" 200
3838
"$Tags.HTTP_USER_AGENT" String
3939
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
40+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
4041
"servlet.context" "/$jspWebappContext"
4142
"servlet.path" "/$jspFileName"
4243
defaultTags()
@@ -114,6 +115,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
114115
"$Tags.HTTP_STATUS" 200
115116
"$Tags.HTTP_USER_AGENT" String
116117
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
118+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
117119
"servlet.context" "/$jspWebappContext"
118120
"servlet.path" "/getQuery.jsp"
119121
defaultTags()
@@ -187,6 +189,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
187189
"$Tags.HTTP_STATUS" 200
188190
"$Tags.HTTP_USER_AGENT" String
189191
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
192+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
190193
"servlet.context" "/$jspWebappContext"
191194
"servlet.path" "/post.jsp"
192195
defaultTags()
@@ -256,6 +259,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
256259
"$Tags.HTTP_STATUS" 500
257260
"$Tags.HTTP_USER_AGENT" String
258261
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
262+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
259263
"servlet.context" "/$jspWebappContext"
260264
"servlet.path" "/$jspFileName"
261265
"error.type" { String tagExceptionType ->
@@ -345,6 +349,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
345349
"$Tags.HTTP_STATUS" 200
346350
"$Tags.HTTP_USER_AGENT" String
347351
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
352+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
348353
"servlet.context" "/$jspWebappContext"
349354
"servlet.path" "/includes/includeHtml.jsp"
350355
defaultTags()
@@ -414,6 +419,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
414419
"$Tags.HTTP_STATUS" 200
415420
"$Tags.HTTP_USER_AGENT" String
416421
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
422+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
417423
"servlet.context" "/$jspWebappContext"
418424
"servlet.path" "/includes/includeMulti.jsp"
419425
defaultTags()
@@ -537,6 +543,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
537543
"$Tags.HTTP_STATUS" 500
538544
"$Tags.HTTP_USER_AGENT" String
539545
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
546+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
540547
"servlet.context" "/$jspWebappContext"
541548
"servlet.path" "/$jspFileName"
542549
errorTags(JasperException, String)
@@ -602,6 +609,7 @@ class JSPInstrumentationBasicTests extends JSPTestBase {
602609
"$Tags.HTTP_STATUS" 200
603610
"$Tags.HTTP_USER_AGENT" String
604611
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
612+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
605613
"servlet.context" "/$jspWebappContext"
606614
"servlet.path" "/$staticFile"
607615
defaultTags()

‎dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class JSPInstrumentationForwardTests extends JSPTestBase {
3434
"$Tags.HTTP_STATUS" 200
3535
"$Tags.HTTP_USER_AGENT" String
3636
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
37+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
3738
"servlet.context" "/$jspWebappContext"
3839
"servlet.path" "/$forwardFromFileName"
3940
defaultTags()
@@ -136,6 +137,7 @@ class JSPInstrumentationForwardTests extends JSPTestBase {
136137
"$Tags.HTTP_STATUS" 200
137138
"$Tags.HTTP_USER_AGENT" String
138139
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
140+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
139141
"servlet.context" "/$jspWebappContext"
140142
"servlet.path" "/forwards/forwardToHtml.jsp"
141143
defaultTags()
@@ -205,6 +207,7 @@ class JSPInstrumentationForwardTests extends JSPTestBase {
205207
"$Tags.HTTP_STATUS" 200
206208
"$Tags.HTTP_USER_AGENT" String
207209
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
210+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
208211
"servlet.context" "/$jspWebappContext"
209212
"servlet.path" "/forwards/forwardToIncludeMulti.jsp"
210213
defaultTags()
@@ -358,6 +361,7 @@ class JSPInstrumentationForwardTests extends JSPTestBase {
358361
"$Tags.HTTP_STATUS" 200
359362
"$Tags.HTTP_USER_AGENT" String
360363
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
364+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
361365
"servlet.context" "/$jspWebappContext"
362366
"servlet.path" "/forwards/forwardToJspForward.jsp"
363367
defaultTags()
@@ -483,6 +487,7 @@ class JSPInstrumentationForwardTests extends JSPTestBase {
483487
"$Tags.HTTP_STATUS" 500
484488
"$Tags.HTTP_USER_AGENT" String
485489
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
490+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
486491
"servlet.context" "/$jspWebappContext"
487492
"servlet.path" "/forwards/forwardToCompileError.jsp"
488493
errorTags(JasperException, String)
@@ -569,6 +574,7 @@ class JSPInstrumentationForwardTests extends JSPTestBase {
569574
"$Tags.HTTP_STATUS" 404
570575
"$Tags.HTTP_USER_AGENT" String
571576
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
577+
"$Tags.NETWORK_CLIENT_IP" "127.0.0.1"
572578
"servlet.context" "/$jspWebappContext"
573579
"servlet.path" "/forwards/forwardToNonExistent.jsp"
574580
defaultTags()

0 commit comments

Comments
 (0)