Add servlet 2.2 instrumentation

instrumentation/servlet/servlet-2.2/build.gradle.kts

@@ -16,7 +16,7 @@ muzzle {
     fail {
         group = "javax.servlet"
         module = "javax.servlet-api"
-        versions = "(,)"
+        versions = "[3.0,)"
     }
 }
 
@@ -37,12 +37,4 @@ dependencies {
     testRuntimeOnly("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-servlet-common-bootstrap:${versions["opentelemetry_java_agent"]}")
     muzzleBootstrap("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-servlet-common-bootstrap:${versions["opentelemetry_java_agent"]}")
     compileOnly("javax.servlet:servlet-api:2.2")
-
-//    testInstrumentation(project(":instrumentation:servlet:servlet-3.0:javaagent"))
-//
-//    testLibrary("org.eclipse.jetty:jetty-server:7.0.0.v20091005")
-//    testLibrary("org.eclipse.jetty:jetty-servlet:7.0.0.v20091005")
-//
-//    latestDepTestLibrary("org.eclipse.jetty:jetty-server:7.+") // see servlet-3.0 module
-//    latestDepTestLibrary("org.eclipse.jetty:jetty-servlet:7.+") // see servlet-3.0 module
 }

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v2_2/nowrapping/Servlet2AndFilterInstrumentation.java

