Ensure RequestDispatcher span is part of expected trace
MODIFY
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java
@@ -19,6 +19,8 @@ public interface AgentSpan {
AgentSpan getLocalRootSpan();
+ boolean isSameTrace(AgentSpan otherSpan);
+
Context context();
void finish();

MODIFY
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java
@@ -182,6 +182,12 @@ public class AgentTracer {
return this;
}
+ @Override
+ public boolean isSameTrace(final AgentSpan otherSpan) {
+ // Not sure if this is the best idea...
+ return otherSpan instanceof NoopAgentSpan;
+ }
+
@Override
public Context context() {
return NoopContext.INSTANCE;

MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java
@@ -198,6 +198,20 @@ public final class OpenTracing32 implements TracerAPI {
return this;
}
+ @Override
+ public boolean isSameTrace(final AgentSpan otherAgentSpan) {
+ if (otherAgentSpan instanceof OT32Span) {
+ final Span otherSpan = ((OT32Span) otherAgentSpan).span;
+ if (span instanceof DDSpan && otherSpan instanceof DDSpan) {
+ // minor optimization to avoid BigInteger.toString()
+ return ((DDSpan) span).getTraceId().equals(((DDSpan) otherSpan).getTraceId());
+ } else {
+ return span.context().toTraceId().equals(otherSpan.context().toTraceId());
+ }
+ }
+ return false;
+ }
+
@Override
public OT32Context context() {
final SpanContext context = span.context();

MODIFY
dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java
dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java
@@ -10,7 +10,9 @@ import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import lombok.extern.slf4j.Slf4j;
+@Slf4j
public class Servlet3Decorator
extends HttpServerDecorator<HttpServletRequest, HttpServletRequest, HttpServletResponse> {
public static final Servlet3Decorator DECORATE = new Servlet3Decorator();
@@ -78,11 +80,11 @@ public class Servlet3Decorator
final AgentSpan span, final HttpServletRequest request, final ServletContext context) {
final Object attribute = context.getAttribute("dd.dispatcher-filter");
if (attribute instanceof Filter) {
- final Filter filter = (Filter) attribute;
- try {
request.setAttribute(DD_SPAN_ATTRIBUTE, span);
- filter.doFilter(request, null, null);
+ try {
+ ((Filter) attribute).doFilter(request, null, null);
} catch (final IOException | ServletException e) {
+ log.debug("Exception unexpectedly thrown by filter", e);
}
}
}

MODIFY
dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java
dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java
@@ -79,12 +79,27 @@ public final class RequestDispatcherInstrumentation extends Instrumenter.Default
@Advice.This final RequestDispatcher dispatcher,
@Advice.Local("_requestSpan") Object requestSpan,
@Advice.Argument(0) final ServletRequest request) {
- if (activeSpan() == null) {
+ final AgentSpan parentSpan = activeSpan();
+
+ final Object servletSpanObject = request.getAttribute(DD_SPAN_ATTRIBUTE);
+ final AgentSpan servletSpan =
+ servletSpanObject instanceof AgentSpan ? (AgentSpan) servletSpanObject : null;
+
+ if (parentSpan == null && servletSpan == null) {
// Don't want to generate a new top-level span
return null;
}
+ final AgentSpan.Context parent;
+ if (servletSpan == null || (parentSpan != null && servletSpan.isSameTrace(parentSpan))) {
+ // Use the parentSpan if the servletSpan is null or part of the same trace.
+ parent = parentSpan.context();
+ } else {
+ // parentSpan is part of a different trace, so lets ignore it.
+ // This can happen with the way Tomcat does error handling.
+ parent = servletSpan.context();
+ }
- final AgentSpan span = startSpan("servlet." + method);
+ final AgentSpan span = startSpan("servlet." + method, parent);
DECORATE.afterStart(span);
final String target =

MODIFY
dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy
dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy
@@ -25,6 +25,7 @@ class RequestDispatcherTest extends AgentTestRunner {
dispatcher.include("")
then:
+ 2 * request.getAttribute(DD_SPAN_ATTRIBUTE)
assertTraces(2) {
trace(0, 1) {
basicSpan(it, 0, "forward-child")
@@ -45,6 +46,7 @@ class RequestDispatcherTest extends AgentTestRunner {
}
then:
+ 1 * request.getAttribute(DD_SPAN_ATTRIBUTE)
assertTraces(1) {
trace(0, 3) {
basicSpan(it, 0, "parent")
@@ -97,6 +99,7 @@ class RequestDispatcherTest extends AgentTestRunner {
def th = thrown(ServletException)
th == ex
+ 1 * request.getAttribute(DD_SPAN_ATTRIBUTE)
assertTraces(1) {
trace(0, 3) {
basicSpan(it, 0, "parent", null, ex)

MODIFY
dd-java-agent/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle
dd-java-agent/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle
@@ -48,6 +48,7 @@ dependencies {
}
// Include servlet instrumentation for verifying the tomcat requests
+ testCompile project(':dd-java-agent:instrumentation:servlet')
testCompile project(':dd-java-agent:instrumentation:servlet:request-3')
testCompile group: 'javax.validation', name: 'validation-api', version: '1.1.0.Final'

MODIFY
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy
@@ -25,7 +25,7 @@ class AppConfig extends WebMvcConfigurerAdapter {
.defaultContentTypeStrategy(new ContentNegotiationStrategy() {
@Override
List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException {
- return [MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON]
+ return [MediaType.TEXT_PLAIN]
}
})
}

MODIFY
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy
@@ -91,6 +91,21 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
}
it.remove(renderSpan)
}
+ def responseSpan = it.find {
+ it.operationName == "servlet.response"
+ }
+ if (responseSpan) {
+ SpanAssert.assertSpan(responseSpan) {
+ operationName "servlet.response"
+ resourceName { it == "HttpServletResponse.sendRedirect" || it == "HttpServletResponse.sendError" }
+ errored false
+ tags {
+ "$Tags.COMPONENT" "java-web-servlet-response"
+ defaultTags()
+ }
+ }
+ it.remove(responseSpan)
+ }
}
super.cleanAndAssertTraces(size, spec)

MODIFY
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy
package test.filter
+import com.google.common.base.Charsets
import datadog.trace.agent.test.base.HttpServerTest
import org.apache.catalina.connector.Connector
import org.springframework.boot.autoconfigure.SpringBootApplication
@@ -7,7 +8,14 @@ import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory
import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomizer
import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory
import org.springframework.context.annotation.Bean
+import org.springframework.http.HttpInputMessage
+import org.springframework.http.HttpOutputMessage
import org.springframework.http.MediaType
+import org.springframework.http.converter.AbstractHttpMessageConverter
+import org.springframework.http.converter.HttpMessageConverter
+import org.springframework.http.converter.HttpMessageNotReadableException
+import org.springframework.http.converter.HttpMessageNotWritableException
+import org.springframework.util.StreamUtils
import org.springframework.web.HttpMediaTypeNotAcceptableException
import org.springframework.web.accept.ContentNegotiationStrategy
import org.springframework.web.context.request.NativeWebRequest
@@ -42,7 +50,7 @@ class FilteredAppConfig extends WebMvcConfigurerAdapter {
.defaultContentTypeStrategy(new ContentNegotiationStrategy() {
@Override
List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException {
- return [MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON]
+ return [MediaType.TEXT_PLAIN]
}
})
}
@@ -62,6 +70,27 @@ class FilteredAppConfig extends WebMvcConfigurerAdapter {
return factory
}
+ @Bean
+ HttpMessageConverter<Map<String, Object>> createPlainMapMessageConverter() {
+ return new AbstractHttpMessageConverter<Map<String, Object>>(MediaType.TEXT_PLAIN) {
+
+ @Override
+ protected boolean supports(Class<?> clazz) {
+ return Map.isAssignableFrom(clazz)
+ }
+
+ @Override
+ protected Map<String, Object> readInternal(Class<? extends Map<String, Object>> clazz, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException {
+ return null
+ }
+
+ @Override
+ protected void writeInternal(Map<String, Object> stringObjectMap, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException {
+ StreamUtils.copy(stringObjectMap.get("message"), Charsets.UTF_8, outputMessage.getBody())
+ }
+ }
+ }
+
@Bean
Filter servletFilter() {
return new Filter() {

MODIFY
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy
@@ -96,6 +96,21 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> {
}
it.remove(renderSpan)
}
+ def responseSpan = it.find {
+ it.operationName == "servlet.response"
+ }
+ if (responseSpan) {
+ SpanAssert.assertSpan(responseSpan) {
+ operationName "servlet.response"
+ resourceName { it == "HttpServletResponse.sendRedirect" || it == "HttpServletResponse.sendError" }
+ errored false
+ tags {
+ "$Tags.COMPONENT" "java-web-servlet-response"
+ defaultTags()
+ }
+ }
+ it.remove(responseSpan)
+ }
}
super.cleanAndAssertTraces(size, spec)

MODIFY
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy
@@ -140,6 +140,7 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
final int status
final String body
final Boolean errored
+ final boolean hasPathParam
ServerEndpoint(String uri, int status, String body) {
def uriObj = URI.create(uri)
@@ -149,6 +150,7 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
this.status = status
this.body = body
this.errored = status >= 500
+ this.hasPathParam = body == "123"
}
String getPath() {
@@ -164,7 +166,7 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
}
String resource(String method, URI address, String pathParam) {
- return status == 404 ? "404" : "$method ${pathParam ? pathParam : resolve(address).path}"
+ return status == 404 ? "404" : "$method ${hasPathParam ? pathParam : resolve(address).path}"
}
private static final Map<String, ServerEndpoint> PATH_MAP = values().collectEntries { [it.path, it] }

Ensure RequestDispatcher span is part of expected trace