From 78fe61061f0ef864badb5b6534c718006b010707 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 12 Dec 2017 23:50:27 +0800 Subject: [PATCH] Separates MVC AsyncHandler as it requires Spring 3+; lowers java levels This separates out the async MVC functionality and tests the synchronous part works with Spring 2.5. This also lowers the minimum java level of instrumentation likely to be used in Spring 2.5 applications. --- instrumentation/httpclient/pom.xml | 2 + instrumentation/jaxrs2/pom.xml | 2 + instrumentation/mysql/pom.xml | 2 + instrumentation/servlet/pom.xml | 2 + instrumentation/spring-webmvc/README.md | 15 +-- .../spring-webmvc/src/it/spring25/README.md | 2 + .../spring-webmvc/src/it/spring25/pom.xml | 74 ++++++++++++++ .../webmvc25/ITTracingHandlerInterceptor.java | 90 +++++++++++++++++ .../TracingAsyncHandlerInterceptor.java | 42 ++++++++ .../webmvc/TracingHandlerInterceptor.java | 12 ++- .../ITTracingAsyncHandlerInterceptor.java | 99 +++++++++++++++++++ .../webmvc/ITTracingHandlerInterceptor.java | 6 +- ...ngAsyncHandlerInterceptorAutowireTest.java | 35 +++++++ 13 files changed, 370 insertions(+), 13 deletions(-) create mode 100644 instrumentation/spring-webmvc/src/it/spring25/README.md create mode 100644 instrumentation/spring-webmvc/src/it/spring25/pom.xml create mode 100644 instrumentation/spring-webmvc/src/it/spring25/src/test/java/brave/spring/webmvc25/ITTracingHandlerInterceptor.java create mode 100644 instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java create mode 100644 instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java create mode 100644 instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/TracingAsyncHandlerInterceptorAutowireTest.java diff --git a/instrumentation/httpclient/pom.xml b/instrumentation/httpclient/pom.xml index f334e362f4..5460ded090 100644 --- a/instrumentation/httpclient/pom.xml +++ b/instrumentation/httpclient/pom.xml @@ -12,6 +12,8 @@ ${project.basedir}/../.. + 1.6 + java16 diff --git a/instrumentation/jaxrs2/pom.xml b/instrumentation/jaxrs2/pom.xml index 93e176258c..b8ce3e0406 100644 --- a/instrumentation/jaxrs2/pom.xml +++ b/instrumentation/jaxrs2/pom.xml @@ -12,6 +12,8 @@ ${project.basedir}/../.. + 1.6 + java16 diff --git a/instrumentation/mysql/pom.xml b/instrumentation/mysql/pom.xml index e23972dc8b..60cc49b1e8 100644 --- a/instrumentation/mysql/pom.xml +++ b/instrumentation/mysql/pom.xml @@ -12,6 +12,8 @@ ${project.basedir}/../.. + 1.6 + java16 diff --git a/instrumentation/servlet/pom.xml b/instrumentation/servlet/pom.xml index 48bcc70bd6..7c3298702b 100644 --- a/instrumentation/servlet/pom.xml +++ b/instrumentation/servlet/pom.xml @@ -12,6 +12,8 @@ ${project.basedir}/../.. + 1.6 + java16 diff --git a/instrumentation/spring-webmvc/README.md b/instrumentation/spring-webmvc/README.md index ea8e777ce1..139f41e575 100644 --- a/instrumentation/spring-webmvc/README.md +++ b/instrumentation/spring-webmvc/README.md @@ -1,8 +1,8 @@ # brave-instrumentation-spring-webmvc -This module contains a tracing interceptor for [Spring WebMVC](https://docs.spring.io/spring/docs/current/spring-framework-reference/html/mvc.html) -`TracingHandlerInterceptor` extracts trace state from incoming requests. -Then, it reports Zipkin how long each request takes, along with relevant -tags like the http url. +This module contains tracing interceptors for [Spring WebMVC](https://docs.spring.io/spring/docs/current/spring-framework-reference/html/mvc.html) +`TracingAsyncHandlerInterceptor` and `TracingHandlerInterceptor` extract trace state from incoming +requests. Then, they report Zipkin how long each request takes, along with relevant tags like the +http url. ## Configuration @@ -11,19 +11,20 @@ it is in place before proceeding. Here's an example in [XML](https://github.com/ Then, configure `TracingHandlerInterceptor` in either XML or Java. +Here's an example of using the synchronous handler, which works with Spring 2.5+ ```xml - - + ``` +Here's an example of the asynchronous-capable handler, which works with Spring 3+ ```java @Configuration @EnableWebMvc class TracingConfig extends WebMvcConfigurerAdapter { @Bean AsyncHandlerInterceptor tracingInterceptor(HttpTracing httpTracing) { - return TracingHandlerInterceptor.create(httpTracing); + return TracingAsyncHandlerInterceptor.create(httpTracing); } @Autowired diff --git a/instrumentation/spring-webmvc/src/it/spring25/README.md b/instrumentation/spring-webmvc/src/it/spring25/README.md new file mode 100644 index 0000000000..fe9edff997 --- /dev/null +++ b/instrumentation/spring-webmvc/src/it/spring25/README.md @@ -0,0 +1,2 @@ +# nozipkin1 +This tests that Tracing can be initialized without a runtime dependency on `io.zipkin.java:zipkin` diff --git a/instrumentation/spring-webmvc/src/it/spring25/pom.xml b/instrumentation/spring-webmvc/src/it/spring25/pom.xml new file mode 100644 index 0000000000..06112f8f55 --- /dev/null +++ b/instrumentation/spring-webmvc/src/it/spring25/pom.xml @@ -0,0 +1,74 @@ + + + 4.0.0 + + @project.groupId@ + spring-webmvc-spring25 + @project.version@ + spring-webmvc-spring25 + + + 1.8 + 1.8 + + + + + javax.servlet + servlet-api + 2.5 + provided + + + + org.springframework + spring-webmvc + 2.5.6 + provided + + + + org.eclipse.jetty + jetty-servlet + @jetty-servlet25.version@ + + + + ${project.groupId} + brave-instrumentation-spring-webmvc + ${project.version} + + + + ${project.groupId} + brave-instrumentation-http-tests + ${project.version} + + + + junit + junit + @junit.version@ + + + + org.assertj + assertj-core + @assertj.version@ + + + + + + + maven-failsafe-plugin + @maven-failsafe-plugin.version@ + + true + + + + + diff --git a/instrumentation/spring-webmvc/src/it/spring25/src/test/java/brave/spring/webmvc25/ITTracingHandlerInterceptor.java b/instrumentation/spring-webmvc/src/it/spring25/src/test/java/brave/spring/webmvc25/ITTracingHandlerInterceptor.java new file mode 100644 index 0000000000..3d1ccf45f0 --- /dev/null +++ b/instrumentation/spring-webmvc/src/it/spring25/src/test/java/brave/spring/webmvc25/ITTracingHandlerInterceptor.java @@ -0,0 +1,90 @@ +package brave.spring.webmvc25; + +import brave.Tracer; +import brave.http.HttpTracing; +import brave.http.ITServletContainer; +import brave.spring.webmvc.TracingHandlerInterceptor; +import java.io.IOException; +import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.Test; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.support.StaticWebApplicationContext; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter; +import org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping; + +import static org.springframework.web.servlet.DispatcherServlet.HANDLER_ADAPTER_BEAN_NAME; +import static org.springframework.web.servlet.DispatcherServlet.HANDLER_MAPPING_BEAN_NAME; + +public class ITTracingHandlerInterceptor extends ITServletContainer { + + @Controller + public static class TestController { + final Tracer tracer; + + @Autowired public TestController(HttpTracing httpTracing) { + this.tracer = httpTracing.tracing().tracer(); + } + + @RequestMapping(value = "/foo") + public void foo(HttpServletResponse response) throws IOException { + response.getWriter().write("foo"); + } + + @RequestMapping(value = "/badrequest") + public void badrequest(HttpServletResponse response) throws IOException { + response.sendError(400); + response.flushBuffer(); + } + + @RequestMapping(value = "/child") + public void child() { + tracer.nextSpan().name("child").start().finish(); + } + + @RequestMapping(value = "/exception") + public void disconnect() throws IOException { + throw new IOException(); + } + } + + // TODO: investigate why the status isn't being added as a tag + @Override @Test(expected = AssertionError.class) + public void addsStatusCode_badRequest() throws Exception { + super.addsStatusCode_badRequest(); + } + + @Override public void init(ServletContextHandler handler) { + StaticWebApplicationContext wac = new StaticWebApplicationContext(); + wac.getBeanFactory() + .registerSingleton("testController", new TestController(httpTracing)); // the test resource + + DefaultAnnotationHandlerMapping mapping = new DefaultAnnotationHandlerMapping(); + mapping.setInterceptors(new Object[] {TracingHandlerInterceptor.create(httpTracing)}); + mapping.setApplicationContext(wac); + + wac.getBeanFactory().registerSingleton(HANDLER_MAPPING_BEAN_NAME, mapping); + wac.getBeanFactory().registerSingleton(HANDLER_ADAPTER_BEAN_NAME, new AnnotationMethodHandlerAdapter()); + + handler.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, wac); + handler.addServlet(new ServletHolder(new DispatcherServlet() { + { + wac.refresh(); + setDetectAllHandlerMappings(false); + setDetectAllHandlerAdapters(false); + setPublishEvents(false); + } + + @Override protected WebApplicationContext initWebApplicationContext() throws BeansException { + onRefresh(wac); + return wac; + } + }), "/*"); + } +} diff --git a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java new file mode 100644 index 0000000000..b140b5e3c6 --- /dev/null +++ b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java @@ -0,0 +1,42 @@ +package brave.spring.webmvc; + +import brave.Tracing; +import brave.http.HttpTracing; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.web.servlet.AsyncHandlerInterceptor; +import org.springframework.web.servlet.HandlerInterceptor; +import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; + +/** + * Tracing interceptor for Spring Web MVC, which can be used as both an {@link + * AsyncHandlerInterceptor} or a normal {@link HandlerInterceptor}. + */ +public final class TracingAsyncHandlerInterceptor extends HandlerInterceptorAdapter { + public static AsyncHandlerInterceptor create(Tracing tracing) { + return new TracingAsyncHandlerInterceptor(HttpTracing.create(tracing)); + } + + public static AsyncHandlerInterceptor create(HttpTracing httpTracing) { + return new TracingAsyncHandlerInterceptor(httpTracing); + } + + final HandlerInterceptor delegate; + + @Autowired TracingAsyncHandlerInterceptor(HttpTracing httpTracing) { // internal + delegate = TracingHandlerInterceptor.create(httpTracing); + } + + @Override + public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object o) + throws Exception { + return delegate.preHandle(request, response, o); + } + + @Override + public void afterCompletion(HttpServletRequest request, HttpServletResponse response, + Object o, Exception ex) throws Exception { + delegate.afterCompletion(request, response, o, ex); + } +} diff --git a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java index aac46616e2..e06f1cc6f5 100644 --- a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java +++ b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java @@ -14,13 +14,14 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.web.servlet.AsyncHandlerInterceptor; import org.springframework.web.servlet.HandlerInterceptor; +import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; /** * Tracing interceptor for Spring Web MVC, which can be used as both an {@link * AsyncHandlerInterceptor} or a normal {@link HandlerInterceptor}. */ -public final class TracingHandlerInterceptor extends HandlerInterceptorAdapter { +public final class TracingHandlerInterceptor implements HandlerInterceptor { static final Propagation.Getter GETTER = new Propagation.Getter() { @Override public String get(HttpServletRequest carrier, String key) { @@ -32,11 +33,11 @@ public final class TracingHandlerInterceptor extends HandlerInterceptorAdapter { } }; - public static AsyncHandlerInterceptor create(Tracing tracing) { + public static HandlerInterceptor create(Tracing tracing) { return new TracingHandlerInterceptor(HttpTracing.create(tracing)); } - public static AsyncHandlerInterceptor create(HttpTracing httpTracing) { + public static HandlerInterceptor create(HttpTracing httpTracing) { return new TracingHandlerInterceptor(httpTracing); } @@ -61,6 +62,11 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons return true; } + @Override + public void postHandle(HttpServletRequest request, HttpServletResponse response, Object handler, + ModelAndView modelAndView) throws Exception { + } + @Override public void afterCompletion(HttpServletRequest request, HttpServletResponse response, Object o, Exception ex) { diff --git a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java new file mode 100644 index 0000000000..92727b1575 --- /dev/null +++ b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java @@ -0,0 +1,99 @@ +package brave.spring.webmvc; + +import brave.Tracer; +import brave.http.HttpTracing; +import brave.http.ITServletContainer; +import java.io.IOException; +import java.util.concurrent.Callable; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; +import org.springframework.web.servlet.AsyncHandlerInterceptor; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.config.annotation.EnableWebMvc; +import org.springframework.web.servlet.config.annotation.InterceptorRegistry; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; + +public class ITTracingAsyncHandlerInterceptor extends ITServletContainer { + + @Controller static class TestController { + final Tracer tracer; + + @Autowired TestController(HttpTracing httpTracing) { + this.tracer = httpTracing.tracing().tracer(); + } + + @RequestMapping(value = "/foo") + public ResponseEntity foo() throws IOException { + return new ResponseEntity<>(HttpStatus.OK); + } + + @RequestMapping(value = "/badrequest") + public ResponseEntity badrequest() throws IOException { + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + } + + @RequestMapping(value = "/child") + public ResponseEntity child() { + tracer.nextSpan().name("child").start().finish(); + return new ResponseEntity<>(HttpStatus.OK); + } + + @RequestMapping(value = "/async") + public Callable> async() throws IOException { + return () -> new ResponseEntity<>(HttpStatus.OK); + } + + @RequestMapping(value = "/exception") + public ResponseEntity disconnect() throws IOException { + throw new IOException(); + } + + @RequestMapping(value = "/exceptionAsync") + public Callable> disconnectAsync() throws IOException { + return () -> { + throw new IOException(); + }; + } + } + + @Configuration + @EnableWebMvc + static class TracingConfig extends WebMvcConfigurerAdapter { + @Bean AsyncHandlerInterceptor tracingInterceptor(HttpTracing httpTracing) { + return TracingAsyncHandlerInterceptor.create(httpTracing); + } + + @Autowired + private AsyncHandlerInterceptor tracingInterceptor; + + @Override + public void addInterceptors(InterceptorRegistry registry) { + registry.addInterceptor(tracingInterceptor); + } + } + + @Override public void init(ServletContextHandler handler) { + AnnotationConfigWebApplicationContext appContext = + new AnnotationConfigWebApplicationContext() { + // overriding this allows us to register dependencies of TracingHandlerInterceptor + // without passing static state to a configuration class. + @Override protected void loadBeanDefinitions(DefaultListableBeanFactory beanFactory) { + beanFactory.registerSingleton("httpTracing", httpTracing); + super.loadBeanDefinitions(beanFactory); + } + }; + + appContext.register(TestController.class); // the test resource + appContext.register(TracingConfig.class); // generic tracing setup + handler.addServlet(new ServletHolder(new DispatcherServlet(appContext)), "/*"); + } +} diff --git a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java index f60089d623..94e08968db 100644 --- a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java +++ b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java @@ -16,8 +16,8 @@ import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; -import org.springframework.web.servlet.AsyncHandlerInterceptor; import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.config.annotation.InterceptorRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; @@ -68,12 +68,12 @@ public Callable> disconnectAsync() throws IOException { @Configuration @EnableWebMvc static class TracingConfig extends WebMvcConfigurerAdapter { - @Bean AsyncHandlerInterceptor tracingInterceptor(HttpTracing httpTracing) { + @Bean HandlerInterceptor tracingInterceptor(HttpTracing httpTracing) { return TracingHandlerInterceptor.create(httpTracing); } @Autowired - private AsyncHandlerInterceptor tracingInterceptor; + private HandlerInterceptor tracingInterceptor; @Override public void addInterceptors(InterceptorRegistry registry) { diff --git a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/TracingAsyncHandlerInterceptorAutowireTest.java b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/TracingAsyncHandlerInterceptorAutowireTest.java new file mode 100644 index 0000000000..d0a6d2a7a8 --- /dev/null +++ b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/TracingAsyncHandlerInterceptorAutowireTest.java @@ -0,0 +1,35 @@ +package brave.spring.webmvc; + +import brave.Tracing; +import brave.http.HttpTracing; +import org.junit.Test; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.web.servlet.HandlerInterceptor; + +public class TracingAsyncHandlerInterceptorAutowireTest { + + @Configuration static class HttpTracingConfiguration { + @Bean HttpTracing httpTracing() { + return HttpTracing.create(Tracing.newBuilder().build()); + } + } + + // NOTE: while bean configuration via @Import works with Spring 4, it does not with Spring 3 + @Configuration @Import(HttpTracingConfiguration.class) + static class BeanConfiguration { + @Bean HandlerInterceptor tracingInterceptor(HttpTracing httpTracing) { + return TracingAsyncHandlerInterceptor.create(httpTracing); + } + } + + @Test public void autowiredWithBeanConfig() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(BeanConfiguration.class); + ctx.refresh(); + + ctx.getBean(HandlerInterceptor.class); + } +}