Update spring-webmvc to Instrumenter API (#3132)

* Update spring-webmvc to Instrumenter API

* Remove hasExceptionOnServerSpan from grails too
This commit is contained in:
Trask Stalnaker 2021-05-31 08:57:51 -07:00 committed by GitHub
parent d2906841bd
commit 5e8568807b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 208 additions and 178 deletions

View File

@ -86,11 +86,6 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
true
}
@Override
boolean hasExceptionOnServerSpan(ServerEndpoint endpoint) {
true
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND

View File

@ -5,10 +5,10 @@
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcTracer.tracer;
import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext;
import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcInstrumenters.modelAndViewInstrumenter;
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.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
@ -17,7 +17,6 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import java.util.List;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
@ -48,12 +47,6 @@ public class DispatcherServletInstrumentation implements TypeInstrumentation {
.and(named("render"))
.and(takesArgument(0, named("org.springframework.web.servlet.ModelAndView"))),
DispatcherServletInstrumentation.class.getName() + "$RenderAdvice");
transformer.applyAdviceToMethod(
isMethod()
.and(isProtected())
.and(nameStartsWith("processHandlerException"))
.and(takesArgument(3, Exception.class)),
DispatcherServletInstrumentation.class.getName() + "$ErrorHandlerAdvice");
}
/**
@ -84,32 +77,24 @@ public class DispatcherServletInstrumentation implements TypeInstrumentation {
@Advice.Argument(0) ModelAndView mv,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
context = tracer().startSpan(mv);
scope = context.makeCurrent();
Context parentContext = currentContext();
if (modelAndViewInstrumenter().shouldStart(parentContext, mv)) {
context = modelAndViewInstrumenter().start(parentContext, mv);
scope = context.makeCurrent();
}
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Argument(0) ModelAndView mv,
@Advice.Thrown Throwable throwable,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (scope == null) {
return;
}
scope.close();
if (throwable == null) {
tracer().end(context);
} else {
tracer().endExceptionally(context, throwable);
}
}
}
public static class ErrorHandlerAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void nameResource(@Advice.Argument(3) Exception exception) {
if (exception != null) {
// It is fine to set status=ERROR here, end(Context, Response) call will overwrite it if the
// exception turns out to be a valid result
tracer().onException(Java8BytecodeBridge.currentContext(), exception);
}
modelAndViewInstrumenter().end(context, mv, null, throwable);
}
}
}

View File

@ -7,7 +7,7 @@ package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.extension.matcher.ClassLoaderMatcher.hasClassesNamed;
import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcTracer.tracer;
import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcInstrumenters.handlerInstrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
@ -57,14 +57,19 @@ public class HandlerAdapterInstrumentation implements TypeInstrumentation {
@Advice.Argument(2) Object handler,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
// TODO (trask) should there be a way to customize Instrumenter.shouldStart()?
if (handler.getClass().getName().startsWith("org.grails.")) {
// skip creating handler span for grails, grails instrumentation will take care of it
return;
}
Context parentContext = Java8BytecodeBridge.currentContext();
Span serverSpan = ServerSpan.fromContextOrNull(parentContext);
// TODO (trask) is it important to check serverSpan != null here?
if (serverSpan != null) {
// Name the parent span based on the matching pattern
tracer().updateServerSpanName(parentContext, request);
ServerNameUpdater.updateServerSpanName(parentContext, request);
// Now create a span for handler/controller execution.
context = tracer().startHandlerSpan(parentContext, handler);
context = handlerInstrumenter().start(parentContext, handler);
if (context != null) {
scope = context.makeCurrent();
}
@ -73,6 +78,7 @@ public class HandlerAdapterInstrumentation implements TypeInstrumentation {
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Argument(2) Object handler,
@Advice.Thrown Throwable throwable,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
@ -80,11 +86,7 @@ public class HandlerAdapterInstrumentation implements TypeInstrumentation {
return;
}
scope.close();
if (throwable == null) {
tracer().end(context);
} else {
tracer().endExceptionally(context, throwable);
}
handlerInstrumenter().end(context, handler, null, throwable);
}
}
}

View File

@ -5,8 +5,6 @@
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcTracer.tracer;
import io.opentelemetry.context.Context;
import java.io.IOException;
import java.util.ArrayList;
@ -48,7 +46,7 @@ public class HandlerMappingResourceNameFilter implements Filter, Ordered {
// Name the parent span based on the matching pattern
// Let the parent span resource name be set with the attribute set in findMapping.
tracer().updateServerSpanName(context, (HttpServletRequest) request);
ServerNameUpdater.updateServerSpanName(context, (HttpServletRequest) request);
}
} catch (Exception ignored) {
// mapping.getHandler() threw exception. Ignore

View File

@ -0,0 +1,48 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.instrumentation.api.tracer.SpanNames.spanNameForMethod;
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import java.lang.reflect.Method;
import javax.servlet.Servlet;
import org.springframework.web.HttpRequestHandler;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.servlet.mvc.Controller;
public class HandlerSpanNameExtractor implements SpanNameExtractor<Object> {
@Override
public String extract(Object handler) {
Class<?> clazz;
String methodName;
if (handler instanceof HandlerMethod) {
// name span based on the class and method name defined in the handler
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 = "<annotation>";
}
return spanNameForMethod(clazz, methodName);
}
}

View File

@ -0,0 +1,38 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.instrumentation.api.tracer.SpanNames.spanNameForClass;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.View;
public class ModelAndViewAttributesExtractor extends AttributesExtractor<ModelAndView, Void> {
private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES =
Config.get()
.getBooleanProperty(
"otel.instrumentation.spring-webmvc.experimental-span-attributes", false);
@Override
protected void onStart(AttributesBuilder attributes, ModelAndView modelAndView) {
if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) {
attributes.put("spring-webmvc.view.name", modelAndView.getViewName());
View view = modelAndView.getView();
if (view != null) {
attributes.put("spring-webmvc.view.type", spanNameForClass(view.getClass()));
}
}
}
@Override
protected void onEnd(
AttributesBuilder attributes, ModelAndView modelAndView, @Nullable Void unused) {}
}

View File

@ -0,0 +1,26 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.View;
public class ModelAndViewSpanNameExtractor implements SpanNameExtractor<ModelAndView> {
@Override
public String extract(ModelAndView modelAndView) {
String viewName = modelAndView.getViewName();
if (viewName != null) {
return "Render " + viewName;
}
View view = modelAndView.getView();
if (view != null) {
return "Render " + view.getClass().getSimpleName();
}
// either viewName or view should be non-null, but just in case
return "Render <unknown>";
}
}

View File

@ -0,0 +1,30 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import javax.servlet.http.HttpServletRequest;
import org.springframework.web.servlet.HandlerMapping;
public class ServerNameUpdater {
public static void updateServerSpanName(Context context, HttpServletRequest request) {
if (request != null) {
Object bestMatchingPattern =
request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
if (bestMatchingPattern != null) {
ServerSpanNaming.updateServerSpanName(
context,
CONTROLLER,
() -> ServletContextPath.prepend(context, bestMatchingPattern.toString()));
}
}
}
}

View File

@ -0,0 +1,43 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import org.springframework.web.servlet.ModelAndView;
public final class SpringWebMvcInstrumenters {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.javaagent.spring-webmvc-3.1";
private static final Instrumenter<Object, Void> HANDLER_INSTRUMENTER;
private static final Instrumenter<ModelAndView, Void> MODEL_AND_VIEW_INSTRUMENTER;
static {
HANDLER_INSTRUMENTER =
Instrumenter.<Object, Void>newBuilder(
GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, new HandlerSpanNameExtractor())
.newInstrumenter();
MODEL_AND_VIEW_INSTRUMENTER =
Instrumenter.<ModelAndView, Void>newBuilder(
GlobalOpenTelemetry.get(),
INSTRUMENTATION_NAME,
new ModelAndViewSpanNameExtractor())
.addAttributesExtractor(new ModelAndViewAttributesExtractor())
.newInstrumenter();
}
public static Instrumenter<Object, Void> handlerInstrumenter() {
return HANDLER_INSTRUMENTER;
}
public static Instrumenter<ModelAndView, Void> modelAndViewInstrumenter() {
return MODEL_AND_VIEW_INSTRUMENTER;
}
private SpringWebMvcInstrumenters() {}
}

View File

@ -1,130 +0,0 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
import io.opentelemetry.api.trace.SpanBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import java.lang.reflect.Method;
import javax.servlet.Servlet;
import javax.servlet.http.HttpServletRequest;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.springframework.web.HttpRequestHandler;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.servlet.HandlerMapping;
import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.View;
import org.springframework.web.servlet.mvc.Controller;
public class SpringWebMvcTracer extends BaseTracer {
private static final SpringWebMvcTracer TRACER = new SpringWebMvcTracer();
private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES =
Config.get()
.getBooleanProperty(
"otel.instrumentation.spring-webmvc.experimental-span-attributes", false);
public static SpringWebMvcTracer tracer() {
return TRACER;
}
@Nullable
public Context startHandlerSpan(Context parentContext, Object handler) {
String spanName = spanNameOnHandle(handler);
if (spanName != null) {
return startSpan(parentContext, spanName, INTERNAL);
}
return null;
}
public Context startSpan(ModelAndView mv) {
Context parentContext = Context.current();
SpanBuilder span = spanBuilder(parentContext, spanNameOnRender(mv), INTERNAL);
onRender(span, mv);
return parentContext.with(span.startSpan());
}
public void updateServerSpanName(Context context, HttpServletRequest request) {
if (request != null) {
Object bestMatchingPattern =
request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
if (bestMatchingPattern != null) {
ServerSpanNaming.updateServerSpanName(
context,
CONTROLLER,
() -> ServletContextPath.prepend(context, bestMatchingPattern.toString()));
}
}
}
private String spanNameOnHandle(Object handler) {
Class<?> clazz;
String methodName;
if (handler instanceof HandlerMethod) {
// name span based on the class and method name defined in the handler
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 if (handler.getClass().getName().startsWith("org.grails.")) {
// skip creating handler span for grails, grails instrumentation will take care of it
return null;
} else {
// perhaps org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter
clazz = handler.getClass();
methodName = "<annotation>";
}
return spanNameForMethod(clazz, methodName);
}
private String spanNameOnRender(ModelAndView mv) {
String viewName = mv.getViewName();
if (viewName != null) {
return "Render " + viewName;
}
View view = mv.getView();
if (view != null) {
return "Render " + view.getClass().getSimpleName();
}
// either viewName or view should be non-null, but just in case
return "Render <unknown>";
}
private void onRender(SpanBuilder span, ModelAndView mv) {
if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) {
span.setAttribute("spring-webmvc.view.name", mv.getViewName());
View view = mv.getView();
if (view != null) {
span.setAttribute("spring-webmvc.view.type", spanNameForClass(view.getClass()));
}
}
}
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.spring-webmvc-3.1";
}
}

View File

@ -54,11 +54,6 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
true
}
@Override
boolean hasExceptionOnServerSpan(ServerEndpoint endpoint) {
true
}
@Override
boolean hasRenderSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT