Capture servlet request bodies explicitly (#270)

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
hypertrace · Feb 18, 2021 · d004d6d · d004d6d
1 parent dad264d
commit d004d6d
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 4 deletions.
diff --git a/.../io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java b/.../io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java
@@ -96,11 +96,24 @@ public static void resetRequestBodyBuffers(
     requestContextStore.put(httpServletRequest, null);
 
     if (requestStreamReaderHolder.getAssociatedObject() instanceof ServletInputStream) {
-      streamContextStore.put(
-          (ServletInputStream) requestStreamReaderHolder.getAssociatedObject(), null);
+      ServletInputStream servletInputStream =
+          (ServletInputStream) requestStreamReaderHolder.getAssociatedObject();
+      ByteBufferSpanPair byteBufferSpanPair = streamContextStore.get(servletInputStream);
+      if (byteBufferSpanPair != null) {
+        // capture body explicitly e.g. Jackson does not call ServletInputStream$read() until -1 is
+        // returned
+        // it does not even call ServletInputStream#available()
+        byteBufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
+        streamContextStore.put(servletInputStream, null);
+      }
     } else if (requestStreamReaderHolder.getAssociatedObject() instanceof BufferedReader) {
-      bufferedReaderContextStore.put(
-          (BufferedReader) requestStreamReaderHolder.getAssociatedObject(), null);
+      BufferedReader bufferedReader =
+          (BufferedReader) requestStreamReaderHolder.getAssociatedObject();
+      CharBufferSpanPair charBufferSpanPair = bufferedReaderContextStore.get(bufferedReader);
+      if (charBufferSpanPair != null) {
+        charBufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
+        bufferedReaderContextStore.put(bufferedReader, null);
+      }
     }
   }
 }
diff --git a/...umentation/hypertrace/servlet/v3_0/nowrapping/Servlet30NoWrappingInstrumentationTest.java b/...umentation/hypertrace/servlet/v3_0/nowrapping/Servlet30NoWrappingInstrumentationTest.java
@@ -18,9 +18,11 @@
 
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoAsyncResponse_stream;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoAsyncResponse_writer;
+import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoReader_read_large_array;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoStream_arr;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoStream_arr_offset;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoStream_readLine_print;
+import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoStream_read_large_array;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoStream_single_byte;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.EchoWriter_single_char;
 import io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.TestServlets.GetHello;
@@ -73,6 +75,8 @@ public static void startServer() throws Exception {
     handler.addServlet(TestServlets.Forward_to_post.class, "/forward_to_echo");
     handler.addServlet(EchoAsyncResponse_stream.class, "/echo_async_response_stream");
     handler.addServlet(EchoAsyncResponse_writer.class, "/echo_async_response_writer");
+    handler.addServlet(EchoStream_read_large_array.class, "/echo_stream_read_large_array");
+    handler.addServlet(EchoReader_read_large_array.class, "/echo_reader_read_large_array");
     server.setHandler(handler);
     server.start();
     serverPort = server.getConnectors()[0].getLocalPort();
@@ -103,6 +107,16 @@ public void postJson_stream_single_byte() throws Exception {
     postJson(String.format("http://localhost:%d/echo_stream_single_byte", serverPort));
   }
 
+  @Test
+  public void postJson_stream_read_large_array() throws Exception {
+    postJson(String.format("http://localhost:%d/echo_stream_read_large_array", serverPort));
+  }
+
+  @Test
+  public void postJson_reader_read_large_array() throws Exception {
+    postJson(String.format("http://localhost:%d/echo_reader_read_large_array", serverPort));
+  }
+
   @Test
   public void postJson_stream_arr() throws Exception {
     postJson(String.format("http://localhost:%d/echo_stream_arr", serverPort));

diff --git a/...ntelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/TestServlets.java b/...ntelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/TestServlets.java
@@ -265,4 +265,30 @@ protected void service(HttpServletRequest req, HttpServletResponse resp)
       req.getRequestDispatcher("/echo_stream_single_byte").forward(req, resp);
     }
   }
+
+  public static class EchoStream_read_large_array extends HttpServlet {
+    @Override
+    protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+      req.getInputStream().read(new byte[1000], 0, 1000);
+
+      resp.setStatus(200);
+      resp.setContentType("application/json");
+      resp.setHeader(RESPONSE_HEADER, RESPONSE_HEADER_VALUE);
+
+      resp.getWriter().print(RESPONSE_BODY.toCharArray());
+    }
+  }
+
+  public static class EchoReader_read_large_array extends HttpServlet {
+    @Override
+    protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+      req.getReader().read(new char[1000], 0, 1000);
+
+      resp.setStatus(200);
+      resp.setContentType("application/json");
+      resp.setHeader(RESPONSE_HEADER, RESPONSE_HEADER_VALUE);
+
+      resp.getWriter().print(RESPONSE_BODY.toCharArray());
+    }
+  }
 }