Merge pull request #52 from nicmunroe/feature/old-servlet-support

Add proper support for a single RequestTracingFilter that supports Servlet 2.x and Servlet 3 environments at the same time

README.md

@@ -15,10 +15,8 @@ There are a few modules associated with this project:
 functionality.
 * [wingtips-java8](wingtips-java8/README.md) - Provides several Java 8 helpers, particularly around helping tracing and 
 MDC information to hop threads in asynchronous/non-blocking use cases.
-* [wingtips-servlet-api](wingtips-servlet-api/README.md) - A plugin for Servlet 3+ based applications for integrating 
-distributed tracing with a simple Servlet Filter.
-* [wingtips-old-servlet-api](wingtips-old-servlet-api/README.md) - A plugin for Servlet 2.x based applications for 
-integrating distributed tracing with a simple Servlet Filter.
+* [wingtips-servlet-api](wingtips-servlet-api/README.md) - A plugin for Servlet based applications for integrating 
+distributed tracing with a simple Servlet Filter. Supports Servlet 2.x and Servlet 3 (async request) environments. 
 * [wingtips-zipkin](wingtips-zipkin/README.md) - A plugin providing easy Zipkin integration by converting Wingtips 
 spans to Zipkin spans and sending them to a Zipkin server.
 
@@ -118,10 +116,9 @@ The `extractParentSpanFromRequest()` method is potentially different for differe
 <a name="servlet_filter_info"></a>  
 #### Is your application running in a Servlet-based framework?
 
-If your application is running in a Servlet environment (e.g. Spring MVC, Jersey, raw Servlets, etc) then this entire 
-lifecycle can be handled by a Servlet `Filter`. We've created one for you that's ready to drop in and go - see the 
-[wingtips-servlet-api](wingtips-servlet-api/README.md) Wingtips plugin module library for details if you're in a 
-Servlet 3+ environment. For Servlet 2.x see [wingtips-old-servlet-api](wingtips-old-servlet-api/README.md). That plugin 
+If your application is running in a Servlet environment (e.g. Spring Boot, Spring MVC, Jersey, raw Servlets, etc) then 
+this entire lifecycle can be handled by a Servlet `Filter`. We've created one for you that's ready to drop in and go - 
+see the [wingtips-servlet-api](wingtips-servlet-api/README.md) Wingtips plugin module library for details. That plugin 
 module is also a good resource to see how the code for a production-ready implementation of this library might look.
 
 <a name="try_with_resources_info"></a>

build.gradle

