Skip to content

Commit 9250f83

Browse files
cataphractcataphract
committed
Support response substitution on tomcat
1 parent 4a73a97 commit 9250f83

File tree

27 files changed

+700
-90
lines changed

27 files changed

+700
-90
lines changed

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

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,21 @@ public AgentSpan onResponseStatus(final AgentSpan span, final int status) {
311311
return span;
312312
}
313313

314+
/**
315+
* Whether AppSec should NOT be called during onResponse() for analysis of the status code and
316+
* headers.
317+
*
318+
* <p>{@link #onResponse(AgentSpan, Object)} is usually called too late for AppSec to be able to
319+
* alter the response, so for those modules where we support blocking on response this is <code>
320+
* true</code> and AppSec has its own (earlier) hook point for processing the response (just
321+
* before commit).
322+
*
323+
* @return whether AppSec analysis of the response is run separately from onResponse
324+
*/
325+
protected boolean isAppSecOnResponseSeparate() {
326+
return false;
327+
}
328+
314329
public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
315330
if (response != null) {
316331
final int status = status(response);
@@ -325,7 +340,9 @@ public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
325340
}
326341
}
327342

328-
callIGCallbackResponseAndHeaders(span, response, status);
343+
if (!isAppSecOnResponseSeparate()) {
344+
callIGCallbackResponseAndHeaders(span, response, status);
345+
}
329346
}
330347
return span;
331348
}
@@ -402,30 +419,40 @@ private Flow<Void> callIGCallbackRequestHeaders(AgentSpan span, REQUEST_CARRIER
402419
return Flow.ResultFlow.empty();
403420
}
404421

