✨ Evaluate http bodies for Java EE with Hypertrace Filter (#349)

* ♻️ enhance mock filter to block when block=true

* ✅ add request for blocking servlet request bodies'

* 🚧 add exception for blocking request'

* ✨ suppress HT evaluation exception for servlets

* ✨ block request based on filter response when reading from servlet request bodies

* 🐛 fix bad null check resulting in NPE being thrown and polluting logs/hurting performance

* ✨ remove suppression for body reading instrumentation

* 🐛 remove log statement that causes module to not load in production

* ♻️ modify Filter API to accept headers for evaluateRequestBody

* 🚧 provide headers to ByteBufferSpanPair via ServletRequestInstrumentation

* ✨ include headers in SpanAndObjectPair

* ✨ rovide headers to ByteBufferSpanPair from SpanAndObject pair

* ✨ support body evaluation for char buffers

* ➕ add bootstrap api to compile classpath

* 🚧 functional custom exception handler

* ♻️ avoid exception handler approach, instead use try-catch in each onMethodExit

Author: ryandens

filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java

@@ -52,11 +52,11 @@ public boolean evaluateRequestHeaders(Span span, Map<String, String> headers) {
   }
 
   @Override
-  public boolean evaluateRequestBody(Span span, String body) {
+  public boolean evaluateRequestBody(Span span, String body, Map<String, String> headers) {
     boolean shouldBlock = false;
     for (Filter filter : filters) {
       try {
-        if (filter.evaluateRequestBody(span, body)) {
+        if (filter.evaluateRequestBody(span, body, headers)) {
           shouldBlock = true;
         }
       } catch (Throwable t) {

filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java

@@ -37,8 +37,10 @@ public interface Filter {
   /**
    * Evaluate the execution.
    *
+   * @param span of the HTTP request associated with this body
    * @param body request body
+   * @param headers of the request associated with this body
    * @return filter result
    */
-  boolean evaluateRequestBody(Span span, String body);
+  boolean evaluateRequestBody(Span span, String body, Map<String, String> headers);
 }

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

@@ -32,7 +32,7 @@ val versions: Map<String, String> by extra
 dependencies {
     implementation("io.opentelemetry.instrumentation:opentelemetry-servlet-3.0:${versions["opentelemetry_java_agent"]}")
     testImplementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-servlet-3.0:${versions["opentelemetry_java_agent"]}")
-
+    compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-bootstrap:${versions["opentelemetry_java_agent"]}")
     compileOnly("javax.servlet:javax.servlet-api:3.1.0")
 
     testImplementation(project(":instrumentation:servlet:servlet-rw"))

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

@@ -32,6 +32,7 @@
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
 import java.io.BufferedReader;
 import java.io.PrintWriter;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
@@ -46,6 +47,7 @@
 import net.bytebuddy.matcher.ElementMatcher;
 import org.hypertrace.agent.core.config.InstrumentationConfig;
 import org.hypertrace.agent.core.instrumentation.HypertraceCallDepthThreadLocalMap;
+import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException;
 import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes;
 import org.hypertrace.agent.core.instrumentation.SpanAndObjectPair;
 import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream;
@@ -78,6 +80,7 @@ public void transform(TypeTransformer transformer) {
   }
 
   public static class ServletAdvice {
+
     @Advice.OnMethodEnter(suppress = Throwable.class, skipOn = Advice.OnNonDefaultValue.class)
     public static boolean start(
         @Advice.Argument(value = 0) ServletRequest request,
@@ -99,15 +102,6 @@ public static boolean start(
 
       InstrumentationConfig instrumentationConfig = InstrumentationConfig.ConfigProvider.get();
 
-      String contentType = httpRequest.getContentType();
-      if (instrumentationConfig.httpBody().request()
-          && ContentTypeUtils.shouldCapture(contentType)) {
-        // The HttpServletRequest instrumentation uses this to
-        // enable the instrumentation
-        InstrumentationContext.get(HttpServletRequest.class, SpanAndObjectPair.class)
-            .put(httpRequest, new SpanAndObjectPair(currentSpan));
-      }
-
       Utils.addSessionId(currentSpan, httpRequest);
 
       // set request headers
@@ -130,13 +124,24 @@ public static boolean start(
         // skip execution of the user code
         return true;
       }
+
+      if (instrumentationConfig.httpBody().request()
+          && ContentTypeUtils.shouldCapture(httpRequest.getContentType())) {
+        // The HttpServletRequest instrumentation uses this to
+        // enable the instrumentation
+        InstrumentationContext.get(HttpServletRequest.class, SpanAndObjectPair.class)
+            .put(
+                httpRequest,
+                new SpanAndObjectPair(currentSpan, Collections.unmodifiableMap(headers)));
+      }
       return false;
     }
 
-    @Advice.OnMethodExit(suppress = Throwable.class)
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void exit(
         @Advice.Argument(0) ServletRequest request,
         @Advice.Argument(1) ServletResponse response,
+        @Advice.Thrown(readOnly = false) Throwable throwable,
         @Advice.Local("currentSpan") Span currentSpan) {
       int callDepth =
           HypertraceCallDepthThreadLocalMap.decrementCallDepth(Servlet30InstrumentationName.class);
@@ -153,6 +158,14 @@ public static void exit(
       HttpServletRequest httpRequest = (HttpServletRequest) request;
       InstrumentationConfig instrumentationConfig = InstrumentationConfig.ConfigProvider.get();
 
+      if (throwable instanceof HypertraceEvaluationException) {
+        httpResponse.setStatus(403);
+        // bytebuddy treats the reassignment of this variable to null as an instruction to suppress
+        // this exception, which is what we want
+        throwable = null;
+        return;
+      }
+
       // response context to capture body and clear the context
       ContextStore<HttpServletResponse, SpanAndObjectPair> responseContextStore =
           InstrumentationContext.get(HttpServletResponse.class, SpanAndObjectPair.class);

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

@@ -90,7 +90,7 @@ public static void resetRequestBodyBuffers(
       ContextStore<BufferedReader, CharBufferSpanPair> bufferedReaderContextStore) {
 
     SpanAndObjectPair requestStreamReaderHolder = requestContextStore.get(httpServletRequest);
-    if (requestContextStore == null) {
+    if (requestStreamReaderHolder == null) {
       return;
     }
     requestContextStore.put(httpServletRequest, null);