@@ -87,7 +87,7 @@ ext {
         //      like try-with-resources that generate many many branches in the bytecode that are realistically impossible to get coverage for.
         //      The combination of those issues mean we get artificially low coverage numbers even though it's clean correct code, so we just
         //      have to visually verify it.
-        configure(subprojects.findAll { !it.name.contains("wingtips-zipkin") && !it.name.startsWith("sample")}) {
+        configure(subprojects.findAll { !it.name.contains("wingtips-zipkin") && !it.name.startsWith("sample") && !it.name.startsWith("testonly")}) {
             jacocoCoverage {
                 // Enforce minimum code coverage. See https://github.com/palantir/gradle-jacoco-coverage for the full list of options.
                 reportThreshold 0.95, INSTRUCTION

gradle/bintrayPublishing.gradle

@@ -1,5 +1,5 @@
 configure(subprojects.findAll {
-    return !it.name.startsWith("sample")
+    return !it.name.startsWith("sample") && !it.name.startsWith("testonly")
 }) {
     apply plugin: 'maven'
     apply plugin: 'maven-publish'

gradle/jacoco.gradle

@@ -32,8 +32,8 @@ subprojects {
 def subprojectsToIncludeForJacocoComboReport(Set<Project> origSubprojects) {
     Set<Project> projectsToInclude = new HashSet<>()
     for (Project subproj : origSubprojects) {
-        // For this project we'll include everything that's not a sample
-        if (!subproj.getName().startsWith("sample")) {
+        // For this project we'll include everything that's not a sample or a testonly module
+        if (!subproj.getName().startsWith("sample") && !subproj.getName().startsWith("testonly")) {
             projectsToInclude.add(subproj)
         }
     }

settings.gradle

@@ -2,10 +2,11 @@ rootProject.name = 'wingtips'
 
         // Published-artifact modules
 include "wingtips-core",
-        "wingtips-old-servlet-api",
         "wingtips-servlet-api",
         "wingtips-zipkin",
         "wingtips-java8",
+        // Test-only modules (not published)
+        "testonly:testonly-old-servlet",
         // Sample modules (not published)
         "samples:sample-jersey1",
         "samples:sample-jersey2",

testonly/testonly-old-servlet/README.md

@@ -0,0 +1,17 @@
+# Wingtips - testonly-old-servlet
+
+Wingtips is a distributed tracing solution for Java 7 and greater based on the 
+[Google Dapper paper](http://static.googleusercontent.com/media/research.google.com/en/us/pubs/archive/36356.pdf).
+
+This submodule contains tests to verify that the [wingtips-servlet-api](../../wingtips-servlet-api) module's 
+functionality works as expected in old Servlet 2.x environments. We need a separate tests-only module for this because
+we need to limit the dependencies at test runtime to force a Servlet 2.x-only environment.
+
+## More Info
+
+See the [base project README.md](../../README.md) and Wingtips repository source code and javadocs for general Wingtips 
+information.
+
+## License
+
+Wingtips is released under the [Apache License, Version 2.0](http://www.apache.org/licenses/LICENSE-2.0)

wingtips-old-servlet-api/build.gradle → testonly/testonly-old-servlet/build.gradle

@@ -6,19 +6,15 @@ ext {
 }
 
 dependencies {
-    compile(
-            project(":wingtips-core"),
-    )
-    compileOnly(
-            "javax.servlet:servlet-api:$oldServletApiTargetVersion"
-    )
     testCompile(
+            project(":wingtips-servlet-api"),
             "junit:junit-dep:$junitVersion",
             "org.mockito:mockito-core:$mockitoVersion",
             "ch.qos.logback:logback-classic:$logbackVersion",
             "org.assertj:assertj-core:$assertJVersion",
             "com.tngtech.java:junit-dataprovider:$junitDataproviderVersion",
             "io.rest-assured:rest-assured:$restAssuredVersion",
+            "javax.servlet:servlet-api:$oldServletApiTargetVersion",
             "org.mortbay.jetty:jetty:$oldMortbayJettyVersion"
     )
 }

wingtips-old-servlet-api/src/test/java/com/nike/wingtips/componenttest/RequestTracingFilterOldServletComponentTest.java → testonly/testonly-old-servlet/src/test/java/com/nike/wingtips/componenttest/RequestTracingFilterOldServletComponentTest.java

@@ -6,7 +6,7 @@
 import com.nike.wingtips.TraceHeaders;
 import com.nike.wingtips.Tracer;
 import com.nike.wingtips.lifecyclelistener.SpanLifecycleListener;
-import com.nike.wingtips.servlet.RequestTracingFilterOldServlet;
+import com.nike.wingtips.servlet.RequestTracingFilter;
 
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
@@ -32,6 +32,7 @@
 import java.util.concurrent.TimeUnit;
 
 import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -40,9 +41,11 @@
 
 import static io.restassured.RestAssured.given;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 
 /**
- * Component test to verify that {@link RequestTracingFilterOldServlet} works as expected when deployed to a real running server.
+ * Component test to verify that {@link RequestTracingFilter} works as expected when deployed to a real running server
+ * that *only* supports Servlet 2.x (no Servlet 3 API available on the classpath).
  *
  * @author Nic Munroe
  */
@@ -55,7 +58,30 @@ public class RequestTracingFilterOldServletComponentTest {
     private SpanRecorder spanRecorder;
 
     @BeforeClass
+    @SuppressWarnings("JavaReflectionMemberAccess")
     public static void beforeClass() throws Exception {
+        try {
+            ServletRequest.class.getMethod("getAsyncContext");
+            fail(
+                "Expected this test to run in an environment that does *NOT* support Servlet 3 API, "
+                + "however ServletRequest.getAsyncContext() method was found."
+            );
+        }
+        catch(NoSuchMethodException ex) {
+            // Expected - do nothing
+        }
+
+        try {
+            Class.forName("javax.servlet.AsyncListener");
+            fail(
+                "Expected this test to run in an environment that does *NOT* support Servlet 3 API, "
+                + "however javax.servlet.AsyncListener class was found."
+            );
+        }
+        catch(ClassNotFoundException ex) {
+            // Expected - do nothing
+        }
+
         port = findFreePort();
         server = new Server(port);
         server.setHandler(generateServletContextHandler());
@@ -187,7 +213,7 @@ private void verifySingleSpanCompletedAndReturnedInResponse(ExtractableResponse
 
     public static class SpanRecorder implements SpanLifecycleListener {
 
-        public final List<Span> completedSpans = new ArrayList<>();
+        final List<Span> completedSpans = new ArrayList<>();
 
         @Override
         public void spanStarted(Span span) { }
@@ -242,7 +268,7 @@ private static Handler generateServletContextHandler() throws IOException {
 
         servletHandler.addServletWithMapping(BlockingServlet.class, BLOCKING_PATH);
         servletHandler.addServletWithMapping(BlockingForwardServlet.class, BLOCKING_FORWARD_PATH);
-        servletHandler.addFilterWithMapping(RequestTracingFilterOldServlet.class.getName(), "/*", Handler.ALL);
+        servletHandler.addFilterWithMapping(RequestTracingFilter.class.getName(), "/*", Handler.ALL);
 
         Context context = new Context(null, null, null, servletHandler, null);
         context.setContextPath("/");