From d0c2e8c0ec8a4374c5e7d69abe8b18a5e95d5af3 Mon Sep 17 00:00:00 2001 From: bradAtTraceable <105816707+bradAtTraceable@users.noreply.github.com> Date: Wed, 1 Jun 2022 13:28:48 -0700 Subject: [PATCH] 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 --- .../Servlet30AndFilterInstrumentation.java | 13 ++- .../servlet/v3_0/nowrapping/Utils.java | 15 ++- .../async/BodyCaptureAsyncListener.java | 14 ++- .../async/Servlet30AsyncInstrumentation.java | 8 +- .../ServletRequestInstrumentation.java | 96 ++++++++++++++++++- .../v3_0/nowrapping/request/Utils.java | 14 +++ javaagent-core/build.gradle.kts | 1 + .../buffer/StringMapSpanPair.java | 53 ++++++++++ .../utils/ContentTypeUtils.java | 17 ++++ otel-extensions/build.gradle.kts | 2 +- 10 files changed, 209 insertions(+), 24 deletions(-) create mode 100644 javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/StringMapSpanPair.java diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java index 6aabe915d..6479a1914 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/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 readerContextStore = VirtualField.find(BufferedReader.class, CharBufferSpanPair.class); + VirtualField 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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java index a6468d3ef..3c271ca11 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java @@ -21,6 +21,7 @@ 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; @@ -28,10 +29,7 @@ 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 requestContextStore, VirtualField streamContextStore, - VirtualField bufferedReaderContextStore) { + VirtualField bufferedReaderContextStore, + VirtualField 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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/BodyCaptureAsyncListener.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/BodyCaptureAsyncListener.java index 3e2fb7621..4b48fc46b 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/BodyCaptureAsyncListener.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/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 { @@ -52,6 +49,7 @@ public final class BodyCaptureAsyncListener implements ServletAsyncListener requestContextStore; private final VirtualField inputStreamContextStore; private final VirtualField readerContextStore; + private final VirtualField urlEncodedMapContextStore; private final HttpServletRequest request; public BodyCaptureAsyncListener( @@ -62,6 +60,7 @@ public BodyCaptureAsyncListener( VirtualField requestContextStore, VirtualField inputStreamContextStore, VirtualField readerContextStore, + VirtualField 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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/Servlet30AsyncInstrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/Servlet30AsyncInstrumentation.java index 26a5461f1..3c106f281 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/async/Servlet30AsyncInstrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/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 readerContextStore = VirtualField.find(BufferedReader.class, CharBufferSpanPair.class); + VirtualField 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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletRequestInstrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletRequestInstrumentation.java index ddcaafd2e..b701924e3 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletRequestInstrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/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 stringMap; + + VirtualField 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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java index 3c4d84f19..27d412c9b 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/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 stringMap, Span span, Map headers) { + return new StringMapSpanPair(span, stringMap, headers); + } } diff --git a/javaagent-core/build.gradle.kts b/javaagent-core/build.gradle.kts index e4364bf12..42473059e 100644 --- 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/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/StringMapSpanPair.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/StringMapSpanPair.java new file mode 100644 index 000000000..911050e40 --- /dev/null +++ b/javaagent-core/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 headers; + public final Map stringMap; + + /** A flag to signalize that map has been added to span. */ + private boolean mapCaptured; + + public StringMapSpanPair(Span span, Map stringMap, Map 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 attributeKey) { + if (!mapCaptured) { + String json = convertToJSONString(stringMap); + span.setAttribute(attributeKey, json); + mapCaptured = true; + } + } +} diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/utils/ContentTypeUtils.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/utils/ContentTypeUtils.java index 84840307b..4ee6a4e3e 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/utils/ContentTypeUtils.java +++ b/javaagent-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); + } + } } diff --git a/otel-extensions/build.gradle.kts b/otel-extensions/build.gradle.kts index eaa70aaa2..51d0c2056 100644 --- a/otel-extensions/build.gradle.kts +++ b/otel-extensions/build.gradle.kts @@ -47,7 +47,7 @@ dependencies { api("com.google.protobuf:protobuf-java") api("com.google.protobuf:protobuf-java-util") // convert yaml to json, since java protobuf impl supports only json - implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.12.6") + implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.13.3") // fix vulnerability constraints { api("com.google.code.gson:gson:2.8.9")