fix version scan and test versions

dd-java-agent/instrumentation/jsp-2.3/jsp-2.3.gradle

@@ -3,7 +3,7 @@ apply plugin: 'version-scan'
 versionScan {
   group = "org.apache.tomcat"
   module = "tomcat-jasper"
-  versions = "[8.0.1,)"
+  versions = "[7.0.0,)"
   scanDependencies = true
   verifyPresent = [
     "org.apache.jasper.servlet.JspServletWrapper": null,
@@ -21,7 +21,8 @@ testSets {
 }
 
 dependencies {
-  compileOnly group: 'org.apache.tomcat', name: 'tomcat-jasper', version: '7.0.42'
+  // compiling against tomcat 7.0.20 because there seems to be some issues with Tomcat's dependency < 7.0.20
+  compileOnly group: 'org.apache.tomcat', name: 'tomcat-jasper', version: '7.0.20'
   compileOnly group: 'javax.servlet.jsp', name: 'javax.servlet.jsp-api', version: '2.3.0'
   compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.1.0'
 
@@ -35,10 +36,11 @@ dependencies {
 
   testCompile project(':dd-java-agent:testing')
   testCompile project(':dd-java-agent:instrumentation:servlet-3')
-  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '7.0.42'
-  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '7.0.42'
-  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-logging-juli', version: '7.0.42'
-  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '7.0.42'
+  // using tomcat 7.0.37 because there seems to be some issues with Tomcat's jar scanning in versions < 7.0.37
+  // https://stackoverflow.com/questions/23484098/org-apache-tomcat-util-bcel-classfile-classformatexception-invalid-byte-tag-in
+  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '7.0.37'
+  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-logging-juli', version: '7.0.37'
+  testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '7.0.37'
 
   testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.10.0'
 }
@@ -49,5 +51,6 @@ configurations.latestDepTestCompile {
     force group: 'javax.servlet', name: 'javax.servlet-api', version: '+'
     force group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '+'
     force group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '+'
+    force group: 'org.apache.tomcat.embed', name: 'tomcat-embed-logging-juli', version: '+'
   }
 }

dd-java-agent/instrumentation/jsp-2.3/src/main/java/datadog/trace/instrumentation/jsp/JasperJSPCompilationContextInstrumentation.java

@@ -56,11 +56,12 @@ public final class JasperJSPCompilationContextInstrumentation extends Instrument
               .buildSpan("jsp.compile")
               .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
               .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
-              .withTag(
-                  "servlet.context", jspCompilationContext.getServletContext().getContextPath())
               .startActive(true);
 
       final Span span = scope.span();
+      if (jspCompilationContext.getServletContext() != null) {
+        span.setTag("servlet.context", jspCompilationContext.getServletContext().getContextPath());
+      }
       span.setTag(DDTags.RESOURCE_NAME, jspCompilationContext.getJspFile());
       Tags.COMPONENT.set(span, "jsp-http-servlet");
       return scope;
@@ -74,13 +75,16 @@ public final class JasperJSPCompilationContextInstrumentation extends Instrument
 
       final Span span = scope.span();
       if (jspCompilationContext != null) {
-        span.setTag("jsp.compiler", jspCompilationContext.getCompiler().getClass().getName());
+        if (jspCompilationContext.getCompiler() != null) {
+          span.setTag("jsp.compiler", jspCompilationContext.getCompiler().getClass().getName());
+        }
         span.setTag("jsp.classFQCN", jspCompilationContext.getFQCN());
         if (throwable != null) {
           span.setTag("jsp.javaFile", jspCompilationContext.getServletJavaFileName());
           span.setTag("jsp.classpath", jspCompilationContext.getClassPath());
         }
       }
+
       if (throwable != null) {
         Tags.ERROR.set(span, Boolean.TRUE);
         span.log(Collections.singletonMap(ERROR_OBJECT, throwable));

dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy

@@ -11,8 +11,6 @@ import okhttp3.Response
 import org.apache.catalina.Context
 import org.apache.catalina.startup.Tomcat
 import org.apache.jasper.JasperException
-//import org.apache.tomcat.JarScanFilter
-//import org.apache.tomcat.JarScanType
 import spock.lang.Unroll
 
 import static datadog.trace.agent.test.ListWriterAssert.assertTraces
@@ -21,6 +19,12 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
 
   static {
     System.setProperty("dd.integration.jsp.enabled", "true")
+    // skip jar scanning using environment variables:
+    // http://tomcat.apache.org/tomcat-7.0-doc/config/systemprops.html#JAR_Scanning
+    // having this set allows us to test with old versions of the tomcat api since
+    // JarScanFilter did not exist in the tomcat 7 api
+    System.setProperty("org.apache.catalina.startup.ContextConfig.jarsToSkip", "*")
+    System.setProperty("org.apache.catalina.startup.TldConfig.jarsToSkip", "*")
   }
 
   static final int PORT = TestUtils.randomOpenPort()
@@ -54,14 +58,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
     appContext = tomcatServer.addWebapp("/$JSP_WEBAPP_CONTEXT",
       JSPInstrumentationBasicTests.getResource("/webapps/jsptest").getPath())
 
-    // Speed up startup by disabling jar scanning:
-//    appContext.getJarScanner().setJarScanFilter(new JarScanFilter() {
-//      @Override
-//      boolean check(JarScanType jarScanType, String jarName) {
-//        return false
-//      }
-//    })
-
     tomcatServer.start()
     System.out.println(
       "Tomcat server: http://" + tomcatServer.getHost().getName() + ":" + PORT + "/")

dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy

@@ -7,8 +7,6 @@ import okhttp3.*
 import org.apache.catalina.Context
 import org.apache.catalina.startup.Tomcat
 import org.apache.jasper.JasperException
-//import org.apache.tomcat.JarScanFilter
-//import org.apache.tomcat.JarScanType
 import spock.lang.Unroll
 
 import static datadog.trace.agent.test.ListWriterAssert.assertTraces
@@ -17,6 +15,12 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
 
   static {
     System.setProperty("dd.integration.jsp.enabled", "true")
+    // skip jar scanning using environment variables:
+    // http://tomcat.apache.org/tomcat-7.0-doc/config/systemprops.html#JAR_Scanning
+    // having this set allows us to test with old versions of the tomcat api since
+    // JarScanFilter did not exist in the tomcat 7 api
+    System.setProperty("org.apache.catalina.startup.ContextConfig.jarsToSkip", "*")
+    System.setProperty("org.apache.catalina.startup.TldConfig.jarsToSkip", "*")
   }
 
   static final int PORT = TestUtils.randomOpenPort()
@@ -50,14 +54,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
     appContext = tomcatServer.addWebapp("/$JSP_WEBAPP_CONTEXT",
       JSPInstrumentationForwardTests.getResource("/webapps/jsptest").getPath())
 
-    // Speed up startup by disabling jar scanning:
-//    appContext.getJarScanner().setJarScanFilter(new JarScanFilter() {
-//      @Override
-//      boolean check(JarScanType jarScanType, String jarName) {
-//        return false
-//      }
-//    })
-
     tomcatServer.start()
     System.out.println(
       "Tomcat server: http://" + tomcatServer.getHost().getName() + ":" + PORT + "/")

settings.gradle

@@ -32,7 +32,7 @@ include ':dd-java-agent:instrumentation:jedis-1.4'
 include ':dd-java-agent:instrumentation:jetty-8'
 include ':dd-java-agent:instrumentation:jms-1'
 include ':dd-java-agent:instrumentation:jms-2'
-include ':dd-java-agent:instrumentation:jsp'
+include ':dd-java-agent:instrumentation:jsp-2.3'
 include ':dd-java-agent:instrumentation:kafka-clients-0.11'
 include ':dd-java-agent:instrumentation:kafka-streams-0.11'
 include ':dd-java-agent:instrumentation:lettuce-5'