ENG-16503 Show request body for x-www-form-urlencoded HTTP requests (#…

…363) * ENG-16503 Show request body for x-www-form-urlencoded HTTP requests * Fix snyk-scan issues * Switch to version 2.13.3 of jackson.dataformat * Use Jackson to convert object into JSON * Fix formatting of ContentTypeUtils
hypertrace · Jun 1, 2022 · d0c2e8c · d0c2e8c
1 parent 8f9e708
commit d0c2e8c
Show file tree

Hide file tree

Showing 10 changed files with 209 additions and 24 deletions.
diff --git a/...instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java b/...instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java
@@ -49,10 +49,7 @@
 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;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedCharArrayWriter;
-import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
-import org.hypertrace.agent.core.instrumentation.buffer.CharBufferSpanPair;
+import org.hypertrace.agent.core.instrumentation.buffer.*;
 import org.hypertrace.agent.core.instrumentation.utils.ContentTypeUtils;
 import org.hypertrace.agent.filter.FilterRegistry;
 
@@ -180,6 +177,8 @@ public static void exit(
           VirtualField.find(ServletInputStream.class, ByteBufferSpanPair.class);
       VirtualField<BufferedReader, CharBufferSpanPair> readerContextStore =
           VirtualField.find(BufferedReader.class, CharBufferSpanPair.class);
+      VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore =
+          VirtualField.find(HttpServletRequest.class, StringMapSpanPair.class);
 
       if (!request.isAsyncStarted()) {
         if (instrumentationConfig.httpHeaders().response()) {
@@ -205,7 +204,11 @@ public static void exit(
         if (instrumentationConfig.httpBody().request()
             && ContentTypeUtils.shouldCapture(httpRequest.getContentType())) {
           Utils.resetRequestBodyBuffers(
-              httpRequest, requestContextStore, inputStreamContextStore, readerContextStore);
+              httpRequest,
+              requestContextStore,
+              inputStreamContextStore,
+              readerContextStore,
+              urlEncodedMapContextStore);
         }
       }
     }

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
@@ -21,17 +21,15 @@
 import java.io.BufferedReader;
 import java.io.PrintWriter;
 import java.io.UnsupportedEncodingException;
+import java.util.Map;
 import javax.servlet.ServletInputStream;
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes;
 import org.hypertrace.agent.core.instrumentation.SpanAndObjectPair;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedCharArrayWriter;
-import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
-import org.hypertrace.agent.core.instrumentation.buffer.CharBufferSpanPair;
+import org.hypertrace.agent.core.instrumentation.buffer.*;
 
 public class Utils {
 
@@ -87,7 +85,8 @@ public static void resetRequestBodyBuffers(
       HttpServletRequest httpServletRequest,
       VirtualField<HttpServletRequest, SpanAndObjectPair> requestContextStore,
       VirtualField<ServletInputStream, ByteBufferSpanPair> streamContextStore,
-      VirtualField<BufferedReader, CharBufferSpanPair> bufferedReaderContextStore) {
+      VirtualField<BufferedReader, CharBufferSpanPair> bufferedReaderContextStore,
+      VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore) {
 
     SpanAndObjectPair requestStreamReaderHolder = requestContextStore.get(httpServletRequest);
     if (requestStreamReaderHolder == null) {
@@ -114,6 +113,12 @@ public static void resetRequestBodyBuffers(
         charBufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
         bufferedReaderContextStore.set(bufferedReader, null);
       }
+    } else if (requestStreamReaderHolder.getAssociatedObject() instanceof Map) {
+      StringMapSpanPair stringMapSpanPair = urlEncodedMapContextStore.get(httpServletRequest);
+      if (stringMapSpanPair != null) {
+        stringMapSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
+        urlEncodedMapContextStore.set(httpServletRequest, null);
+      }
     }
   }
 }
diff --git a/...nt/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/BodyCaptureAsyncListener.java b/...nt/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/BodyCaptureAsyncListener.java
@@ -31,10 +31,7 @@
 import org.hypertrace.agent.core.config.InstrumentationConfig;
 import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes;
 import org.hypertrace.agent.core.instrumentation.SpanAndObjectPair;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedCharArrayWriter;
-import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
-import org.hypertrace.agent.core.instrumentation.buffer.CharBufferSpanPair;
+import org.hypertrace.agent.core.instrumentation.buffer.*;
 import org.hypertrace.agent.core.instrumentation.utils.ContentTypeUtils;
 
 public final class BodyCaptureAsyncListener implements ServletAsyncListener<HttpServletResponse> {
@@ -52,6 +49,7 @@ public final class BodyCaptureAsyncListener implements ServletAsyncListener<Http
   private final VirtualField<HttpServletRequest, SpanAndObjectPair> requestContextStore;
   private final VirtualField<ServletInputStream, ByteBufferSpanPair> inputStreamContextStore;
   private final VirtualField<BufferedReader, CharBufferSpanPair> readerContextStore;
+  private final VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore;
   private final HttpServletRequest request;
 
   public BodyCaptureAsyncListener(
@@ -62,6 +60,7 @@ public BodyCaptureAsyncListener(
       VirtualField<HttpServletRequest, SpanAndObjectPair> requestContextStore,
       VirtualField<ServletInputStream, ByteBufferSpanPair> inputStreamContextStore,
       VirtualField<BufferedReader, CharBufferSpanPair> readerContextStore,
+      VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore,
       HttpServletRequest request) {
     this.responseHandled = responseHandled;
     this.span = Span.fromContext(Servlet3Singletons.helper().getServerContext(request));
@@ -71,6 +70,7 @@ public BodyCaptureAsyncListener(
     this.requestContextStore = requestContextStore;
     this.inputStreamContextStore = inputStreamContextStore;
     this.readerContextStore = readerContextStore;
+    this.urlEncodedMapContextStore = urlEncodedMapContextStore;
     this.request = request;
   }
 
@@ -115,7 +115,11 @@ private void captureResponseDataAndClearRequestBuffer(
       if (instrumentationConfig.httpBody().request()
           && ContentTypeUtils.shouldCapture(servletRequest.getContentType())) {
         Utils.resetRequestBodyBuffers(
-            servletRequest, requestContextStore, inputStreamContextStore, readerContextStore);
+            servletRequest,
+            requestContextStore,
+            inputStreamContextStore,
+            readerContextStore,
+            urlEncodedMapContextStore);
       }
     }
   }

diff --git a/...strumentation/hypertrace/servlet/v3_0/nowrapping/async/Servlet30AsyncInstrumentation.java b/...strumentation/hypertrace/servlet/v3_0/nowrapping/async/Servlet30AsyncInstrumentation.java
@@ -42,10 +42,7 @@
 import net.bytebuddy.matcher.ElementMatcher;
 import org.hypertrace.agent.core.instrumentation.HypertraceCallDepthThreadLocalMap;
 import org.hypertrace.agent.core.instrumentation.SpanAndObjectPair;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream;
-import org.hypertrace.agent.core.instrumentation.buffer.BoundedCharArrayWriter;
-import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
-import org.hypertrace.agent.core.instrumentation.buffer.CharBufferSpanPair;
+import org.hypertrace.agent.core.instrumentation.buffer.*;
 
 public final class Servlet30AsyncInstrumentation implements TypeInstrumentation {
 
@@ -97,6 +94,8 @@ public static void startAsyncExit(@Advice.This ServletRequest servletRequest) {
           VirtualField.find(ServletInputStream.class, ByteBufferSpanPair.class);
       VirtualField<BufferedReader, CharBufferSpanPair> readerContextStore =
           VirtualField.find(BufferedReader.class, CharBufferSpanPair.class);
+      VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore =
+          VirtualField.find(HttpServletRequest.class, StringMapSpanPair.class);
 
       if (servletRequest instanceof HttpServletRequest) {
         HttpServletRequest request = (HttpServletRequest) servletRequest;
@@ -114,6 +113,7 @@ public static void startAsyncExit(@Advice.This ServletRequest servletRequest) {
                   requestContextStore,
                   inputStreamContextStore,
                   readerContextStore,
+                  urlEncodedMapContextStore,
                   request),
               helper.getAsyncListenerResponse(request));
           accessor.setRequestAttribute(request, HYPERTRACE_ASYNC_LISTENER_ATTRIBUTE, true);

diff --git a/...rumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletRequestInstrumentation.java b/...rumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletRequestInstrumentation.java
@@ -17,15 +17,14 @@
 package io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.request;
 
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperType;
-import static net.bytebuddy.matcher.ElementMatchers.isPublic;
-import static net.bytebuddy.matcher.ElementMatchers.named;
-import static net.bytebuddy.matcher.ElementMatchers.returns;
-import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+import static net.bytebuddy.matcher.ElementMatchers.*;
 
 import io.opentelemetry.instrumentation.api.field.VirtualField;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import java.io.BufferedReader;
+import java.util.HashMap;
+import java.util.Map;
 import javax.servlet.ServletInputStream;
 import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServletRequest;
@@ -36,6 +35,7 @@
 import org.hypertrace.agent.core.instrumentation.SpanAndObjectPair;
 import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
 import org.hypertrace.agent.core.instrumentation.buffer.CharBufferSpanPair;
+import org.hypertrace.agent.core.instrumentation.buffer.StringMapSpanPair;
 
 public class ServletRequestInstrumentation implements TypeInstrumentation {
 
@@ -58,6 +58,13 @@ public void transform(TypeTransformer transformer) {
             //            .and(returns(BufferedReader.class))
             .and(isPublic()),
         ServletRequestInstrumentation.class.getName() + "$ServletRequest_getReader_advice");
+    transformer.applyAdviceToMethod(
+        named("getParameter")
+            .and(takesArguments(1))
+            .and(takesArgument(0, is(String.class)))
+            .and(returns(String.class))
+            .and(isPublic()),
+        ServletRequestInstrumentation.class.getName() + "$ServletRequest_getParameter_advice");
   }
 
   static class ServletRequest_getInputStream_advice {
@@ -165,4 +172,85 @@ public static void exit(
       spanAndObjectPair.setAssociatedObject(reader);
     }
   }
+
+  /** Provides instrumentation template for ServletRequest.getParameter() method. */
+  static class ServletRequest_getParameter_advice {
+
+    /**
+     * Instrumentation template for ServletRequest.getParameter() entry point.
+     *
+     * @param servletRequest
+     * @return a (possibly null) SpanAndObjectPair, which will be passed to the method exit
+     *     instrumentation
+     */
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static SpanAndObjectPair enter(@Advice.This ServletRequest servletRequest) {
+      HttpServletRequest httpServletRequest = (HttpServletRequest) servletRequest;
+      SpanAndObjectPair spanAndObjectPair =
+          VirtualField.find(HttpServletRequest.class, SpanAndObjectPair.class)
+              .get(httpServletRequest);
+      if (spanAndObjectPair == null) {
+        return null;
+      }
+
+      HypertraceCallDepthThreadLocalMap.incrementCallDepth(ServletRequest.class);
+      return spanAndObjectPair;
+    }
+
+    /**
+     * Instrumentation template for ServletRequest.getParameter() exit point(s).
+     *
+     * @param servletRequest the ServletRequest instance
+     * @param returnValue the value that is being returned by getParameter()
+     * @param parmName the argument that was passed to getParameter()
+     * @param throwable the Throwable object, if exiting method because of a 'throw'
+     * @param spanAndObjectPair the value returned by the getParameter() method entry
+     *     instrumentation
+     */
+    @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
+    public static void exit(
+        @Advice.This ServletRequest servletRequest,
+        @Advice.Return String returnValue,
+        @Advice.Argument(0) String parmName,
+        @Advice.Thrown Throwable throwable,
+        @Advice.Enter SpanAndObjectPair spanAndObjectPair) {
+      if (spanAndObjectPair == null) {
+        return;
+      }
+
+      int callDepth = HypertraceCallDepthThreadLocalMap.decrementCallDepth(ServletRequest.class);
+      if (callDepth > 0) {
+        return;
+      }
+
+      if (returnValue == null) {
+        return;
+      }
+
+      if (!(servletRequest instanceof HttpServletRequest) || throwable != null) {
+        return;
+      }
+      HttpServletRequest httpServletRequest = (HttpServletRequest) servletRequest;
+
+      Map<String, String> stringMap;
+
+      VirtualField<HttpServletRequest, StringMapSpanPair> contextStore =
+          VirtualField.find(HttpServletRequest.class, StringMapSpanPair.class);
+
+      StringMapSpanPair stringMapSpanPair = contextStore.get(httpServletRequest);
+
+      if (stringMapSpanPair != null) {
+        stringMap = stringMapSpanPair.stringMap;
+      } else {
+        stringMap = new HashMap<>();
+        stringMapSpanPair =
+            Utils.createStringMapSpanPair(
+                stringMap, spanAndObjectPair.getSpan(), spanAndObjectPair.getHeaders());
+        contextStore.set(httpServletRequest, stringMapSpanPair);
+      }
+
+      stringMap.put(parmName, returnValue);
+      spanAndObjectPair.setAssociatedObject(stringMap);
+    }
+  }
 }
diff --git a/...telemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java b/...telemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java
@@ -23,6 +23,7 @@
 import org.hypertrace.agent.core.instrumentation.buffer.BoundedBuffersFactory;
 import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
 import org.hypertrace.agent.core.instrumentation.buffer.CharBufferSpanPair;
+import org.hypertrace.agent.core.instrumentation.buffer.StringMapSpanPair;
 import org.hypertrace.agent.core.instrumentation.utils.ContentLengthUtils;
 import org.hypertrace.agent.core.instrumentation.utils.ContentTypeCharsetUtils;
 import org.hypertrace.agent.filter.FilterRegistry;
@@ -61,4 +62,17 @@ public static CharBufferSpanPair createRequestCharBufferSpanPair(
         filter::evaluateRequestBody,
         headers);
   }
+
+  /**
+   * Create a StringMapSpanPair.
+   *
+   * @param stringMap
+   * @param span
+   * @param headers
+   * @return
+   */
+  public static StringMapSpanPair createStringMapSpanPair(
+      Map<String, String> stringMap, Span span, Map<String, String> headers) {
+    return new StringMapSpanPair(span, stringMap, headers);
+  }
 }
diff --git a/javaagent-core/build.gradle.kts b/javaagent-core/build.gradle.kts
@@ -9,4 +9,5 @@ dependencies {
     api("io.opentelemetry:opentelemetry-api:${versions["opentelemetry"]}")
     api("io.opentelemetry.javaagent:opentelemetry-javaagent-instrumentation-api:${versions["opentelemetry_java_agent"]}")
     implementation("org.slf4j:slf4j-api:${versions["slf4j"]}")
+    implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.13.3")
 }
diff --git a/...ore/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/StringMapSpanPair.java b/...ore/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/StringMapSpanPair.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright The Hypertrace Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.hypertrace.agent.core.instrumentation.buffer;
+
+import static org.hypertrace.agent.core.instrumentation.utils.ContentTypeUtils.convertToJSONString;
+
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.trace.Span;
+import java.util.Map;
+
+/** Created to represent the request body that is in x-www-form-urlencoded format. */
+public class StringMapSpanPair {
+
+  public final Span span;
+  public final Map<String, String> headers;
+  public final Map<String, String> stringMap;
+
+  /** A flag to signalize that map has been added to span. */
+  private boolean mapCaptured;
+
+  public StringMapSpanPair(Span span, Map<String, String> stringMap, Map<String, String> headers) {
+    this.span = span;
+    this.stringMap = stringMap;
+    this.headers = headers;
+  }
+
+  /**
+   * Return a JSON string representing the contents of the x-www-form-urlencoded body.
+   *
+   * @param attributeKey
+   */
+  public void captureBody(AttributeKey<String> attributeKey) {
+    if (!mapCaptured) {
+      String json = convertToJSONString(stringMap);
+      span.setAttribute(attributeKey, json);
+      mapCaptured = true;
+    }
+  }
+}
diff --git a/...-core/src/main/java/org/hypertrace/agent/core/instrumentation/utils/ContentTypeUtils.java b/...-core/src/main/java/org/hypertrace/agent/core/instrumentation/utils/ContentTypeUtils.java
@@ -16,6 +16,9 @@
 
 package org.hypertrace.agent.core.instrumentation.utils;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 public class ContentTypeUtils {
 
   private ContentTypeUtils() {}
@@ -62,4 +65,18 @@ public static String parseCharset(String contentType) {
     }
     return null;
   }
+
+  /**
+   * Converts an arbitrary object into JSON format.
+   *
+   * @param obj
+   * @return
+   */
+  public static String convertToJSONString(Object obj) {
+    try {
+      return new ObjectMapper().writeValueAsString(obj);
+    } catch (JsonProcessingException e) {
+      throw new RuntimeException(e);
+    }
+  }
 }