From fff3118decb13d2412985d417a3e1a89aeab0915 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 7 Aug 2018 16:32:37 +1000 Subject: [PATCH] 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); - } - } - } }