405-
private void callIGCallbackResponseAndHeaders(AgentSpan span, RESPONSE carrier, int status) {
422+
private Flow<Void> callIGCallbackResponseAndHeaders(
423+
AgentSpan span, RESPONSE carrier, int status) {
424+
return callIGCallbackResponseAndHeaders(span, carrier, status, responseGetter());
425+
}
426+
427+
public <RESP> Flow<Void> callIGCallbackResponseAndHeaders(
428+
AgentSpan span,
429+
RESP carrier,
430+
int status,
431+
AgentPropagation.ContextVisitor<RESP> contextVisitor) {
406432
CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
407433
RequestContext requestContext = span.getRequestContext();
408434
if (cbp == null || requestContext == null) {
409-
return;
435+
return Flow.ResultFlow.empty();
410436
}
437+
411438
BiFunction<RequestContext, Integer, Flow<Void>> addrCallback =
412439
cbp.getCallback(EVENTS.responseStarted());
413440
if (null != addrCallback) {
414441
addrCallback.apply(requestContext, status);
415442
}
416-
AgentPropagation.ContextVisitor<RESPONSE> getter = responseGetter();
417-
if (getter == null) {
418-
return;
443+
if (contextVisitor == null) {
444+
return Flow.ResultFlow.empty();
419445
}
420446
IGKeyClassifier igKeyClassifier =
421447
IGKeyClassifier.create(
422448
requestContext,
423449
cbp.getCallback(EVENTS.responseHeader()),
424450
cbp.getCallback(EVENTS.responseHeaderDone()));
425451
if (null != igKeyClassifier) {
426-
getter.forEachKey(carrier, igKeyClassifier);
427-
igKeyClassifier.done();
452+
contextVisitor.forEachKey(carrier, igKeyClassifier);
453+
return igKeyClassifier.done();
428454
}
455+
return Flow.ResultFlow.empty();
429456
}
430457

431458
private Flow<Void> callIGCallbackURI(
@@ -499,9 +526,9 @@ private Flow<Void> callIGCallbackAddressAndPort(
499526
}
500527

501528
/** This passes the headers through to the InstrumentationGateway */
502-
private static final class IGKeyClassifier implements AgentPropagation.KeyClassifier {
529+
protected static final class IGKeyClassifier implements AgentPropagation.KeyClassifier {
503530

504-
private static IGKeyClassifier create(
531+
public static IGKeyClassifier create(
505532
RequestContext requestContext,
506533
TriConsumer<RequestContext, String, String> headerCallback,
507534
Function<RequestContext, Flow<Void>> doneCallback) {

‎dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/TomcatServlet3Test.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
3636
@Shared
3737
TestAccessLogValve accessLogValue
3838

39+
@Override
40+
boolean testBlockingOnResponse() {
41+
true
42+
}
43+
3944
class TomcatServer implements HttpServer {
4045
def port = 0
4146
final Tomcat server

‎dd-java-agent/instrumentation/spring-webmvc-3.1/build.gradle‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ dependencies {
5858
testImplementation project(':dd-java-agent:instrumentation:servlet-common')
5959
testImplementation project(':dd-java-agent:instrumentation:servlet:request-3')
6060

61-
// include also tomcat appsec instrumentation for intercepting urlencoded/multipart bodies
61+
// include also tomcat instrumentation
62+
testRuntimeOnly project(':dd-java-agent:instrumentation:tomcat-5.5')
6263
testRuntimeOnly project(':dd-java-agent:instrumentation:tomcat-appsec-6')
6364
testRuntimeOnly project(':dd-java-agent:instrumentation:tomcat-appsec-7')
6465

‎dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import datadog.trace.agent.test.asserts.TraceAssert
44
import datadog.trace.agent.test.base.HttpServer
55
import datadog.trace.agent.test.base.HttpServerTest
66
import datadog.trace.api.DDSpanTypes
7+
import datadog.trace.api.DDTags
78
import datadog.trace.api.iast.InstrumentationBridge
89
import datadog.trace.api.iast.source.WebModule
910
import datadog.trace.bootstrap.instrumentation.api.Tags
1011
import datadog.trace.core.DDSpan
11-
import datadog.trace.instrumentation.servlet3.Servlet3Decorator
1212
import datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator
1313
import okhttp3.FormBody
1414
import okhttp3.Request
@@ -64,7 +64,7 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
6464

6565
@Override
6666
String component() {
67-
return Servlet3Decorator.DECORATE.component()
67+
'tomcat-server'
6868
}
6969

7070
String getServletContext() {
@@ -154,9 +154,12 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
154154
int spanCount(ServerEndpoint endpoint) {
155155
if (endpoint == REDIRECT) {
156156
// Spring is generates a RenderView and ResponseSpan for REDIRECT
157-
return super.spanCount(endpoint) + 1
157+
super.spanCount(endpoint) + 1
158+
} else if (endpoint == NOT_FOUND) {
159+
super.spanCount(endpoint) + 2
160+
} else {
161+
super.spanCount(endpoint)
158162
}
159-
return super.spanCount(endpoint)
160163
}
161164

162165
@Override
@@ -398,4 +401,37 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
398401
}
399402
}
400403
}
404+
405+
406+
protected void trailingSpans(TraceAssert traceAssert, ServerEndpoint serverEndpoint) {
407+
if (serverEndpoint == NOT_FOUND) {
408+
traceAssert.with {
409+
span {
410+
spanType 'web'
411+
serviceName expectedServiceName()
412+
operationName 'servlet.forward'
413+
resourceName 'GET /error'
414+
tags {
415+
"$Tags.COMPONENT" 'java-web-servlet-dispatcher'
416+
"$Tags.HTTP_ROUTE" '/error'
417+
'servlet.context' "/$servletContext"
418+
'servlet.path' '/not-found'
419+
"$DDTags.PATHWAY_HASH" String
420+
defaultTags()
421+
}
422+
}
423+
span {
424+
spanType 'web'
425+
serviceName expectedServiceName()
426+
operationName 'spring.handler'
427+
resourceName 'BasicErrorController.error'
428+
tags {
429+
"$Tags.COMPONENT" 'spring-web-controller'
430+
"$Tags.SPAN_KIND" 'server'
431+
defaultTags()
432+
}
433+
}
434+
}
435+
}
436+
}
401437
}

‎dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/dynamic/DynamicRoutingTest.groovy‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
package test.dynamic
22

3-
43
import datadog.trace.agent.test.asserts.TraceAssert
54
import datadog.trace.agent.test.base.HttpServer
65
import datadog.trace.agent.test.base.HttpServerTest
76
import datadog.trace.api.DDSpanTypes
7+
import datadog.trace.api.DDTags
88
import datadog.trace.bootstrap.instrumentation.api.Tags
9-
import datadog.trace.instrumentation.servlet3.Servlet3Decorator
109
import datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator
1110
import org.springframework.boot.SpringApplication
1211
import org.springframework.boot.context.embedded.EmbeddedWebApplicationContext
1312
import org.springframework.context.ConfigurableApplicationContext
1413
import org.springframework.web.servlet.view.RedirectView
14+
import org.springframework.web.util.NestedServletException
1515
import test.boot.SecurityConfig
1616

1717
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
@@ -61,9 +61,14 @@ class DynamicRoutingTest extends HttpServerTest<ConfigurableApplicationContext>
6161
return new SpringBootServer()
6262
}
6363

64+
@Override
65+
String expectedServiceName() {
66+
'root-servlet'
67+
}
68+
6469
@Override
6570
String component() {
66-
return Servlet3Decorator.DECORATE.component()
71+
'tomcat-server'
6772
}
6873

6974
@Override
@@ -110,15 +115,18 @@ class DynamicRoutingTest extends HttpServerTest<ConfigurableApplicationContext>
110115

111116
@Override
112117
Map<String, Serializable> expectedExtraServerTags(ServerEndpoint endpoint) {
113-
["servlet.path": endpoint.path]
118+
["servlet.path": endpoint.path, 'servlet.context': '/']
114119
}
115120

116121
int spanCount(ServerEndpoint endpoint) {
117122
if (endpoint == REDIRECT) {
118123
// Spring is generates a RenderView and ResponseSpan for REDIRECT
119-
return super.spanCount(endpoint) + 1
124+
super.spanCount(endpoint) + 1
125+
} else if (endpoint == EXCEPTION) {
126+
super.spanCount(endpoint) +2
127+
} else {
128+
super.spanCount(endpoint)
120129
}
121-
return super.spanCount(endpoint)
122130
}
123131

124132
@Override
@@ -187,4 +195,47 @@ class DynamicRoutingTest extends HttpServerTest<ConfigurableApplicationContext>
187195
int firstUnderscore = x.indexOf('_')
188196
return firstUnderscore == -1 ? x : x.substring(0, firstUnderscore)
189197
}
198+
199+
Map<String, Serializable> expectedExtraErrorInformation(ServerEndpoint endpoint) {
200+
if (endpoint == EXCEPTION) {
201+
["error.message" : 'Request processing failed; nested exception is java.lang.Exception: controller exception',
202+
"error.type" : NestedServletException.name,
203+
"error.stack": String]
204+
} else {
205+
super.expectedExtraErrorInformation(endpoint)
206+
}
207+
}
208+
209+
protected void trailingSpans(TraceAssert traceAssert, ServerEndpoint serverEndpoint) {
210+
if (serverEndpoint == EXCEPTION) {
211+
traceAssert.with {
212+
span {
213+
spanType 'web'
214+
serviceName expectedServiceName()
215+
operationName 'servlet.forward'
216+
resourceName 'GET /error'
217+
tags {
218+
"$Tags.COMPONENT" 'java-web-servlet-dispatcher'
219+
"$Tags.HTTP_ROUTE" '/error'
220+
'servlet.context' '/'
221+
'servlet.path' serverEndpoint.path
222+
"$DDTags.PATHWAY_HASH" String
223+
defaultTags()
224+
}
225+
}
226+
span {
227+
spanType 'web'
228+
childOfPrevious()
229+
serviceName expectedServiceName()
230+
operationName 'spring.handler'
231+
resourceName 'BasicErrorController.error'
232+
tags {
233+
"$Tags.COMPONENT" 'spring-web-controller'
234+
"$Tags.SPAN_KIND" 'server'
235+
defaultTags()
236+
}
237+
}
238+
}
239+
}
240+
}
190241
}

0 commit comments

Comments
 (0)