@@ -66,7 +66,6 @@ public ElementMatcher<TypeDescription> typeMatcher() {
 
   @Override
   public void transform(TypeTransformer transformer) {
-    // print version with Implmentation Version
     transformer.applyAdviceToMethod(
         namedOneOf("doFilter", "service")
             .and(takesArgument(0, named("javax.servlet.ServletRequest")))
@@ -79,17 +78,17 @@ public void transform(TypeTransformer transformer) {
   public static class ServletAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class, skipOn = Advice.OnNonDefaultValue.class)
-    public static boolean start(
+    public static void start(
         @Advice.Argument(value = 0) ServletRequest request,
         @Advice.Argument(value = 1) ServletResponse response,
         @Advice.Local("currentSpan") Span currentSpan) {
       int callDepth =
           HypertraceCallDepthThreadLocalMap.incrementCallDepth(Servlet2InstrumentationName.class);
       if (callDepth > 0) {
-        return false;
+        return;
       }
       if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
-        return false;
+        return;
       }
 
       HttpServletRequest httpRequest = (HttpServletRequest) request;
@@ -123,7 +122,7 @@ public static boolean start(
       if (FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers)) {
         httpResponse.setStatus(403);
         // skip execution of the user code
-        return true;
+        return;
       }
 
       if (instrumentationConfig.httpBody().request()
@@ -135,7 +134,7 @@ public static boolean start(
                 httpRequest,
                 new SpanAndObjectPair(currentSpan, Collections.unmodifiableMap(headers)));
       }
-      return false;
+      return;
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
@@ -179,7 +178,6 @@ public static void exit(
             VirtualField.find(HttpServletRequest.class, StringMapSpanPair.class);
 
         // capture response body
-        // TODO: capture response headers
         // FIXME: capture body based on response content type
         if (instrumentationConfig.httpBody().response()) {
           Utils.captureResponseBody(

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v2_2/nowrapping/request/ServletInputStreamInstrumentation.java

@@ -65,15 +65,20 @@ public void transform(TypeTransformer transformer) {
     transformer.applyAdviceToMethod(
         named("readAllBytes").and(takesArguments(0)).and(isPublic()),
         ServletInputStreamInstrumentation.class.getName() + "$InputStream_ReadAllBytes");
-    // TODO: readNBytes(int len) is not transformed
     transformer.applyAdviceToMethod(
         named("readNBytes")
-            .and(takesArguments(0))
+            .and(takesArguments(3))
             .and(takesArgument(0, is(byte[].class)))
             .and(takesArgument(1, is(int.class)))
             .and(takesArgument(2, is(int.class)))
             .and(isPublic()),
         ServletInputStreamInstrumentation.class.getName() + "$InputStream_ReadNBytes");
+    transformer.applyAdviceToMethod(
+        named("readNBytes")
+            .and(takesArguments(1))
+            .and(takesArgument(0, is(int.class)))
+            .and(isPublic()),
+        ServletInputStreamInstrumentation.class.getName() + "$InputStream_ReadNBytesLength");
 
     // ServletInputStream methods
     transformer.applyAdviceToMethod(
@@ -322,4 +327,48 @@ public static void exit(
       }
     }
   }
+
+  @SuppressWarnings("unused")
+  public static class InputStream_ReadNBytesLength {
+
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static ByteBufferSpanPair enter(@Advice.This ServletInputStream thizz) {
+      ByteBufferSpanPair bufferSpanPair =
+          VirtualField.find(ServletInputStream.class, ByteBufferSpanPair.class).get(thizz);
+      if (bufferSpanPair == null) {
+        return null;
+      }
+
+      HypertraceCallDepthThreadLocalMap.incrementCallDepth(ServletInputStream.class);
+      return bufferSpanPair;
+    }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class)
+    public static void exit(
+        @Advice.This ServletInputStream thizz,
+        @Advice.Return byte[] b,
+        @Advice.Enter ByteBufferSpanPair bufferSpanPair)
+        throws IOException {
+      try {
+        if (bufferSpanPair == null) {
+          return;
+        }
+        int callDepth =
+            HypertraceCallDepthThreadLocalMap.decrementCallDepth(ServletInputStream.class);
+        if (callDepth > 0) {
+          return;
+        }
+        bufferSpanPair.writeToBuffer(b);
+        if (thizz.available() == 0) {
+          bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
+        }
+      } catch (Throwable t) {
+        if (t instanceof HypertraceEvaluationException) {
+          throw t;
+        } else {
+          // ignore
+        }
+      }
+    }
+  }
 }

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v2_2/nowrapping/request/ServletRequestInstrumentation.java

@@ -53,10 +53,7 @@ public void transform(TypeTransformer transformer) {
             .and(isPublic()),
         ServletRequestInstrumentation.class.getName() + "$ServletRequest_getInputStream_advice");
     transformer.applyAdviceToMethod(
-        named("getReader")
-            .and(takesArguments(0))
-            //            .and(returns(BufferedReader.class))
-            .and(isPublic()),
+        named("getReader").and(takesArguments(0)).and(isPublic()),
         ServletRequestInstrumentation.class.getName() + "$ServletRequest_getReader_advice");
     transformer.applyAdviceToMethod(
         named("getParameter")

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v2_2/nowrapping/response/ServletOutputStreamInstrumentation.java

@@ -69,9 +69,6 @@ public void transform(TypeTransformer transformer) {
             .and(takesArgument(2, is(int.class)))
             .and(isPublic()),
         ServletOutputStreamInstrumentation.class.getName() + "$OutputStream_writeByteArrOffset");
-
-    // close() is not instrumented due to some issue with Tomcat
-    // refer the comment in servlet-3.0 code
   }
 
   @SuppressWarnings("unused")

instrumentation/servlet/servlet-3.0/build.gradle.kts

@@ -9,7 +9,8 @@ muzzle {
     pass {
         group = "javax.servlet"
         module = "javax.servlet-api"
-        versions = "[3.0.0,)"
+        versions = "[3.0,)"
+        assertInverse = true
     }
     // fail on all old servlet-api. This groupId was changed in 3.x to javax.servlet-api
     fail {

instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentation.java

@@ -68,7 +68,7 @@ public void transform(TypeTransformer transformer) {
     // TODO: readNBytes(int len) is not transformed
     transformer.applyAdviceToMethod(
         named("readNBytes")
-            .and(takesArguments(0))
+            .and(takesArguments(3))
             .and(takesArgument(0, is(byte[].class)))
             .and(takesArgument(1, is(int.class)))
             .and(takesArgument(2, is(int.class)))

instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletRequestInstrumentation.java

@@ -53,10 +53,7 @@ public void transform(TypeTransformer transformer) {
             .and(isPublic()),
         ServletRequestInstrumentation.class.getName() + "$ServletRequest_getInputStream_advice");
     transformer.applyAdviceToMethod(
-        named("getReader")
-            .and(takesArguments(0))
-            //            .and(returns(BufferedReader.class))
-            .and(isPublic()),
+        named("getReader").and(takesArguments(0)).and(isPublic()),
         ServletRequestInstrumentation.class.getName() + "$ServletRequest_getReader_advice");
     transformer.applyAdviceToMethod(
         named("getParameter")
@@ -182,7 +179,7 @@ static class ServletRequest_getParameter_advice {
     /**
      * Instrumentation template for ServletRequest.getParameter() entry point.
      *
-     * @param servletRequest servletRequest
+     * @param servletRequest the ServletRequest instance
      * @return a (possibly null) SpanAndObjectPair, which will be passed to the method exit
      *     instrumentation
      */

instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java

@@ -64,12 +64,12 @@ public static CharBufferSpanPair createRequestCharBufferSpanPair(
   }
 
   /**
-   * Create a StringMapSpanPair.
+   * Create a StringMapSpanPair with provided values.
    *
-   * @param stringMap stringMap
-   * @param span span
-   * @param headers headers
-   * @return StringMapSpanPair
+   * @param stringMap the stringMap instance
+   * @param span the span instance
+   * @param headers the headers instance
+   * @return a StringMapSpanPair created with values of the above instances
    */
   public static StringMapSpanPair createStringMapSpanPair(
       Map<String, String> stringMap, Span span, Map<String, String> headers) {

javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/StringMapSpanPair.java

@@ -39,9 +39,9 @@ public StringMapSpanPair(Span span, Map<String, String> stringMap, Map<String, S
   }
 
   /**
-   * Return a JSON string representing the contents of the x-www-form-urlencoded body.
+   * Capture the contents of the x-www-form-urlencoded body in the span with provided attributeKey.
    *
-   * @param attributeKey attributeKey
+   * @param attributeKey the attributeKey to store contents of the captured body in the span
    */
   public void captureBody(AttributeKey<String> attributeKey) {
     if (!mapCaptured) {

javaagent/build.gradle.kts

@@ -13,7 +13,6 @@ dependencies {
     // https://dl.bintray.com/open-telemetry/maven/
     implementation("io.opentelemetry.javaagent", "opentelemetry-javaagent", version = "${versions["opentelemetry_java_agent_all"]}")
     implementation(project(":filter-api"))
-    implementation("io.opentelemetry:opentelemetry-sdk-trace:1.13.0")
 }
 
 base.archivesBaseName = "hypertrace-agent"