diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java index 958241ac4b..c3e70161a7 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java @@ -122,19 +122,27 @@ public abstract class BaseDecorator { * @return */ public String spanNameForMethod(final Method method) { - final Class declaringClass = method.getDeclaringClass(); - String className; - if (declaringClass.isAnonymousClass()) { - className = declaringClass.getName(); - if (declaringClass.getPackage() != null) { - final String pkgName = declaringClass.getPackage().getName(); - if (!pkgName.isEmpty()) { - className = declaringClass.getName().replace(pkgName, "").substring(1); - } - } - } else { - className = declaringClass.getSimpleName(); + return spanNameForClass(method.getDeclaringClass()) + "." + method.getName(); + } + + /** + * This method is used to generate an acceptable span (operation) name based on a given class + * reference. Anonymous classes are named based on their parent. + * + * @param clazz + * @return + */ + public String spanNameForClass(final Class clazz) { + if (!clazz.isAnonymousClass()) { + return clazz.getSimpleName(); } - return className + "." + method.getName(); + String className = clazz.getName(); + if (clazz.getPackage() != null) { + final String pkgName = clazz.getPackage().getName(); + if (!pkgName.isEmpty()) { + className = clazz.getName().replace(pkgName, "").substring(1); + } + } + return className; } } diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java index 74536e62b3..a099e82a37 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.springweb; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE; +import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE_RENDER; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; @@ -11,10 +12,8 @@ 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; import java.util.HashMap; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -35,6 +34,17 @@ public final class DispatcherServletInstrumentation extends Instrumenter.Default return named("org.springframework.web.servlet.DispatcherServlet"); } + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".SpringWebHttpServerDecorator", + packageName + ".SpringWebHttpServerDecorator$1", + }; + } + @Override public Map, String> transformers() { final Map, String> transformers = new HashMap<>(); @@ -57,29 +67,17 @@ public final class DispatcherServletInstrumentation extends Instrumenter.Default @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); + final Scope scope = GlobalTracer.get().buildSpan("response.render").startActive(true); + DECORATE_RENDER.afterStart(scope); + DECORATE_RENDER.onRender(scope, mv); + return scope; } @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)); - } + DECORATE_RENDER.onError(scope, throwable); + DECORATE_RENDER.beforeFinish(scope); scope.close(); } } @@ -90,7 +88,7 @@ public final class DispatcherServletInstrumentation extends Instrumenter.Default final Scope scope = GlobalTracer.get().scopeManager().active(); if (scope != null && exception != null) { final Span span = scope.span(); - span.log(Collections.singletonMap(ERROR_OBJECT, exception)); + DECORATE.onError(span, 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/HandlerAdapterInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java index edefe55c9b..b9881a993e 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.springweb; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -14,11 +14,7 @@ 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.Map; @@ -30,7 +26,6 @@ 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) @@ -46,6 +41,17 @@ public final class HandlerAdapterInstrumentation extends Instrumenter.Default { .and(safeHasSuperType(named("org.springframework.web.servlet.HandlerAdapter"))); } + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".SpringWebHttpServerDecorator", + packageName + ".SpringWebHttpServerDecorator$1", + }; + } + @Override public Map, String> transformers() { return singletonMap( @@ -66,15 +72,8 @@ public final class HandlerAdapterInstrumentation extends Instrumenter.Default { // 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; - parentScope.span().setTag(DDTags.RESOURCE_NAME, resourceName); - parentScope.span().setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER); - } + if (parentScope != null) { + DECORATE.onRequest(parentScope.span(), request); } // Now create a span for controller execution. @@ -105,33 +104,18 @@ public final class HandlerAdapterInstrumentation extends Instrumenter.Default { 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 = DECORATE.spanNameForClass(clazz) + "." + methodName; - final String operationName = className + "." + methodName; - - return GlobalTracer.get() - .buildSpan(operationName) - .withTag(Tags.COMPONENT.getKey(), "spring-web-controller") - .startActive(true); + final Scope scope = GlobalTracer.get().buildSpan(operationName).startActive(true); + DECORATE.afterStart(scope); + return scope; } @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(singletonMap(ERROR_OBJECT, throwable)); - } + DECORATE.onError(scope, throwable); + DECORATE.beforeFinish(scope); scope.close(); } } diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java new file mode 100644 index 0000000000..8c8e9dd38c --- /dev/null +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java @@ -0,0 +1,87 @@ +package datadog.trace.instrumentation.springweb; + +import datadog.trace.agent.decorator.HttpServerDecorator; +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import io.opentracing.Scope; +import io.opentracing.Span; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import lombok.extern.slf4j.Slf4j; +import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.ModelAndView; + +@Slf4j +public class SpringWebHttpServerDecorator + extends HttpServerDecorator { + public static final SpringWebHttpServerDecorator DECORATE = new SpringWebHttpServerDecorator(); + public static final SpringWebHttpServerDecorator DECORATE_RENDER = + new SpringWebHttpServerDecorator() { + @Override + protected String component() { + return "spring-webmvc"; + } + }; + + @Override + protected String[] instrumentationNames() { + return new String[] {"spring-web"}; + } + + @Override + protected String component() { + return "spring-web-controller"; + } + + @Override + protected String method(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getMethod(); + } + + @Override + protected String url(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getRequestURL().toString(); + } + + @Override + protected String hostname(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getServerName(); + } + + @Override + protected Integer port(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getServerPort(); + } + + @Override + protected Integer status(final HttpServletResponse httpServletResponse) { + return httpServletResponse.getStatus(); + } + + @Override + public Span onRequest(final Span span, final HttpServletRequest request) { + super.onRequest(span, request); + if (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; + span.setTag(DDTags.RESOURCE_NAME, resourceName); + span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER); + } + } + return span; + } + + public Scope onRender(final Scope scope, final ModelAndView mv) { + final Span span = scope.span(); + if (mv.getViewName() != null) { + span.setTag("view.name", mv.getViewName()); + } + if (mv.getView() != null) { + span.setTag("view.type", mv.getView().getClass().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 ba4c30c1bb..9836cea8a5 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 @@ -368,10 +368,12 @@ class SpringBootBasedTest extends AgentTestRunner { serviceName "unnamed-java-app" operationName name resourceName name + spanType DDSpanTypes.HTTP_SERVER childOf(trace.span(0)) errored errorType != null tags { "$Tags.COMPONENT.key" "spring-web-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER if (errorType) { "error.msg" String errorTags(errorType) diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/AbstractWebfluxInstrumentation.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/AbstractWebfluxInstrumentation.java index ae61ea6bea..58e39d7d39 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/AbstractWebfluxInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/AbstractWebfluxInstrumentation.java @@ -4,8 +4,6 @@ import datadog.trace.agent.tooling.Instrumenter; public abstract class AbstractWebfluxInstrumentation extends Instrumenter.Default { - public static final String PACKAGE = AbstractWebfluxInstrumentation.class.getPackage().getName(); - public AbstractWebfluxInstrumentation(final String... additionalNames) { super("spring-webflux", additionalNames); } @@ -13,11 +11,14 @@ public abstract class AbstractWebfluxInstrumentation extends Instrumenter.Defaul @Override public String[] helperClassNames() { return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + packageName + ".SpringWebfluxHttpServerDecorator", // Some code comes from reactor's instrumentation's helper "datadog.trace.instrumentation.reactor.core.ReactorCoreAdviceUtils", "datadog.trace.instrumentation.reactor.core.ReactorCoreAdviceUtils$TracingSubscriber", - PACKAGE + ".AdviceUtils", - PACKAGE + ".RouteOnSuccessOrError" + packageName + ".AdviceUtils", + packageName + ".RouteOnSuccessOrError" }; } } diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/DispatcherHandlerInstrumentation.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/DispatcherHandlerInstrumentation.java index a8c65dee66..c6a0c3e436 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/DispatcherHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/DispatcherHandlerInstrumentation.java @@ -31,6 +31,6 @@ public final class DispatcherHandlerInstrumentation extends AbstractWebfluxInstr .and(takesArgument(0, named("org.springframework.web.server.ServerWebExchange"))) .and(takesArguments(1)), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".DispatcherHandlerAdvice"); + packageName + ".DispatcherHandlerAdvice"); } } diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/HandlerAdapterInstrumentation.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/HandlerAdapterInstrumentation.java index 97512d53a9..520d396cda 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/HandlerAdapterInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/HandlerAdapterInstrumentation.java @@ -38,6 +38,6 @@ public final class HandlerAdapterInstrumentation extends AbstractWebfluxInstrume .and(takesArgument(1, named("java.lang.Object"))) .and(takesArguments(2)), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".HandlerAdapterAdvice"); + packageName + ".HandlerAdapterAdvice"); } } diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/RouterFunctionInstrumentation.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/RouterFunctionInstrumentation.java index c15b25f4d7..133b56ca81 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/RouterFunctionInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java/datadog/trace/instrumentation/springwebflux/RouterFunctionInstrumentation.java @@ -45,6 +45,6 @@ public final class RouterFunctionInstrumentation extends AbstractWebfluxInstrume 0, named("org.springframework.web.reactive.function.server.ServerRequest"))) .and(takesArguments(1)), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".RouterFunctionAdvice"); + packageName + ".RouterFunctionAdvice"); } } diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/AdviceUtils.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/AdviceUtils.java index ac34726898..108bb223d8 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/AdviceUtils.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/AdviceUtils.java @@ -1,5 +1,7 @@ package datadog.trace.instrumentation.springwebflux; +import static datadog.trace.instrumentation.springwebflux.SpringWebfluxHttpServerDecorator.DECORATE; + import datadog.trace.instrumentation.reactor.core.ReactorCoreAdviceUtils; import io.opentracing.Span; import lombok.extern.slf4j.Slf4j; @@ -11,10 +13,10 @@ public class AdviceUtils { public static final String SPAN_ATTRIBUTE = "datadog.trace.instrumentation.springwebflux.Span"; public static final String PARENT_SPAN_ATTRIBUTE = - "datadog.trace.instrumentation.springwebflux.ParentSpan"; + "datadog.trace.instrumentation.springwebflux.ParentSpan"; public static String parseOperationName(final Object handler) { - final String className = parseClassName(handler.getClass()); + final String className = DECORATE.spanNameForClass(handler.getClass()); final String operationName; final int lambdaIdx = className.indexOf("$$Lambda$"); @@ -26,29 +28,15 @@ public class AdviceUtils { return operationName; } - public static String parseClassName(final Class clazz) { - 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); - } - } - } - return className; + public static void finishSpanIfPresent( + final ServerWebExchange exchange, final Throwable throwable) { + ReactorCoreAdviceUtils.finishSpanIfPresent( + (Span) exchange.getAttributes().remove(SPAN_ATTRIBUTE), throwable); } public static void finishSpanIfPresent( - final ServerWebExchange exchange, final Throwable throwable) { + final ServerRequest serverRequest, final Throwable throwable) { ReactorCoreAdviceUtils.finishSpanIfPresent( - (Span) exchange.getAttributes().remove(SPAN_ATTRIBUTE), throwable); - } - - public static void finishSpanIfPresent( - final ServerRequest serverRequest, final Throwable throwable) { - ReactorCoreAdviceUtils.finishSpanIfPresent( - (Span) serverRequest.attributes().remove(SPAN_ATTRIBUTE), throwable); + (Span) serverRequest.attributes().remove(SPAN_ATTRIBUTE), throwable); } } diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/DispatcherHandlerAdvice.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/DispatcherHandlerAdvice.java index 0a28bc8ef9..f7f72e9e3a 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/DispatcherHandlerAdvice.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/DispatcherHandlerAdvice.java @@ -1,12 +1,11 @@ package datadog.trace.instrumentation.springwebflux; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; +import static datadog.trace.instrumentation.springwebflux.SpringWebfluxHttpServerDecorator.DECORATE; + import datadog.trace.context.TraceScope; import datadog.trace.instrumentation.reactor.core.ReactorCoreAdviceUtils; import io.opentracing.Scope; import io.opentracing.Span; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.util.function.Function; import net.bytebuddy.asm.Advice; @@ -29,12 +28,8 @@ public class DispatcherHandlerAdvice { if (parentSpan != null) { exchange.getAttributes().put(AdviceUtils.PARENT_SPAN_ATTRIBUTE, parentSpan); } - final Scope scope = - GlobalTracer.get() - .buildSpan("DispatcherHandler.handle") - .withTag(Tags.COMPONENT.getKey(), "spring-webflux-controller") - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER) - .startActive(false); + final Scope scope = GlobalTracer.get().buildSpan("DispatcherHandler.handle").startActive(false); + DECORATE.afterStart(scope); ((TraceScope) scope).setAsyncPropagation(true); exchange.getAttributes().put(AdviceUtils.SPAN_ATTRIBUTE, scope.span()); return scope; @@ -42,13 +37,13 @@ public class DispatcherHandlerAdvice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Enter final Scope scope, - @Advice.Thrown final Throwable throwable, - @Advice.Argument(0) final ServerWebExchange exchange, - @Advice.Return(readOnly = false) Mono mono) { + @Advice.Enter final Scope scope, + @Advice.Thrown final Throwable throwable, + @Advice.Argument(0) final ServerWebExchange exchange, + @Advice.Return(readOnly = false) Mono mono) { if (throwable == null && mono != null) { final Function, ? extends Publisher> function = - ReactorCoreAdviceUtils.finishSpanNextOrError(); + ReactorCoreAdviceUtils.finishSpanNextOrError(); mono = ReactorCoreAdviceUtils.setPublisherSpan(mono, scope.span()); } else if (throwable != null) { AdviceUtils.finishSpanIfPresent(exchange, throwable); diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/HandlerAdapterAdvice.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/HandlerAdapterAdvice.java index 5547c5053d..df050bb9e7 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/HandlerAdapterAdvice.java +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/HandlerAdapterAdvice.java @@ -1,5 +1,7 @@ package datadog.trace.instrumentation.springwebflux; +import static datadog.trace.instrumentation.springwebflux.SpringWebfluxHttpServerDecorator.DECORATE; + import datadog.trace.api.DDTags; import datadog.trace.context.TraceScope; import io.opentracing.Scope; @@ -15,8 +17,8 @@ public class HandlerAdapterAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope methodEnter( - @Advice.Argument(0) final ServerWebExchange exchange, - @Advice.Argument(1) final Object handler) { + @Advice.Argument(0) final ServerWebExchange exchange, + @Advice.Argument(1) final Object handler) { Scope scope = null; final Span span = exchange.getAttribute(AdviceUtils.SPAN_ATTRIBUTE); @@ -27,11 +29,8 @@ public class HandlerAdapterAdvice { if (handler instanceof HandlerMethod) { // Special case for requests mapped with annotations final HandlerMethod handlerMethod = (HandlerMethod) handler; - final Class handlerClass = handlerMethod.getMethod().getDeclaringClass(); - - operationName = - AdviceUtils.parseClassName(handlerClass) + "." + handlerMethod.getMethod().getName(); - handlerType = handlerClass.getName(); + operationName = DECORATE.spanNameForMethod(handlerMethod.getMethod()); + handlerType = handlerMethod.getMethod().getDeclaringClass().getName(); } else { operationName = AdviceUtils.parseOperationName(handler); handlerType = handler.getClass().getName(); @@ -46,11 +45,11 @@ public class HandlerAdapterAdvice { final Span parentSpan = exchange.getAttribute(AdviceUtils.PARENT_SPAN_ATTRIBUTE); final PathPattern bestPattern = - exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); if (parentSpan != null && bestPattern != null) { parentSpan.setTag( - DDTags.RESOURCE_NAME, - exchange.getRequest().getMethodValue() + " " + bestPattern.getPatternString()); + DDTags.RESOURCE_NAME, + exchange.getRequest().getMethodValue() + " " + bestPattern.getPatternString()); } return scope; @@ -58,9 +57,9 @@ public class HandlerAdapterAdvice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Argument(0) final ServerWebExchange exchange, - @Advice.Enter final Scope scope, - @Advice.Thrown final Throwable throwable) { + @Advice.Argument(0) final ServerWebExchange exchange, + @Advice.Enter final Scope scope, + @Advice.Thrown final Throwable throwable) { if (throwable != null) { AdviceUtils.finishSpanIfPresent(exchange, throwable); } diff --git a/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/SpringWebfluxHttpServerDecorator.java b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/SpringWebfluxHttpServerDecorator.java new file mode 100644 index 0000000000..1616366e5c --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webflux/src/main/java8/datadog/trace/instrumentation/springwebflux/SpringWebfluxHttpServerDecorator.java @@ -0,0 +1,26 @@ +package datadog.trace.instrumentation.springwebflux; + +import datadog.trace.agent.decorator.ServerDecorator; +import datadog.trace.api.DDSpanTypes; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class SpringWebfluxHttpServerDecorator extends ServerDecorator { + public static final SpringWebfluxHttpServerDecorator DECORATE = + new SpringWebfluxHttpServerDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"spring-webflux"}; + } + + @Override + protected String spanType() { + return DDSpanTypes.HTTP_SERVER; + } + + @Override + protected String component() { + return "spring-webflux-controller"; + } +} diff --git a/dd-java-agent/instrumentation/spring-webflux/src/test/groovy/SpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring-webflux/src/test/groovy/SpringWebfluxTest.groovy index 1d7f718e58..ae6ffbe0dc 100644 --- a/dd-java-agent/instrumentation/spring-webflux/src/test/groovy/SpringWebfluxTest.groovy +++ b/dd-java-agent/instrumentation/spring-webflux/src/test/groovy/SpringWebfluxTest.groovy @@ -62,6 +62,7 @@ class SpringWebfluxTest extends AgentTestRunner { childOf(span(1)) tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER if (annotatedMethod == null) { // Functional API "request.predicate" "(GET && $urlPathWithVariables)" @@ -135,6 +136,7 @@ class SpringWebfluxTest extends AgentTestRunner { childOf(span(1)) tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER if (annotatedMethod == null) { // Functional API "request.predicate" "(GET && $urlPathWithVariables)" @@ -234,6 +236,7 @@ class SpringWebfluxTest extends AgentTestRunner { errored true tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "handler.type" "org.springframework.web.reactive.resource.ResourceWebHandler" errorTags(ResponseStatusException, String) defaultTags() @@ -265,6 +268,7 @@ class SpringWebfluxTest extends AgentTestRunner { childOf(span(1)) tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "request.predicate" "(POST && /echo)" "handler.type" { String tagVal -> return tagVal.contains(EchoHandlerFunction.getName()) @@ -348,6 +352,7 @@ class SpringWebfluxTest extends AgentTestRunner { errored true tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER if (annotatedMethod == null) { // Functional API "request.predicate" "(GET && $urlPathWithVariables)" @@ -412,6 +417,7 @@ class SpringWebfluxTest extends AgentTestRunner { childOf(span(0)) tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "request.predicate" "(GET && /double-greet-redirect)" "handler.type" { String tagVal -> return (tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) @@ -429,6 +435,7 @@ class SpringWebfluxTest extends AgentTestRunner { childOf(span(1)) tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "request.predicate" "(GET && /double-greet)" "handler.type" { String tagVal -> return tagVal.contains(INNER_HANDLER_FUNCTION_CLASS_TAG_PREFIX) @@ -485,6 +492,7 @@ class SpringWebfluxTest extends AgentTestRunner { childOf(span(1)) tags { "$Tags.COMPONENT.key" "spring-webflux-controller" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER if (annotatedMethod == null) { // Functional API "request.predicate" "(GET && $urlPathWithVariables)"