From 05dc51633a613d5ec9efa48a67ee06cc7f21c29c Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 7 Aug 2018 16:04:28 +1000 Subject: [PATCH 1/5] Update Spring Tests --- .../groovy/test/SpringBootBasedTest.groovy | 343 +++++++++--------- 1 file changed, 170 insertions(+), 173 deletions(-) diff --git a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy index 97d82bae46..d6eee233c8 100644 --- a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy @@ -10,6 +10,8 @@ import org.springframework.boot.test.web.client.TestRestTemplate import org.springframework.web.bind.MethodArgumentNotValidException import org.springframework.web.util.NestedServletException +import static datadog.trace.agent.test.ListWriterAssert.assertTraces + @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) class SpringBootBasedTest extends AgentTestRunner { @@ -32,211 +34,206 @@ class SpringBootBasedTest extends AgentTestRunner { def "generates spans"() { expect: restTemplate.getForObject("http://localhost:$port/param/asdf1234/", String) == "Hello asdf1234" - TEST_WRITER.waitForTraces(1) - TEST_WRITER.size() == 1 - def trace = TEST_WRITER.firstTrace() - trace.size() == 1 - def span = trace[0] - - span.context().operationName == "servlet.request" - span.context().resourceName == "GET /param/{parameter}/" - span.context().spanType == DDSpanTypes.WEB_SERVLET - !span.context().getErrorFlag() - span.context().parentId == "0" - span.context().tags["http.url"] == "http://localhost:$port/param/asdf1234/" - span.context().tags["http.method"] == "GET" - span.context().tags["span.kind"] == "server" - span.context().tags["span.type"] == "web" - span.context().tags["component"] == "java-web-servlet" - span.context().tags["http.status_code"] == 200 - span.context().tags["thread.name"] != null - span.context().tags["thread.id"] != null - span.context().tags.size() == 8 + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /param/{parameter}/" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/param/asdf1234/" + "http.method" "GET" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 200 + defaultTags() + } + } + } + } } def "generates 404 spans"() { + setup: def response = restTemplate.getForObject("http://localhost:$port/invalid", Map) + expect: response.get("status") == 404 response.get("error") == "Not Found" - TEST_WRITER.waitForTraces(2) - TEST_WRITER.size() == 2 - and: // trace 0 - def trace0 = TEST_WRITER.get(0) - trace0.size() == 1 - def span0 = trace0[0] - - span0.context().operationName == "servlet.request" - span0.context().resourceName == "404" - span0.context().spanType == DDSpanTypes.WEB_SERVLET - !span0.context().getErrorFlag() - span0.context().parentId == "0" - span0.context().tags["http.url"] == "http://localhost:$port/invalid" - span0.context().tags["http.method"] == "GET" - span0.context().tags["span.kind"] == "server" - span0.context().tags["span.type"] == "web" - span0.context().tags["component"] == "java-web-servlet" - span0.context().tags["http.status_code"] == 404 - span0.context().tags["thread.name"] != null - span0.context().tags["thread.id"] != null - span0.context().tags.size() == 8 - - and: // trace 1 - def trace1 = TEST_WRITER.get(1) - trace1.size() == 1 - def span1 = trace1[0] - - span1.context().operationName == "servlet.request" - span1.context().resourceName == "404" - span1.context().spanType == DDSpanTypes.WEB_SERVLET - !span1.context().getErrorFlag() - span1.context().parentId == "0" - span1.context().tags["http.url"] == "http://localhost:$port/error" - span1.context().tags["http.method"] == "GET" - span1.context().tags["span.kind"] == "server" - span1.context().tags["span.type"] == "web" - span1.context().tags["component"] == "java-web-servlet" - span1.context().tags["http.status_code"] == 404 - span1.context().tags["thread.name"] != null - span1.context().tags["thread.id"] != null - span1.context().tags.size() == 8 + assertTraces(TEST_WRITER, 2) { + trace(0, 1) { + span(0) { + operationName "servlet.request" + resourceName "404" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/invalid" + "http.method" "GET" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 404 + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + operationName "servlet.request" + resourceName "404" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/error" + "http.method" "GET" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 404 + defaultTags() + } + } + } + } } def "generates error spans"() { - expect: + setup: def response = restTemplate.getForObject("http://localhost:$port/error/qwerty/", Map) + + expect: response.get("status") == 500 response.get("error") == "Internal Server Error" response.get("exception") == "java.lang.RuntimeException" response.get("message") == "qwerty" - TEST_WRITER.waitForTraces(2) - TEST_WRITER.size() == 2 - and: // trace 0 - def trace0 = TEST_WRITER.get(0) - trace0.size() == 1 - def span0 = trace0[0] - - span0.context().operationName == "servlet.request" - span0.context().resourceName == "GET /error/{parameter}/" - span0.context().spanType == DDSpanTypes.WEB_SERVLET - span0.context().getErrorFlag() - span0.context().parentId == "0" - span0.context().tags["http.url"] == "http://localhost:$port/error/qwerty/" - span0.context().tags["http.method"] == "GET" - span0.context().tags["span.kind"] == "server" - span0.context().tags["span.type"] == "web" - span0.context().tags["component"] == "java-web-servlet" - span0.context().tags["http.status_code"] == 500 - span0.context().tags["thread.name"] != null - span0.context().tags["thread.id"] != null - span0.context().tags["error"] == true - span0.context().tags["error.msg"] == "Request processing failed; nested exception is java.lang.RuntimeException: qwerty" - span0.context().tags["error.type"] == NestedServletException.getName() - span0.context().tags["error.stack"] != null - span0.context().tags.size() == 12 - - and: // trace 1 - def trace1 = TEST_WRITER.get(1) - trace1.size() == 1 - def span1 = trace1[0] - - span1.context().operationName == "servlet.request" - span1.context().resourceName == "GET /error" - span1.context().spanType == DDSpanTypes.WEB_SERVLET - span1.context().parentId == "0" - span1.context().tags["http.url"] == "http://localhost:$port/error" - span1.context().tags["http.method"] == "GET" - span1.context().tags["span.kind"] == "server" - span1.context().tags["span.type"] == "web" - span1.context().tags["component"] == "java-web-servlet" - span1.context().tags["http.status_code"] == 500 - span1.context().getErrorFlag() - span1.context().tags["thread.name"] != null - span1.context().tags["thread.id"] != null - span1.context().tags.size() == 9 + assertTraces(TEST_WRITER, 2) { + trace(0, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /error/{parameter}/" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored true + tags { + "http.url" "http://localhost:$port/error/qwerty/" + "http.method" "GET" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 500 + errorTags NestedServletException, "Request processing failed; nested exception is java.lang.RuntimeException: qwerty" + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /error" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored true + tags { + "http.url" "http://localhost:$port/error" + "http.method" "GET" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 500 + "error" true + defaultTags() + } + } + } + } } def "validated form"() { expect: restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bob", 20), String) == "Hello bob Person(Name: bob, Age: 20)" - TEST_WRITER.waitForTraces(1) - TEST_WRITER.size() == 1 - def trace = TEST_WRITER.firstTrace() - trace.size() == 1 - def span = trace[0] - - span.context().operationName == "servlet.request" - span.context().resourceName == "POST /validated" - span.context().spanType == DDSpanTypes.WEB_SERVLET - !span.context().getErrorFlag() - span.context().parentId == "0" - span.context().tags["http.url"] == "http://localhost:$port/validated" - span.context().tags["http.method"] == "POST" - span.context().tags["span.kind"] == "server" - span.context().tags["span.type"] == "web" - span.context().tags["component"] == "java-web-servlet" - span.context().tags["http.status_code"] == 200 - span.context().tags["thread.name"] != null - span.context().tags["thread.id"] != null - span.context().tags.size() == 8 + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + operationName "servlet.request" + resourceName "POST /validated" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/validated" + "http.method" "POST" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 200 + defaultTags() + } + } + } + } } def "invalid form"() { - expect: + setup: def response = restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bill", 5), Map, Map) + + expect: response.get("status") == 400 response.get("error") == "Bad Request" response.get("exception") == "org.springframework.web.bind.MethodArgumentNotValidException" response.get("message") == "Validation failed for object='testForm'. Error count: 1" - TEST_WRITER.waitForTraces(2) - TEST_WRITER.size() == 2 - and: // trace 0 - def trace0 = TEST_WRITER.get(0) - trace0.size() == 1 - def span0 = trace0[0] - - span0.context().operationName == "servlet.request" - span0.context().resourceName == "POST /validated" - span0.context().spanType == DDSpanTypes.WEB_SERVLET - !span0.context().getErrorFlag() // This should be an error once we have the http status code decorator working. - span0.context().parentId == "0" - span0.context().tags["http.url"] == "http://localhost:$port/validated" - span0.context().tags["http.method"] == "POST" - span0.context().tags["span.kind"] == "server" - span0.context().tags["span.type"] == "web" - span0.context().tags["component"] == "java-web-servlet" - span0.context().tags["http.status_code"] == 400 - span0.context().tags["thread.name"] != null - span0.context().tags["thread.id"] != null -// span0.context().tags["error"] == true // This should be an error once we have the http status code decorator working. - span0.context().tags["error.msg"].toString().startsWith("Validation failed") - span0.context().tags["error.type"] == MethodArgumentNotValidException.getName() - span0.context().tags["error.stack"] != null - span0.context().tags.size() == 12 - - and: // trace 1 - def trace1 = TEST_WRITER.get(1) - trace1.size() == 1 - def span1 = trace1[0] - - span1.context().operationName == "servlet.request" - span1.context().resourceName == "POST /error" - span1.context().spanType == DDSpanTypes.WEB_SERVLET - !span1.context().getErrorFlag() - span1.context().parentId == "0" - span1.context().tags["http.url"] == "http://localhost:$port/error" - span1.context().tags["http.method"] == "POST" - span1.context().tags["span.kind"] == "server" - span1.context().tags["span.type"] == "web" - span1.context().tags["component"] == "java-web-servlet" - span1.context().tags["http.status_code"] == 400 - span1.context().tags["thread.name"] != null - span1.context().tags["thread.id"] != null - span1.context().tags.size() == 8 + assertTraces(TEST_WRITER, 2) { + trace(0, 1) { + span(0) { + operationName "servlet.request" + resourceName "POST /validated" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/validated" + "http.method" "POST" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 400 + "error" false + "error.msg" String + "error.type" MethodArgumentNotValidException.name + "error.stack" String + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + operationName "servlet.request" + resourceName "POST /error" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/error" + "http.method" "POST" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 400 + defaultTags() + } + } + } + } } } From fff3118decb13d2412985d417a3e1a89aeab0915 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 7 Aug 2018 16:32:37 +1000 Subject: [PATCH 2/5] separate spring web and error instrumentation. --- .../SpringWebErrorInstrumentation.java | 62 +++++++++++++++++++ .../springweb/SpringWebInstrumentation.java | 49 +-------------- 2 files changed, 65 insertions(+), 46 deletions(-) create mode 100644 dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java new file mode 100644 index 0000000000..c4b096156f --- /dev/null +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java @@ -0,0 +1,62 @@ +package datadog.trace.instrumentation.springweb; + +import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isProtected; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class SpringWebErrorInstrumentation extends Instrumenter.Default { + + public SpringWebErrorInstrumentation() { + super("spring-web"); + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(named("org.springframework.web.servlet.DispatcherServlet")); + } + + @Override + public Map transformers() { + final Map transformers = new HashMap<>(); + transformers.put( + isMethod() + .and(isProtected()) + .and(nameStartsWith("processHandlerException")) + .and(takesArgument(3, Exception.class)), + SpringWebErrorHandlerAdvice.class.getName()); + return transformers; + } + + public static class SpringWebErrorHandlerAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void nameResource(@Advice.Argument(3) final Exception exception) { + final Scope scope = GlobalTracer.get().scopeManager().active(); + if (scope != null && exception != null) { + final Span span = scope.span(); + span.log(Collections.singletonMap(ERROR_OBJECT, exception)); + // We want to capture the stacktrace, but that doesn't mean it should be an error. + // We rely on a decorator to set the error state based on response code. (5xx -> error) + Tags.ERROR.set(span, false); + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java index d0ad15571f..688016051a 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java @@ -2,10 +2,8 @@ package datadog.trace.instrumentation.springweb; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithField; -import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -17,14 +15,12 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import javax.servlet.http.HttpServletRequest; import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.springframework.web.servlet.HandlerMapping; @@ -36,13 +32,13 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { } @Override - public ElementMatcher typeMatcher() { + public ElementMatcher typeMatcher() { return not(isInterface()) .and(safeHasSuperType(named("org.springframework.web.servlet.HandlerAdapter"))); } @Override - public ElementMatcher classLoaderMatcher() { + public ElementMatcher classLoaderMatcher() { return classLoaderHasClassWithField( "org.springframework.web.servlet.HandlerMapping", "BEST_MATCHING_PATTERN_ATTRIBUTE"); } @@ -59,31 +55,6 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { return transformers; } - @AutoService(Instrumenter.class) - public static class DispatcherServletInstrumentation extends Default { - - public DispatcherServletInstrumentation() { - super("spring-web"); - } - - @Override - public ElementMatcher typeMatcher() { - return not(isInterface()).and(named("org.springframework.web.servlet.DispatcherServlet")); - } - - @Override - public Map transformers() { - final Map transformers = new HashMap<>(); - transformers.put( - isMethod() - .and(isProtected()) - .and(nameStartsWith("processHandlerException")) - .and(takesArgument(3, Exception.class)), - SpringWebErrorHandlerAdvice.class.getName()); - return transformers; - } - } - public static class SpringWebNamingAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) @@ -101,18 +72,4 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { } } } - - public static class SpringWebErrorHandlerAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void nameResource(@Advice.Argument(3) final Exception exception) { - final Scope scope = GlobalTracer.get().scopeManager().active(); - if (scope != null && exception != null) { - final Span span = scope.span(); - span.log(Collections.singletonMap(ERROR_OBJECT, exception)); - // We want to capture the stacktrace, but that doesn't mean it should be an error. - // We rely on a decorator to set the error state based on response code. (5xx -> error) - Tags.ERROR.set(span, false); - } - } - } } From d05e2cfe8630da41c58448d255ddb9ee64eae4db Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 7 Aug 2018 17:03:07 +1000 Subject: [PATCH 3/5] Add span for spring web controller. --- .../springweb/SpringWebInstrumentation.java | 88 +++++++++++++++++-- .../groovy/test/SpringBootBasedTest.groovy | 48 ++++++++-- 2 files changed, 119 insertions(+), 17 deletions(-) diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java index 688016051a..f14edf8148 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java @@ -2,6 +2,7 @@ package datadog.trace.instrumentation.springweb; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithField; +import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -9,20 +10,29 @@ import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; +import java.lang.reflect.Method; +import java.util.Collections; import java.util.HashMap; import java.util.Map; +import javax.servlet.Servlet; import javax.servlet.http.HttpServletRequest; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import org.springframework.web.HttpRequestHandler; +import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.mvc.Controller; @AutoService(Instrumenter.class) public final class SpringWebInstrumentation extends Instrumenter.Default { @@ -50,26 +60,88 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { isMethod() .and(isPublic()) .and(nameStartsWith("handle")) - .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest"))), - SpringWebNamingAdvice.class.getName()); + .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest"))) + .and(takesArguments(3)), + ControllerAdvice.class.getName()); return transformers; } - public static class SpringWebNamingAdvice { + public static class ControllerAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void nameResource(@Advice.Argument(0) final HttpServletRequest request) { - final Scope scope = GlobalTracer.get().scopeManager().active(); - if (scope != null && request != null) { + public static Scope nameResourceAndStartSpan( + @Advice.Argument(0) final HttpServletRequest request, + @Advice.Argument(2) final Object handler) { + // Name the parent span based on the matching pattern + // This is likely the servlet.request span. + final Scope parentScope = GlobalTracer.get().scopeManager().active(); + if (parentScope != null && request != null) { final String method = request.getMethod(); final Object bestMatchingPattern = request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); if (method != null && bestMatchingPattern != null) { final String resourceName = method + " " + bestMatchingPattern; - scope.span().setTag(DDTags.RESOURCE_NAME, resourceName); - scope.span().setTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET); + parentScope.span().setTag(DDTags.RESOURCE_NAME, resourceName); + parentScope.span().setTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET); } } + + // Now create a span for controller execution. + + final Class clazz; + final String methodName; + + if (handler instanceof HandlerMethod) { + // name span based on the class and method name defined in the handler + final Method method = ((HandlerMethod) handler).getMethod(); + clazz = method.getDeclaringClass(); + methodName = method.getName(); + } else if (handler instanceof HttpRequestHandler) { + // org.springframework.web.servlet.mvc.HttpRequestHandlerAdapter + clazz = handler.getClass(); + methodName = "handleRequest"; + } else if (handler instanceof Controller) { + // org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter + clazz = handler.getClass(); + methodName = "handleRequest"; + } else if (handler instanceof Servlet) { + // org.springframework.web.servlet.handler.SimpleServletHandlerAdapter + clazz = handler.getClass(); + methodName = "service"; + } else { + // perhaps org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter + clazz = handler.getClass(); + methodName = ""; + } + + String className = clazz.getSimpleName(); + if (className.isEmpty()) { + className = clazz.getName(); + if (clazz.getPackage() != null) { + final String pkgName = clazz.getPackage().getName(); + if (!pkgName.isEmpty()) { + className = clazz.getName().replace(pkgName, "").substring(1); + } + } + } + + final String operationName = className + "." + methodName; + + return GlobalTracer.get() + .buildSpan(operationName) + .withTag(Tags.COMPONENT.getKey(), "spring-web-controller") + .startActive(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { + if (throwable != null) { + final Span span = scope.span(); + Tags.ERROR.set(span, true); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + } + scope.close(); } } } diff --git a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy index d6eee233c8..ef3efc9242 100644 --- a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy @@ -1,7 +1,10 @@ package test import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TraceAssert import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import io.opentracing.tag.Tags import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.context.embedded.LocalServerPort import org.springframework.boot.test.context.SpringBootTest @@ -36,7 +39,7 @@ class SpringBootBasedTest extends AgentTestRunner { restTemplate.getForObject("http://localhost:$port/param/asdf1234/", String) == "Hello asdf1234" assertTraces(TEST_WRITER, 1) { - trace(0, 1) { + trace(0, 2) { span(0) { operationName "servlet.request" resourceName "GET /param/{parameter}/" @@ -53,6 +56,7 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "TestController.withParam") } } } @@ -66,7 +70,7 @@ class SpringBootBasedTest extends AgentTestRunner { response.get("error") == "Not Found" assertTraces(TEST_WRITER, 2) { - trace(0, 1) { + trace(0, 2) { span(0) { operationName "servlet.request" resourceName "404" @@ -83,8 +87,9 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "ResourceHttpRequestHandler.handleRequest") } - trace(1, 1) { + trace(1, 2) { span(0) { operationName "servlet.request" resourceName "404" @@ -101,6 +106,7 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "BasicErrorController.error") } } } @@ -108,7 +114,7 @@ class SpringBootBasedTest extends AgentTestRunner { def "generates error spans"() { setup: def response = restTemplate.getForObject("http://localhost:$port/error/qwerty/", Map) - + expect: response.get("status") == 500 response.get("error") == "Internal Server Error" @@ -116,7 +122,7 @@ class SpringBootBasedTest extends AgentTestRunner { response.get("message") == "qwerty" assertTraces(TEST_WRITER, 2) { - trace(0, 1) { + trace(0, 2) { span(0) { operationName "servlet.request" resourceName "GET /error/{parameter}/" @@ -134,8 +140,9 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "TestController.withError", RuntimeException) } - trace(1, 1) { + trace(1, 2) { span(0) { operationName "servlet.request" resourceName "GET /error" @@ -153,6 +160,7 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "BasicErrorController.error") } } } @@ -162,7 +170,7 @@ class SpringBootBasedTest extends AgentTestRunner { restTemplate.postForObject("http://localhost:$port/validated", new TestForm("bob", 20), String) == "Hello bob Person(Name: bob, Age: 20)" assertTraces(TEST_WRITER, 1) { - trace(0, 1) { + trace(0, 2) { span(0) { operationName "servlet.request" resourceName "POST /validated" @@ -179,6 +187,7 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "TestController.withValidation") } } } @@ -194,7 +203,7 @@ class SpringBootBasedTest extends AgentTestRunner { response.get("message") == "Validation failed for object='testForm'. Error count: 1" assertTraces(TEST_WRITER, 2) { - trace(0, 1) { + trace(0, 2) { span(0) { operationName "servlet.request" resourceName "POST /validated" @@ -215,8 +224,9 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "TestController.withValidation", MethodArgumentNotValidException) } - trace(1, 1) { + trace(1, 2) { span(0) { operationName "servlet.request" resourceName "POST /error" @@ -233,6 +243,26 @@ class SpringBootBasedTest extends AgentTestRunner { defaultTags() } } + controllerSpan(it, 1, "BasicErrorController.error") + } + } + } + + def controllerSpan(TraceAssert trace, int index, String name, Class errorType = null) { + trace.span(index) { + serviceName "unnamed-java-app" + operationName name + resourceName name + childOf(trace.span(0)) + errored errorType != null + tags { + "$DDTags.SPAN_TYPE" DDSpanTypes.WEB_SERVLET + "$Tags.COMPONENT.key" "spring-web-controller" + if (errorType) { + "error.msg" String + errorTags(errorType) + } + defaultTags() } } } From ae9d4619a416d3ebb8eceef13b345c8483f6d89c Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 9 Aug 2018 13:10:45 +1000 Subject: [PATCH 4/5] Add instrumentation showing the rendering time --- ... => DispatcherServletInstrumentation.java} | 51 ++++++++++++++++--- ...ava => HandlerAdapterInstrumentation.java} | 9 ++-- 2 files changed, 47 insertions(+), 13 deletions(-) rename dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/{SpringWebErrorInstrumentation.java => DispatcherServletInstrumentation.java} (54%) rename dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/{SpringWebInstrumentation.java => HandlerAdapterInstrumentation.java} (95%) diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java similarity index 54% rename from dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java rename to dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java index c4b096156f..47a36459af 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebErrorInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java @@ -1,18 +1,17 @@ package datadog.trace.instrumentation.springweb; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import io.opentracing.Scope; import io.opentracing.Span; +import io.opentracing.Tracer; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.util.Collections; @@ -21,32 +20,70 @@ import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import org.springframework.web.servlet.ModelAndView; @AutoService(Instrumenter.class) -public final class SpringWebErrorInstrumentation extends Instrumenter.Default { +public final class DispatcherServletInstrumentation extends Instrumenter.Default { - public SpringWebErrorInstrumentation() { + public DispatcherServletInstrumentation() { super("spring-web"); } @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(named("org.springframework.web.servlet.DispatcherServlet")); + return named("org.springframework.web.servlet.DispatcherServlet"); } @Override public Map transformers() { final Map transformers = new HashMap<>(); + transformers.put( + isMethod() + .and(isProtected()) + .and(named("render")) + .and(takesArgument(0, named("org.springframework.web.servlet.ModelAndView"))), + DispatcherAdvice.class.getName()); transformers.put( isMethod() .and(isProtected()) .and(nameStartsWith("processHandlerException")) .and(takesArgument(3, Exception.class)), - SpringWebErrorHandlerAdvice.class.getName()); + ErrorHandlerAdvice.class.getName()); return transformers; } - public static class SpringWebErrorHandlerAdvice { + public static class DispatcherAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope startSpan(@Advice.Argument(0) final ModelAndView mv) { + + final Tracer.SpanBuilder builder = + GlobalTracer.get() + .buildSpan("response.render") + .withTag(Tags.COMPONENT.getKey(), "spring-webmvc"); + if (mv.getViewName() != null) { + builder.withTag("view.name", mv.getViewName()); + } + if (mv.getView() != null) { + builder.withTag("view.type", mv.getView().getClass().getName()); + } + return builder.startActive(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { + + if (throwable != null) { + final Span span = scope.span(); + Tags.ERROR.set(span, true); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + } + scope.close(); + } + } + + public static class ErrorHandlerAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void nameResource(@Advice.Argument(3) final Exception exception) { final Scope scope = GlobalTracer.get().scopeManager().active(); diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java similarity index 95% rename from dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java rename to dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java index f14edf8148..2ba50b9b9d 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java @@ -22,7 +22,6 @@ import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.lang.reflect.Method; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import javax.servlet.Servlet; import javax.servlet.http.HttpServletRequest; @@ -35,9 +34,9 @@ import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.Controller; @AutoService(Instrumenter.class) -public final class SpringWebInstrumentation extends Instrumenter.Default { +public final class HandlerAdapterInstrumentation extends Instrumenter.Default { - public SpringWebInstrumentation() { + public HandlerAdapterInstrumentation() { super("spring-web"); } @@ -55,15 +54,13 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { @Override public Map transformers() { - final Map transformers = new HashMap<>(); - transformers.put( + return Collections.singletonMap( isMethod() .and(isPublic()) .and(nameStartsWith("handle")) .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest"))) .and(takesArguments(3)), ControllerAdvice.class.getName()); - return transformers; } public static class ControllerAdvice { From a3875aff2ca740d34b0b9ab7e2567b2a53af17fe Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 9 Aug 2018 15:12:11 +1000 Subject: [PATCH 5/5] Logging improvements and other misc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit from reviewing a customer’s logs. --- .../trace/agent/tooling/AgentInstaller.java | 1 + .../servlet2/Servlet2Advice.java | 9 +++--- .../servlet3/Servlet3Advice.java | 9 +++--- .../groovy/test/SpringBootBasedTest.groovy | 23 ++++++++++++-- .../main/java/datadog/opentracing/DDSpan.java | 30 ++++++++++--------- .../datadog/opentracing/DDSpanContext.java | 2 +- .../scopemanager/ContinuableScope.java | 7 ++++- .../datadog/trace/DDSpanContextTest.groovy | 6 ++-- 8 files changed, 56 insertions(+), 31 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index fc5fbaebd8..90d4cd1653 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -68,6 +68,7 @@ public class AgentInstaller { .or(nameStartsWith("org.groovy.")) .or(nameStartsWith("com.p6spy.")) .or(nameStartsWith("org.slf4j.")) + .or(nameStartsWith("com.newrelic.")) .or(nameContains("javassist")) .or(nameContains(".asm.")) .or(nameMatches("com\\.mchange\\.v2\\.c3p0\\..*Proxy")); diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 5c7bf2da4b..56307cce8e 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -39,6 +39,9 @@ public class Servlet2Advice { GlobalTracer.get() .buildSpan("servlet.request") .asChildOf(extractedContext) + .withTag(Tags.COMPONENT.getKey(), "java-web-servlet") + .withTag(Tags.HTTP_METHOD.getKey(), httpServletRequest.getMethod()) + .withTag(Tags.HTTP_URL.getKey(), httpServletRequest.getRequestURL().toString()) .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) .withTag("servlet.context", httpServletRequest.getContextPath()) @@ -48,12 +51,8 @@ public class Servlet2Advice { ((TraceScope) scope).setAsyncPropagation(true); } - final Span span = scope.span(); - Tags.COMPONENT.set(span, "java-web-servlet"); - Tags.HTTP_METHOD.set(span, httpServletRequest.getMethod()); - Tags.HTTP_URL.set(span, httpServletRequest.getRequestURL().toString()); if (httpServletRequest.getUserPrincipal() != null) { - span.setTag("user.principal", httpServletRequest.getUserPrincipal().getName()); + scope.span().setTag("user.principal", httpServletRequest.getUserPrincipal().getName()); } return scope; } diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java index fc80a394e4..ffa134f58d 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java @@ -39,6 +39,9 @@ public class Servlet3Advice { GlobalTracer.get() .buildSpan("servlet.request") .asChildOf(extractedContext) + .withTag(Tags.COMPONENT.getKey(), "java-web-servlet") + .withTag(Tags.HTTP_METHOD.getKey(), httpServletRequest.getMethod()) + .withTag(Tags.HTTP_URL.getKey(), httpServletRequest.getRequestURL().toString()) .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) .withTag("servlet.context", httpServletRequest.getContextPath()) @@ -48,12 +51,8 @@ public class Servlet3Advice { ((TraceScope) scope).setAsyncPropagation(true); } - final Span span = scope.span(); - Tags.COMPONENT.set(span, "java-web-servlet"); - Tags.HTTP_METHOD.set(span, httpServletRequest.getMethod()); - Tags.HTTP_URL.set(span, httpServletRequest.getRequestURL().toString()); if (httpServletRequest.getUserPrincipal() != null) { - span.setTag("user.principal", httpServletRequest.getUserPrincipal().getName()); + scope.span().setTag("user.principal", httpServletRequest.getUserPrincipal().getName()); } return scope; } diff --git a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy index ef3efc9242..210fae4dbc 100644 --- a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy @@ -30,8 +30,27 @@ class SpringBootBasedTest extends AgentTestRunner { restTemplate.getForObject("http://localhost:$port/", String) == "Hello World" and: - TEST_WRITER.waitForTraces(1) - TEST_WRITER.size() == 1 + assertTraces(TEST_WRITER, 1) { + trace(0, 2) { + span(0) { + operationName "servlet.request" + resourceName "GET /" + spanType DDSpanTypes.WEB_SERVLET + parent() + errored false + tags { + "http.url" "http://localhost:$port/" + "http.method" "GET" + "span.kind" "server" + "span.type" "web" + "component" "java-web-servlet" + "http.status_code" 200 + defaultTags() + } + } + controllerSpan(it, 1, "TestController.greeting") + } + } } def "generates spans"() { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java index 68f66e9f87..34b08e4930 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java @@ -87,6 +87,7 @@ public class DDSpan implements Span, MutableSpan { private void finishAndAddToTrace(final long durationNano) { // ensure a min duration of 1 if (this.durationNano.compareAndSet(0, Math.max(1, durationNano))) { + log.debug("Finished: {}", this); context.getTrace().addSpan(this); } else { log.debug("{} - already finished!", this); @@ -149,7 +150,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan setTag(final String tag, final String value) { - this.context().setTag(tag, (Object) value); + context().setTag(tag, (Object) value); return this; } @@ -158,7 +159,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan setTag(final String tag, final boolean value) { - this.context().setTag(tag, (Object) value); + context().setTag(tag, (Object) value); return this; } @@ -167,7 +168,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan setTag(final String tag, final Number value) { - this.context().setTag(tag, (Object) value); + context().setTag(tag, (Object) value); return this; } @@ -176,7 +177,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpanContext context() { - return this.context; + return context; } /* (non-Javadoc) @@ -184,7 +185,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final String getBaggageItem(final String key) { - return this.context.getBaggageItem(key); + return context.getBaggageItem(key); } /* (non-Javadoc) @@ -192,7 +193,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan setBaggageItem(final String key, final String value) { - this.context.setBaggageItem(key, value); + context.setBaggageItem(key, value); return this; } @@ -201,7 +202,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan setOperationName(final String operationName) { - this.context().setOperationName(operationName); + context().setOperationName(operationName); return this; } @@ -247,13 +248,13 @@ public class DDSpan implements Span, MutableSpan { @Override public final DDSpan setServiceName(final String serviceName) { - this.context().setServiceName(serviceName); + context().setServiceName(serviceName); return this; } @Override public final DDSpan setResourceName(final String resourceName) { - this.context().setResourceName(resourceName); + context().setResourceName(resourceName); return this; } @@ -264,13 +265,13 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan setSamplingPriority(final int newPriority) { - this.context().setSamplingPriority(newPriority); + context().setSamplingPriority(newPriority); return this; } @Override public final DDSpan setSpanType(final String type) { - this.context().setSpanType(type); + context().setSpanType(type); return this; } @@ -371,7 +372,7 @@ public class DDSpan implements Span, MutableSpan { @Override @JsonIgnore public Map getTags() { - return this.context().getTags(); + return context().getTags(); } @JsonGetter @@ -405,12 +406,13 @@ public class DDSpan implements Span, MutableSpan { this(null); } - public UInt64IDStringSerializer(Class stringClass) { + public UInt64IDStringSerializer(final Class stringClass) { super(stringClass); } @Override - public void serialize(String value, JsonGenerator gen, SerializerProvider provider) + public void serialize( + final String value, final JsonGenerator gen, final SerializerProvider provider) throws IOException { gen.writeNumber(value); } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java index afca7c93db..93de472f1f 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java @@ -325,7 +325,7 @@ public class DDSpanContext implements io.opentracing.SpanContext { public String toString() { final StringBuilder s = new StringBuilder() - .append("Span [ t_id=") + .append("DDSpan [ t_id=") .append(traceId) .append(", s_id=") .append(spanId) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java index 7a44091f0d..d2f1b29403 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java @@ -48,7 +48,7 @@ public class ContinuableScope implements Scope, TraceScope { this.continuation = continuation; this.spanUnderScope = spanUnderScope; this.finishOnClose = finishOnClose; - this.toRestore = scopeManager.tlsScope.get(); + toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); } @@ -96,6 +96,11 @@ public class ContinuableScope implements Scope, TraceScope { } } + @Override + public String toString() { + return super.toString() + "->" + spanUnderScope; + } + public class Continuation implements Closeable, TraceScope.Continuation { public WeakReference ref; diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy index 29bbdf673a..ea116b762c 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy @@ -18,7 +18,7 @@ class DDSpanContextTest extends Specification { context.serviceName == "fakeService" context.resourceName == "fakeResource" context.spanType == "fakeType" - context.toString() == "Span [ t_id=1, s_id=1, p_id=0] trace=fakeService/fakeOperation/fakeResource metrics={} *errored* tags={${extra}span.type=${context.getSpanType()}, thread.id=${Thread.currentThread().id}, thread.name=${Thread.currentThread().name}}" + context.toString() == "DDSpan [ t_id=1, s_id=1, p_id=0] trace=fakeService/fakeOperation/fakeResource metrics={} *errored* tags={${extra}span.type=${context.getSpanType()}, thread.id=${Thread.currentThread().id}, thread.name=${Thread.currentThread().name}}" where: name | extra | tags @@ -35,7 +35,7 @@ class DDSpanContextTest extends Specification { def thread = Thread.currentThread() def expectedTags = [(DDTags.THREAD_NAME): thread.name, (DDTags.THREAD_ID): thread.id, (DDTags.SPAN_TYPE): context.getSpanType()] - def expectedTrace = "Span [ t_id=1, s_id=1, p_id=0] trace=$details metrics={} tags={span.type=${context.getSpanType()}, thread.id=$thread.id, thread.name=$thread.name}" + def expectedTrace = "DDSpan [ t_id=1, s_id=1, p_id=0] trace=$details metrics={} tags={span.type=${context.getSpanType()}, thread.id=$thread.id, thread.name=$thread.name}" expect: context.getTags() == expectedTags @@ -62,7 +62,7 @@ class DDSpanContextTest extends Specification { (DDTags.THREAD_NAME): thread.name, (DDTags.THREAD_ID) : thread.id ] - context.toString() == "Span [ t_id=1, s_id=1, p_id=0] trace=fakeService/fakeOperation/fakeResource metrics={} tags={span.type=${context.getSpanType()}, $name=$value, thread.id=$thread.id, thread.name=$thread.name}" + context.toString() == "DDSpan [ t_id=1, s_id=1, p_id=0] trace=fakeService/fakeOperation/fakeResource metrics={} tags={span.type=${context.getSpanType()}, $name=$value, thread.id=$thread.id, thread.name=$thread.name}" where: name | value