From efa2ac9b3d283efb152ad17cb493969875998773 Mon Sep 17 00:00:00 2001 From: d4294 Date: Mon, 21 Jan 2019 14:59:36 +0100 Subject: [PATCH 01/15] Initial commit for POC that request body can be send to APM server --- .../apm/agent/servlet/InputStreamAdvice.java | 87 ++++++++++++ .../servlet/InputStreamInstrumentation.java | 84 ++++++++++++ .../servlet/ServletTransactionHelper.java | 124 ++++++++---------- ...ic.apm.agent.bci.ElasticApmInstrumentation | 1 + 4 files changed, 224 insertions(+), 72 deletions(-) create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java new file mode 100644 index 0000000000..51031115ee --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java @@ -0,0 +1,87 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.servlet; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Map; + +import javax.annotation.Nullable; + +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.Scope; +import co.elastic.apm.agent.impl.context.TransactionContext; +import co.elastic.apm.agent.impl.transaction.Transaction; +import net.bytebuddy.asm.Advice; + +public class InputStreamAdvice { + @Nullable + @VisibleForAdvice + public static ServletTransactionHelper servletTransactionHelper; + @Nullable + @VisibleForAdvice + public static ElasticApmTracer tracer; + @VisibleForAdvice + public static ThreadLocal excluded = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + + static void init(ElasticApmTracer tracer) { + InputStreamAdvice.tracer = tracer; + servletTransactionHelper = new ServletTransactionHelper(tracer); + } + + @Nullable + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onReadEnter(@Advice.This Object thiz, @Advice.Origin Method advicedObject, @Advice.Local("transaction") Transaction transaction, + @Advice.Local("scope") Scope scope) { + // TODO Remove method from instrumentation + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onReadExit(@Advice.Argument(0) Object argument) { + + if (argument == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { + return; + } + + System.out.println(new String((byte[]) argument)); + + TransactionContext context = tracer.currentTransaction().getContext(); + Map custom = context.getCustom(); + + byte[] fullData = null; + byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); + byte[] newData = (byte[]) argument; + + if (existingData == null) { + fullData = Arrays.copyOf(newData, newData.length); + } else { + fullData = new byte[existingData.length + newData.length]; + System.arraycopy(existingData, 0, fullData, 0, existingData.length); + System.arraycopy(newData, 0, fullData, existingData.length, newData.length); + } + custom.put("REQUESTBODYDATA", fullData); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java new file mode 100644 index 0000000000..1b5dae2024 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -0,0 +1,84 @@ +package co.elastic.apm.agent.servlet; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import java.util.Collection; +import java.util.Collections; + +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 Elastic and contributors + * %% + * 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. + * #L% + */ + +import co.elastic.apm.agent.bci.ElasticApmInstrumentation; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import net.bytebuddy.description.NamedElement; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Instruments servlets to create transactions. + *

+ * If the transaction has already been recorded with the help of {@link FilterChainInstrumentation}, it does not record the transaction again. But if there is + * no filter registered for a servlet, this makes sure to record a transaction in that case. + *

+ */ +public class InputStreamInstrumentation extends ElasticApmInstrumentation { + + + static final String SERVLET_API = "servlet-api"; + + @Override + public void init(ElasticApmTracer tracer) { + InputStreamAdvice.init(tracer); + } + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameContains("ServletInputStream"); + } + + @Override + public ElementMatcher getTypeMatcher() { + return not(isInterface()).and(hasSuperType(named("java.io.InputStream"))); + } + + @Override + public ElementMatcher getMethodMatcher() { + return named("read").and(takesArgument(0, byte[].class)).and(takesArgument(1, int.class)).and(takesArgument(2, int.class)); + } + + @Override + public Class getAdviceClass() { + return InputStreamAdvice.class; + } + + @Override + public Collection getInstrumentationGroupNames() { + return Collections.singleton(SERVLET_API); + } + + +} + diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index d965df92ba..c187f4a122 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -19,6 +19,19 @@ */ package co.elastic.apm.agent.servlet; +import static co.elastic.apm.agent.web.WebConfiguration.EventType.OFF; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import javax.annotation.Nullable; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import co.elastic.apm.agent.bci.VisibleForAdvice; import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; @@ -32,21 +45,10 @@ import co.elastic.apm.agent.web.ClientIpUtils; import co.elastic.apm.agent.web.ResultUtil; import co.elastic.apm.agent.web.WebConfiguration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.annotation.Nullable; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Map; -import java.util.Objects; -import java.util.Set; - -import static co.elastic.apm.agent.web.WebConfiguration.EventType.OFF; /** - * This class must not import classes from {@code javax.servlet} due to class loader issues. - * See https://github.com/raphw/byte-buddy/issues/465 for more information. + * This class must not import classes from {@code javax.servlet} due to class loader issues. See https://github.com/raphw/byte-buddy/issues/465 for more + * information. */ @VisibleForAdvice public class ServletTransactionHelper { @@ -73,35 +75,26 @@ public ServletTransactionHelper(ElasticApmTracer tracer) { /* * As much of the request information as possible should be set before the request processing starts. * - * That way, when recording an error, - * we can copy the transaction context to the error context. + * That way, when recording an error, we can copy the transaction context to the error context. * - * This has the advantage that we don't have to create the context for the error again. - * As creating the context is framework specific, - * this also means less effort when adding support for new frameworks, - * because the creating the context is handled in one central place. + * This has the advantage that we don't have to create the context for the error again. As creating the context is framework specific, this also means less + * effort when adding support for new frameworks, because the creating the context is handled in one central place. * - * Furthermore, it is not trivial to create an error context at an arbitrary location - * (when the user calls ElasticApm.captureException()), - * as we don't necessarily have access to the framework's request and response objects. + * Furthermore, it is not trivial to create an error context at an arbitrary location (when the user calls ElasticApm.captureException()), as we don't + * necessarily have access to the framework's request and response objects. * * Additionally, we only have access to the classes of the instrumented classes inside advice methods. * - * Currently, there is no configuration option to disable tracing but to still enable error tracking. - * But even when introducing that, the approach of copying the transaction context can still work. - * We will then capture the transaction but not report it. - * As the capturing of the transaction is garbage free, this should not add a significant overhead. - * Also, this setting would be rather niche, as we are a APM solution after all. + * Currently, there is no configuration option to disable tracing but to still enable error tracking. But even when introducing that, the approach of + * copying the transaction context can still work. We will then capture the transaction but not report it. As the capturing of the transaction is garbage + * free, this should not add a significant overhead. Also, this setting would be rather niche, as we are a APM solution after all. */ @Nullable @VisibleForAdvice - public Transaction onBefore(String servletPath, @Nullable String pathInfo, - @Nullable String userAgentHeader, - @Nullable String traceContextHeader) { + public Transaction onBefore(String servletPath, @Nullable String pathInfo, @Nullable String userAgentHeader, @Nullable String traceContextHeader) { if (coreConfiguration.isActive() && - // only create a transaction if there is not already one - tracer.currentTransaction() == null && - !isExcluded(servletPath, pathInfo, userAgentHeader)) { + // only create a transaction if there is not already one + tracer.currentTransaction() == null && !isExcluded(servletPath, pathInfo, userAgentHeader)) { return tracer.startTransaction(TraceContext.fromTraceparentHeader(), traceContextHeader).activate(); } else { return null; @@ -109,9 +102,8 @@ public Transaction onBefore(String servletPath, @Nullable String pathInfo, } @VisibleForAdvice - public void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, - String scheme, String serverName, int serverPort, String requestURI, String queryString, - String remoteAddr) { + public void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, String scheme, String serverName, int serverPort, + String requestURI, String queryString, String remoteAddr) { final Request request = transaction.getContext().getRequest(); fillRequest(request, protocol, method, secure, scheme, serverName, serverPort, requestURI, queryString, remoteAddr); @@ -127,10 +119,10 @@ public static void setUsernameIfUnset(@Nullable String userName, TransactionCont @VisibleForAdvice public void onAfter(Transaction transaction, @Nullable Throwable exception, boolean committed, int status, String method, - @Nullable Map parameterMap, String servletPath, @Nullable String pathInfo, @Nullable String contentTypeHeader) { + @Nullable Map parameterMap, String servletPath, @Nullable String pathInfo, @Nullable String contentTypeHeader) { try { fillRequestParameters(transaction, method, parameterMap, contentTypeHeader); - if(exception != null && status == 200) { + if (exception != null && status == 200) { // Probably shouldn't be 200 but 5XX, but we are going to miss this... status = 500; } @@ -172,17 +164,16 @@ void applyDefaultTransactionName(String method, String servletPath, @Nullable St } /* - * Filling the parameter after the request has been processed is safer - * as reading the parameters could potentially decode them in the wrong encoding - * or trigger exceptions, - * for example when the amount of query parameters is longer than the application server allows. - * In that case, we rather not want that the agent looks like the cause for this. + * Filling the parameter after the request has been processed is safer as reading the parameters could potentially decode them in the wrong encoding or + * trigger exceptions, for example when the amount of query parameters is longer than the application server allows. In that case, we rather not want that + * the agent looks like the cause for this. */ - private void fillRequestParameters(Transaction transaction, String method, @Nullable Map parameterMap, @Nullable String contentTypeHeader) { + private void fillRequestParameters(Transaction transaction, String method, @Nullable Map parameterMap, + @Nullable String contentTypeHeader) { Request request = transaction.getContext().getRequest(); if (hasBody(contentTypeHeader, method)) { - if (webConfiguration.getCaptureBody() != OFF && parameterMap != null) { - captureBody(request, parameterMap, contentTypeHeader); + if (webConfiguration.getCaptureBody() != OFF) { + captureBody(request, parameterMap, contentTypeHeader, transaction.getContext()); } else { request.redactBody(); } @@ -191,22 +182,20 @@ private void fillRequestParameters(Transaction transaction, String method, @Null @VisibleForAdvice public boolean captureParameters(String method, @Nullable String contentTypeHeader) { - return contentTypeHeader != null - && contentTypeHeader.startsWith("application/x-www-form-urlencoded") - && hasBody(contentTypeHeader, method) - && webConfiguration.getCaptureBody() != OFF; + return contentTypeHeader != null && contentTypeHeader.startsWith("application/x-www-form-urlencoded") && hasBody(contentTypeHeader, method) + && webConfiguration.getCaptureBody() != OFF; } private boolean isExcluded(String servletPath, @Nullable String pathInfo, @Nullable String userAgentHeader) { final WildcardMatcher excludeUrlMatcher = WildcardMatcher.anyMatch(webConfiguration.getIgnoreUrls(), servletPath, pathInfo); if (excludeUrlMatcher != null) { - logger.debug("Not tracing this request as the URL {}{} is ignored by the matcher {}", - servletPath, Objects.toString(pathInfo, ""), excludeUrlMatcher); + logger.debug("Not tracing this request as the URL {}{} is ignored by the matcher {}", servletPath, Objects.toString(pathInfo, ""), + excludeUrlMatcher); } - final WildcardMatcher excludeAgentMatcher = userAgentHeader != null ? WildcardMatcher.anyMatch(webConfiguration.getIgnoreUserAgents(), userAgentHeader) : null; + final WildcardMatcher excludeAgentMatcher = + userAgentHeader != null ? WildcardMatcher.anyMatch(webConfiguration.getIgnoreUserAgents(), userAgentHeader) : null; if (excludeAgentMatcher != null) { - logger.debug("Not tracing this request as the User-Agent {} is ignored by the matcher {}", - userAgentHeader, excludeAgentMatcher); + logger.debug("Not tracing this request as the User-Agent {} is ignored by the matcher {}", userAgentHeader, excludeAgentMatcher); } return excludeUrlMatcher != null || excludeAgentMatcher != null; } @@ -217,23 +206,15 @@ private void fillResponse(Response response, boolean committed, int status) { response.withStatusCode(status); } - private void fillRequest(Request request, String protocol, String method, boolean secure, String scheme, String serverName, - int serverPort, String requestURI, String queryString, - String remoteAddr) { + private void fillRequest(Request request, String protocol, String method, boolean secure, String scheme, String serverName, int serverPort, + String requestURI, String queryString, String remoteAddr) { request.withHttpVersion(getHttpVersion(protocol)); request.withMethod(method); - request.getSocket() - .withEncrypted(secure) - .withRemoteAddress(ClientIpUtils.getRealIp(request.getHeaders(), remoteAddr)); + request.getSocket().withEncrypted(secure).withRemoteAddress(ClientIpUtils.getRealIp(request.getHeaders(), remoteAddr)); - request.getUrl() - .withProtocol(scheme) - .withHostname(serverName) - .withPort(serverPort) - .withPathname(requestURI) - .withSearch(queryString); + request.getUrl().withProtocol(scheme).withHostname(serverName).withPort(serverPort).withPathname(requestURI).withSearch(queryString); fillFullUrl(request.getUrl(), scheme, serverPort, serverName, requestURI, queryString); } @@ -242,14 +223,14 @@ private boolean hasBody(@Nullable String contentTypeHeader, String method) { return METHODS_WITH_BODY.contains(method) && contentTypeHeader != null; } - private void captureBody(Request request, Map params, @Nullable String contentTypeHeader) { + private void captureBody(Request request, Map params, @Nullable String contentTypeHeader, TransactionContext context) { if (contentTypeHeader != null && contentTypeHeader.startsWith("application/x-www-form-urlencoded")) { for (Map.Entry param : params.entrySet()) { request.addFormUrlEncodedParameters(param.getKey(), param.getValue()); } } else { - // this content-type is not supported (yet) - request.redactBody(); + System.out.println("Set rawBody to: " + new String((byte[]) context.getCustom().get("REQUESTBODYDATA"))); + request.withRawBody(new String((byte[]) context.getCustom().get("REQUESTBODYDATA"))); } } @@ -264,8 +245,7 @@ private void fillFullUrl(Url url, String scheme, int port, String serverName, St fullUrl.append(scheme); fullUrl.append("://"); fullUrl.append(serverName); - if ((scheme.equals("http") && (port != 80)) - || (scheme.equals("https") && (port != 443))) { + if ((scheme.equals("http") && (port != 80)) || (scheme.equals("https") && (port != 443))) { fullUrl.append(':'); fullUrl.append(port); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation index 463a6c9640..2a278185cf 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation @@ -2,3 +2,4 @@ co.elastic.apm.agent.servlet.ServletInstrumentation co.elastic.apm.agent.servlet.FilterChainInstrumentation co.elastic.apm.agent.servlet.AsyncInstrumentation$StartAsyncInstrumentation co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation +co.elastic.apm.agent.servlet.InputStreamInstrumentation \ No newline at end of file From 672d56153131a702d1f1df0463609bd1c882283e Mon Sep 17 00:00:00 2001 From: d4294 Date: Wed, 23 Jan 2019 17:02:53 +0100 Subject: [PATCH 02/15] Started implementing comments --- .../apm/agent/servlet/InputStreamAdvice.java | 87 ------------------- .../servlet/InputStreamInstrumentation.java | 48 ++++++++-- 2 files changed, 41 insertions(+), 94 deletions(-) delete mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java deleted file mode 100644 index 51031115ee..0000000000 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamAdvice.java +++ /dev/null @@ -1,87 +0,0 @@ -/*- - * #%L - * Elastic APM Java agent - * %% - * Copyright (C) 2018 - 2019 Elastic and contributors - * %% - * 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. - * #L% - */ -package co.elastic.apm.agent.servlet; - -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.Map; - -import javax.annotation.Nullable; - -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.Scope; -import co.elastic.apm.agent.impl.context.TransactionContext; -import co.elastic.apm.agent.impl.transaction.Transaction; -import net.bytebuddy.asm.Advice; - -public class InputStreamAdvice { - @Nullable - @VisibleForAdvice - public static ServletTransactionHelper servletTransactionHelper; - @Nullable - @VisibleForAdvice - public static ElasticApmTracer tracer; - @VisibleForAdvice - public static ThreadLocal excluded = new ThreadLocal() { - @Override - protected Boolean initialValue() { - return Boolean.FALSE; - } - }; - - static void init(ElasticApmTracer tracer) { - InputStreamAdvice.tracer = tracer; - servletTransactionHelper = new ServletTransactionHelper(tracer); - } - - @Nullable - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onReadEnter(@Advice.This Object thiz, @Advice.Origin Method advicedObject, @Advice.Local("transaction") Transaction transaction, - @Advice.Local("scope") Scope scope) { - // TODO Remove method from instrumentation - } - - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onReadExit(@Advice.Argument(0) Object argument) { - - if (argument == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { - return; - } - - System.out.println(new String((byte[]) argument)); - - TransactionContext context = tracer.currentTransaction().getContext(); - Map custom = context.getCustom(); - - byte[] fullData = null; - byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); - byte[] newData = (byte[]) argument; - - if (existingData == null) { - fullData = Arrays.copyOf(newData, newData.length); - } else { - fullData = new byte[existingData.length + newData.length]; - System.arraycopy(existingData, 0, fullData, 0, existingData.length); - System.arraycopy(newData, 0, fullData, existingData.length, newData.length); - } - custom.put("REQUESTBODYDATA", fullData); - } -} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java index 1b5dae2024..a38326a954 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -7,8 +7,10 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Map; /*- * #%L @@ -31,7 +33,9 @@ */ import co.elastic.apm.agent.bci.ElasticApmInstrumentation; -import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.context.TransactionContext; +import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; @@ -49,11 +53,6 @@ public class InputStreamInstrumentation extends ElasticApmInstrumentation { static final String SERVLET_API = "servlet-api"; - @Override - public void init(ElasticApmTracer tracer) { - InputStreamAdvice.init(tracer); - } - @Override public ElementMatcher getTypeMatcherPreFilter() { return nameContains("ServletInputStream"); @@ -71,7 +70,7 @@ public ElementMatcher getMethodMatcher() { @Override public Class getAdviceClass() { - return InputStreamAdvice.class; + return InputStreamInstrumentation.InputStreamAdvice.class; } @Override @@ -79,6 +78,41 @@ public Collection getInstrumentationGroupNames() { return Collections.singleton(SERVLET_API); } + public static class InputStreamAdvice { + @VisibleForAdvice + public static ThreadLocal excluded = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onReadExit(@Advice.Argument(0) Object argument) { + System.out.println("Hello"); + if (argument == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { + return; + } + + System.out.println(new String((byte[]) argument)); + + TransactionContext context = tracer.currentTransaction().getContext(); + Map custom = context.getCustom(); + + byte[] fullData = null; + byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); + byte[] newData = (byte[]) argument; + + if (existingData == null) { + fullData = Arrays.copyOf(newData, newData.length); + } else { + fullData = new byte[existingData.length + newData.length]; + System.arraycopy(existingData, 0, fullData, 0, existingData.length); + System.arraycopy(newData, 0, fullData, existingData.length, newData.length); + } + custom.put("REQUESTBODYDATA", fullData); + } + } } From a3cdc195ecacea7a24f4256893649d19cef85c72 Mon Sep 17 00:00:00 2001 From: d4294 Date: Fri, 25 Jan 2019 11:49:18 +0100 Subject: [PATCH 03/15] Implemented change requests as stated in github comments --- .../servlet/InputStreamInstrumentation.java | 52 ++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java index a38326a954..08198301e1 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -12,6 +12,8 @@ import java.util.Collections; import java.util.Map; +import javax.annotation.Nullable; + /*- * #%L * Elastic APM Java agent @@ -34,6 +36,7 @@ import co.elastic.apm.agent.bci.ElasticApmInstrumentation; import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.context.TransactionContext; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; @@ -42,10 +45,11 @@ import net.bytebuddy.matcher.ElementMatcher; /** - * Instruments servlets to create transactions. + * Instruments the ServletInputStream read method. *

- * If the transaction has already been recorded with the help of {@link FilterChainInstrumentation}, it does not record the transaction again. But if there is - * no filter registered for a servlet, this makes sure to record a transaction in that case. + * Due to the fact that an InputStream can only be read once, the read method of the ServletInputStream has to be instrumented in order to passively read the + * InputStream. Within the read method of the InputStream the content of the request body will be copied to the passed in byte array. Therefore the passed in + * byte array will be copied when the read method exits. The copied data is stored in the transaction context and processed at a later stage. *

*/ public class InputStreamInstrumentation extends ElasticApmInstrumentation { @@ -53,6 +57,11 @@ public class InputStreamInstrumentation extends ElasticApmInstrumentation { static final String SERVLET_API = "servlet-api"; + @Override + public void init(ElasticApmTracer tracer) { + InputStreamAdvice.tracer = tracer; + } + @Override public ElementMatcher getTypeMatcherPreFilter() { return nameContains("ServletInputStream"); @@ -60,7 +69,8 @@ public ElementMatcher getTypeMatcherPreFilter() { @Override public ElementMatcher getTypeMatcher() { - return not(isInterface()).and(hasSuperType(named("java.io.InputStream"))); + // return any(); + return not(isInterface()).and(hasSuperType(named("javax.servlet.ServletInputStream"))); } @Override @@ -70,7 +80,7 @@ public ElementMatcher getMethodMatcher() { @Override public Class getAdviceClass() { - return InputStreamInstrumentation.InputStreamAdvice.class; + return InputStreamAdvice.class; } @Override @@ -78,9 +88,15 @@ public Collection getInstrumentationGroupNames() { return Collections.singleton(SERVLET_API); } + @VisibleForAdvice public static class InputStreamAdvice { + @Nullable + @VisibleForAdvice + public static ElasticApmTracer tracer; + + @Nullable @VisibleForAdvice - public static ThreadLocal excluded = new ThreadLocal() { + public static ThreadLocal reading = new ThreadLocal() { @Override protected Boolean initialValue() { return Boolean.FALSE; @@ -88,31 +104,31 @@ protected Boolean initialValue() { }; @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onReadExit(@Advice.Argument(0) Object argument) { - System.out.println("Hello"); - if (argument == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { + public static void onReadExit(@Advice.Argument(0) byte[] newData, @Advice.Argument(1) int from, @Advice.Argument(2) int to) { + + if (newData == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { return; } - System.out.println(new String((byte[]) argument)); - TransactionContext context = tracer.currentTransaction().getContext(); Map custom = context.getCustom(); + int numberOfBytesToRead = to - from; byte[] fullData = null; byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); - byte[] newData = (byte[]) argument; if (existingData == null) { - fullData = Arrays.copyOf(newData, newData.length); + fullData = Arrays.copyOf(newData, numberOfBytesToRead); } else { - fullData = new byte[existingData.length + newData.length]; - System.arraycopy(existingData, 0, fullData, 0, existingData.length); - System.arraycopy(newData, 0, fullData, existingData.length, newData.length); + try { + fullData = new byte[existingData.length + numberOfBytesToRead]; + System.arraycopy(existingData, 0, fullData, 0, existingData.length); + System.arraycopy(newData, 0, fullData, existingData.length, numberOfBytesToRead); + } catch (Exception e) { + + } } custom.put("REQUESTBODYDATA", fullData); } } - } - From 59e0ed164a69f38cf8e6777212e459836158b331 Mon Sep 17 00:00:00 2001 From: d4294 Date: Fri, 25 Jan 2019 13:31:41 +0100 Subject: [PATCH 04/15] Fixed wrong implementation of onReadExit. Parameters of read method had been interpreted wrong --- .../servlet/InputStreamInstrumentation.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java index 08198301e1..20b6131d7b 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -7,7 +7,6 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -104,7 +103,7 @@ protected Boolean initialValue() { }; @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onReadExit(@Advice.Argument(0) byte[] newData, @Advice.Argument(1) int from, @Advice.Argument(2) int to) { + public static void onReadExit(@Advice.Argument(0) byte[] newData, @Advice.Argument(1) int off, @Advice.Argument(2) int len) { if (newData == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { return; @@ -113,17 +112,18 @@ public static void onReadExit(@Advice.Argument(0) byte[] newData, @Advice.Argume TransactionContext context = tracer.currentTransaction().getContext(); Map custom = context.getCustom(); - int numberOfBytesToRead = to - from; - byte[] fullData = null; + int numberOfBytesToRead = len; byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); + int existingDataLength = (existingData == null) ? 0 : existingData.length; + byte[] fullData = new byte[existingDataLength + numberOfBytesToRead]; + if (existingData == null) { - fullData = Arrays.copyOf(newData, numberOfBytesToRead); + System.arraycopy(newData, off, fullData, existingDataLength, numberOfBytesToRead); } else { try { - fullData = new byte[existingData.length + numberOfBytesToRead]; - System.arraycopy(existingData, 0, fullData, 0, existingData.length); - System.arraycopy(newData, 0, fullData, existingData.length, numberOfBytesToRead); + System.arraycopy(existingData, 0, fullData, 0, existingDataLength); + System.arraycopy(newData, off, fullData, existingDataLength, numberOfBytesToRead); } catch (Exception e) { } From 16a7e9558b042f6426864f49945334a948baedf5 Mon Sep 17 00:00:00 2001 From: d4294 Date: Fri, 25 Jan 2019 13:44:17 +0100 Subject: [PATCH 05/15] Extended InputStreamInstrumentation to be able to instrument ReadLine method as well --- .../elastic/apm/agent/servlet/InputStreamInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java index 20b6131d7b..e14cf96bea 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -74,7 +74,7 @@ public ElementMatcher getTypeMatcher() { @Override public ElementMatcher getMethodMatcher() { - return named("read").and(takesArgument(0, byte[].class)).and(takesArgument(1, int.class)).and(takesArgument(2, int.class)); + return (named("read").or(named("readLine"))).and(takesArgument(0, byte[].class)).and(takesArgument(1, int.class)).and(takesArgument(2, int.class)); } @Override From 010bb4c239f95e447ac6f3308a8a26091bf2b9bd Mon Sep 17 00:00:00 2001 From: d4294 Date: Mon, 28 Jan 2019 11:16:05 +0100 Subject: [PATCH 06/15] Continued to implement comments of felixbarny --- .../servlet/InputStreamInstrumentation.java | 73 +++++++++++-------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java index e14cf96bea..a47dc16e5b 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -37,6 +37,7 @@ import co.elastic.apm.agent.bci.VisibleForAdvice; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.context.TransactionContext; +import co.elastic.apm.agent.impl.transaction.Transaction; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -44,11 +45,12 @@ import net.bytebuddy.matcher.ElementMatcher; /** - * Instruments the ServletInputStream read method. + * Instruments the ServletInputStream read and readLine method. *

- * Due to the fact that an InputStream can only be read once, the read method of the ServletInputStream has to be instrumented in order to passively read the - * InputStream. Within the read method of the InputStream the content of the request body will be copied to the passed in byte array. Therefore the passed in - * byte array will be copied when the read method exits. The copied data is stored in the transaction context and processed at a later stage. + * Due to the fact that an InputStream can only be read once, the read and readLine method of the ServletInputStream has to be instrumented in order to + * passively read the InputStream. Within the afore mentioned methods the content of the request body will be copied to the byte array that is passed in to + * these methods as an argument. Therefore the passed in byte array will be copied when the read method exits. The copied data is stored in the transaction + * context and processed at a later stage. *

*/ public class InputStreamInstrumentation extends ElasticApmInstrumentation { @@ -63,12 +65,11 @@ public void init(ElasticApmTracer tracer) { @Override public ElementMatcher getTypeMatcherPreFilter() { - return nameContains("ServletInputStream"); + return nameContains("Stream"); } @Override public ElementMatcher getTypeMatcher() { - // return any(); return not(isInterface()).and(hasSuperType(named("javax.servlet.ServletInputStream"))); } @@ -89,11 +90,11 @@ public Collection getInstrumentationGroupNames() { @VisibleForAdvice public static class InputStreamAdvice { + @Nullable @VisibleForAdvice public static ElasticApmTracer tracer; - @Nullable @VisibleForAdvice public static ThreadLocal reading = new ThreadLocal() { @Override @@ -102,33 +103,45 @@ protected Boolean initialValue() { } }; - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onReadExit(@Advice.Argument(0) byte[] newData, @Advice.Argument(1) int off, @Advice.Argument(2) int len) { - - if (newData == null || tracer == null || tracer.currentTransaction() == null || tracer.currentTransaction().getContext() == null) { - return; - } - - TransactionContext context = tracer.currentTransaction().getContext(); - Map custom = context.getCustom(); - - int numberOfBytesToRead = len; - byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); - int existingDataLength = (existingData == null) ? 0 : existingData.length; - byte[] fullData = new byte[existingDataLength + numberOfBytesToRead]; - + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onReadEnter(@Advice.This Object thiz, + @Advice.Local("transaction") Transaction transaction, + @Advice.Local("alreadyReading") boolean alreadyReading) { + alreadyReading = reading.get(); + reading.set(Boolean.TRUE); + } + - if (existingData == null) { - System.arraycopy(newData, off, fullData, existingDataLength, numberOfBytesToRead); - } else { - try { + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onReadExit(@Advice.Argument(0) byte[] newData, + @Advice.Argument(1) int off, + @Advice.Argument(2) int len, + @Nullable @Advice.Thrown Throwable t, + @Advice.Local("alreadyReading") boolean alreadyReading) { + try { + if (alreadyReading || newData == null || tracer == null || tracer.currentTransaction() == null || t != null) { + return; + } + + TransactionContext context = tracer.currentTransaction().getContext(); + Map custom = context.getCustom(); + + + byte[] existingData = (byte[]) custom.get("REQUESTBODYDATA"); + int existingDataLength = (existingData == null) ? 0 : existingData.length; + byte[] fullData = new byte[existingDataLength + len]; + + if (existingData == null) { + System.arraycopy(newData, off, fullData, existingDataLength, len); + } else { System.arraycopy(existingData, 0, fullData, 0, existingDataLength); - System.arraycopy(newData, off, fullData, existingDataLength, numberOfBytesToRead); - } catch (Exception e) { - + System.arraycopy(newData, off, fullData, existingDataLength, len); } + + custom.put("REQUESTBODYDATA", fullData); + } finally{ + reading.set(Boolean.FALSE); } - custom.put("REQUESTBODYDATA", fullData); } } } From 1da9c25712bab5bd9ba7d1af83e0cd6d9eec3a84 Mon Sep 17 00:00:00 2001 From: d4294 Date: Wed, 6 Feb 2019 10:40:15 +0100 Subject: [PATCH 07/15] Use prepared body request capture infrastructure --- .../servlet/InputStreamInstrumentation.java | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java new file mode 100644 index 0000000000..cd968eafcd --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java @@ -0,0 +1,133 @@ +package co.elastic.apm.agent.servlet; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import java.util.Collection; +import java.util.Collections; + +import javax.annotation.Nullable; + +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 Elastic and contributors + * %% + * 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. + * #L% + */ + +import co.elastic.apm.agent.bci.ElasticApmInstrumentation; +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.util.IOUtils; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.NamedElement; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Instruments the ServletInputStream read and readLine method. + *

+ * Due to the fact that an InputStream can only be read once, the read and readLine method of the ServletInputStream has to be instrumented in order to + * passively read the InputStream. Within the afore mentioned methods the content of the request body will be copied to the byte array that is passed in to + * these methods as an argument. Therefore the passed in byte array will be copied when the read method exits. The copied data is stored in the transaction + * context and processed at a later stage. + *

+ */ +public class InputStreamInstrumentation extends ElasticApmInstrumentation { + + + static final String SERVLET_API = "servlet-api"; + + @Override + public void init(ElasticApmTracer tracer) { + InputStreamAdvice.tracer = tracer; + } + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameContains("Stream"); + } + + @Override + public ElementMatcher getTypeMatcher() { + return not(isInterface()).and(hasSuperType(named("javax.servlet.ServletInputStream"))); + } + + @Override + public ElementMatcher getMethodMatcher() { + return (named("read").or(named("readLine"))).and(takesArgument(0, byte[].class)).and(takesArgument(1, int.class)).and(takesArgument(2, int.class)); + } + + @Override + public Class getAdviceClass() { + return InputStreamAdvice.class; + } + + @Override + public Collection getInstrumentationGroupNames() { + return Collections.singleton(SERVLET_API); + } + + @VisibleForAdvice + public static class InputStreamAdvice { + + @Nullable + @VisibleForAdvice + public static ElasticApmTracer tracer; + + @VisibleForAdvice + public static ThreadLocal reading = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onReadEnter(@Advice.This Object thiz, @Advice.Local("transaction") Transaction transaction, + @Advice.Local("alreadyReading") boolean alreadyReading) { + alreadyReading = reading.get(); + reading.set(Boolean.TRUE); + } + + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onReadExit(@Advice.Argument(0) byte[] bytes, @Advice.Argument(1) int offset, @Advice.Argument(2) int length, + @Nullable @Advice.Thrown Throwable t, @Advice.Local("alreadyReading") boolean alreadyReading) { + try { + if (alreadyReading || + bytes == null || + tracer == null || + tracer.currentTransaction() == null || + tracer.currentTransaction().getContext() == null || + tracer.currentTransaction().getContext().getRequest() == null || + t != null ) { + return; + } + + IOUtils.decodeUtf8Bytes(bytes, offset, length, tracer.currentTransaction().getContext().getRequest().withBodyBuffer()); + } finally { + reading.set(Boolean.FALSE); + } + } + } +} From 643dc3cf93b35476e1895142e092bcec16840df6 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 12 Feb 2019 17:59:04 +0100 Subject: [PATCH 08/15] Wrap ServletInputStream The advantage of this approach is that we don't have to instrument java.io.InputStream which would affect much more than just the HttpServletRequest body. We only create a wrapper in case we want to capture the body so the effect on requests with disabled body capturing is minimal. --- .../apm/agent/impl/context/Request.java | 49 +++- .../impl/context/TransactionContext.java | 4 + .../agent/impl/transaction/Transaction.java | 1 + .../apm/agent/matcher/WildcardMatcher.java | 17 +- .../report/serialize/DslJsonSerializer.java | 13 +- .../servlet/InputStreamInstrumentation.java | 132 ---------- ...RequestStreamRecordingInstrumentation.java | 125 ++++++++++ .../apm/agent/servlet/ServletApiAdvice.java | 2 +- .../servlet/ServletTransactionHelper.java | 57 +++-- .../helper/InputStreamFactoryHelperImpl.java | 32 +++ .../RecordingServletInputStreamWrapper.java | 197 +++++++++++++++ ...ic.apm.agent.bci.ElasticApmInstrumentation | 2 +- .../servlet/TestRequestBodyCapturing.java | 230 ++++++++++++++++++ ...stractServletContainerIntegrationTest.java | 7 +- .../apm/servlet/ServletApiTestApp.java | 19 ++ .../co/elastic/apm/servlet/WebSphereIT.java | 5 - .../java/co/elastic/webapp/EchoServlet.java | 38 +++ .../src/main/webapp/WEB-INF/web.xml | 10 + 18 files changed, 768 insertions(+), 172 deletions(-) delete mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java create mode 100644 apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java create mode 100644 integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java index 72d20481c5..1717cbff2a 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java @@ -71,6 +71,11 @@ public void recycle(CharBuffer object) { * A parsed key-value object of cookies */ private final PotentiallyMultiValuedMap cookies = new PotentiallyMultiValuedMap(); + /** + * Data should only contain the request body (not the query string). It can either be a dictionary (for standard HTTP requests) or a raw request body. + */ + @Nullable + private String rawBody; /** * HTTP version. */ @@ -84,6 +89,7 @@ public void recycle(CharBuffer object) { private String method; @Nullable private CharBuffer bodyBuffer; + private boolean bodyBufferFinished = false; /** * Data should only contain the request body (not the query string). It can either be a dictionary (for standard HTTP requests) or a raw request body. @@ -92,16 +98,29 @@ public void recycle(CharBuffer object) { public Object getBody() { if (!postParams.isEmpty()) { return postParams; + } else if (rawBody != null) { + return rawBody; } else { return bodyBuffer; } } - public void redactBody() { + @Nullable + public String getRawBody() { + return rawBody; + } + + public void setRawBody(String rawBody) { postParams.resetState(); if (bodyBuffer != null) { - bodyBuffer.clear().append("[REDACTED]").flip(); + charBufferPool.recycle(bodyBuffer); + bodyBuffer = null; } + this.rawBody = rawBody; + } + + public void redactBody() { + setRawBody("[REDACTED]"); } public Request addFormUrlEncodedParameter(String key, String value) { @@ -132,6 +151,13 @@ public CharBuffer withBodyBuffer() { return this.bodyBuffer; } + public void endOfBufferInput() { + if (bodyBuffer != null && !bodyBufferFinished) { + bodyBufferFinished = true; + ((Buffer) bodyBuffer).flip(); + } + } + /** * Returns the associated pooled {@link CharBuffer} to record the request body. *

@@ -142,6 +168,15 @@ public CharBuffer withBodyBuffer() { */ @Nullable public CharBuffer getBodyBuffer() { + if (!bodyBufferFinished) { + return bodyBuffer; + } else { + return null; + } + } + + @Nullable + public CharBuffer getBodyBufferForSerialization() { return bodyBuffer; } @@ -231,6 +266,10 @@ public PotentiallyMultiValuedMap getCookies() { return cookies; } + void onTransactionEnd() { + endOfBufferInput(); + } + @Override public void resetState() { postParams.resetState(); @@ -242,8 +281,8 @@ public void resetState() { cookies.resetState(); if (bodyBuffer != null) { charBufferPool.recycle(bodyBuffer); + bodyBuffer = null; } - bodyBuffer = null; } public void copyFrom(Request other) { @@ -255,12 +294,12 @@ public void copyFrom(Request other) { this.url.copyFrom(other.url); this.cookies.copyFrom(other.cookies); if (other.bodyBuffer != null) { - final CharBuffer otherBuffer = other.getBodyBuffer(); + final CharBuffer otherBuffer = other.bodyBuffer; final CharBuffer thisBuffer = this.withBodyBuffer(); for (int i = 0; i < otherBuffer.length(); i++) { thisBuffer.append(otherBuffer.charAt(i)); } - thisBuffer.flip(); + ((Buffer) thisBuffer).flip(); } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java index 0d6a6d1a92..bfd80f8b11 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/TransactionContext.java @@ -91,4 +91,8 @@ public void resetState() { request.resetState(); user.resetState(); } + + public void onTransactionEnd() { + request.onTransactionEnd(); + } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java index 7a2cc84bd7..a4be336b8b 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java @@ -161,6 +161,7 @@ public void doEnd(long epochMicros) { if (type == null) { type = "custom"; } + context.onTransactionEnd(); this.tracer.endTransaction(this); } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java index 6198ebd045..1d8ced99e1 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java @@ -136,7 +136,22 @@ public static WildcardMatcher valueOf(final String wildcardString) { * @return {@code true}, if any of the matchers match the provided string */ @Nullable - public static WildcardMatcher anyMatch(List matchers, String s) { + public static boolean isAnyMatch(List matchers, @Nullable String s) { + return anyMatch(matchers, s) != null; + } + + /** + * Returns {@code true}, if any of the matchers match the provided string. + * + * @param matchers the matchers which should be used to match the provided string + * @param s the string to match against + * @return {@code true}, if any of the matchers match the provided string + */ + @Nullable + public static WildcardMatcher anyMatch(List matchers, @Nullable String s) { + if (s == null) { + return null; + } return anyMatch(matchers, s, null); } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java index 909e94f617..d99ba0896d 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java @@ -798,10 +798,15 @@ private void serializeRequest(final Request request) { // only one of those can be non-empty if (!request.getFormUrlEncodedParameters().isEmpty()) { writeField("body", request.getFormUrlEncodedParameters()); - } else if (request.getBodyBuffer() != null && request.getBodyBuffer().length() > 0) { - writeFieldName("body"); - jw.writeString(request.getBodyBuffer()); - jw.writeByte(COMMA); + } else if (request.getRawBody() != null) { + writeField("body", request.getRawBody()); + } else { + final CharBuffer bodyBuffer = request.getBodyBufferForSerialization(); + if (bodyBuffer != null && bodyBuffer.length() > 0) { + writeFieldName("body"); + jw.writeString(bodyBuffer); + jw.writeByte(COMMA); + } } if (request.getUrl().hasContent()) { serializeUrl(request.getUrl()); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java deleted file mode 100644 index 960c2aa3bc..0000000000 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InputStreamInstrumentation.java +++ /dev/null @@ -1,132 +0,0 @@ -package co.elastic.apm.agent.servlet; - -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; -import static net.bytebuddy.matcher.ElementMatchers.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.nameContains; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import java.util.Collection; -import java.util.Collections; - -import javax.annotation.Nullable; - -/*- - * #%L - * Elastic APM Java agent - * %% - * Copyright (C) 2018 Elastic and contributors - * %% - * 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. - * #L% - */ - -import co.elastic.apm.agent.bci.ElasticApmInstrumentation; -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.transaction.Transaction; -import co.elastic.apm.agent.util.IOUtils; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.NamedElement; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -/** - * Instruments the ServletInputStream read and readLine method. - *

- * Due to the fact that an InputStream can only be read once, the read and readLine method of the ServletInputStream has to be instrumented in order to - * passively read the InputStream. Within the afore mentioned methods the content of the request body will be copied to the byte array that is passed in to - * these methods as an argument. Therefore the passed in byte array will be copied when the read method exits. The copied data is stored in the transaction - * context and processed at a later stage. - *

- */ -public class InputStreamInstrumentation extends ElasticApmInstrumentation { - - - static final String SERVLET_API = "servlet-api"; - - @Override - public void init(ElasticApmTracer tracer) { - InputStreamAdvice.tracer = tracer; - } - - @Override - public ElementMatcher getTypeMatcherPreFilter() { - return nameContains("Stream"); - } - - @Override - public ElementMatcher getTypeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.ServletInputStream"))); - } - - @Override - public ElementMatcher getMethodMatcher() { - return (named("read").or(named("readLine"))).and(takesArgument(0, byte[].class)).and(takesArgument(1, int.class)).and(takesArgument(2, int.class)); - } - - @Override - public Class getAdviceClass() { - return InputStreamAdvice.class; - } - - @Override - public Collection getInstrumentationGroupNames() { - return Collections.singleton(SERVLET_API); - } - - @VisibleForAdvice - public static class InputStreamAdvice { - @Nullable - @VisibleForAdvice - public static ElasticApmTracer tracer; - - @VisibleForAdvice - public static ThreadLocal reading = new ThreadLocal() { - @Override - protected Boolean initialValue() { - return Boolean.FALSE; - } - }; - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onReadEnter(@Advice.This Object thiz, @Advice.Local("transaction") Transaction transaction, - @Advice.Local("alreadyReading") boolean alreadyReading) { - alreadyReading = reading.get(); - reading.set(Boolean.TRUE); - } - - - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onReadExit(@Advice.Argument(0) byte[] bytes, @Advice.Argument(1) int offset, @Advice.Argument(2) int length, - @Nullable @Advice.Thrown Throwable t, @Advice.Local("alreadyReading") boolean alreadyReading) { - try { - if (alreadyReading || - bytes == null || - tracer == null || - tracer.currentTransaction() == null || - tracer.currentTransaction().getContext() == null || - tracer.currentTransaction().getContext().getRequest() == null || - t != null ) { - return; - } - - IOUtils.decodeUtf8Bytes(bytes, offset, length, tracer.currentTransaction().getContext().getRequest().withBodyBuffer()); - } finally { - reading.set(Boolean.FALSE); - } - } - } -} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java new file mode 100644 index 0000000000..e4ad17294a --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java @@ -0,0 +1,125 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.servlet; + +import co.elastic.apm.agent.bci.ElasticApmInstrumentation; +import co.elastic.apm.agent.bci.HelperClassManager; +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.impl.transaction.Transaction; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.NamedElement; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +import javax.annotation.Nullable; +import javax.servlet.ServletInputStream; +import java.util.Arrays; +import java.util.Collection; + +import static co.elastic.apm.agent.servlet.ServletInstrumentation.SERVLET_API; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +public class RequestStreamRecordingInstrumentation extends ElasticApmInstrumentation { + + @Nullable + @VisibleForAdvice + // referring to AsyncContext is legal because of type erasure + public static HelperClassManager wrapperHelperClassManager; + + @Override + public void init(ElasticApmTracer tracer) { + wrapperHelperClassManager = HelperClassManager.ForSingleClassLoader.of(tracer, + "co.elastic.apm.agent.servlet.helper.InputStreamFactoryHelperImpl", + "co.elastic.apm.agent.servlet.helper.RecordingServletInputStreamWrapper"); + } + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameContains("Request"); + } + + @Override + public ElementMatcher getTypeMatcher() { + return hasSuperType(named("javax.servlet.ServletRequest")).and(not(isInterface())); + } + + @Override + public ElementMatcher getMethodMatcher() { + return named("getInputStream").and(returns(named("javax.servlet.ServletInputStream"))); + } + + @Override + public Collection getInstrumentationGroupNames() { + return Arrays.asList(SERVLET_API, "servlet-input-stream"); + } + + @Override + public Class getAdviceClass() { + return GetInputStreamAdvice.class; + } + + public interface InputStreamWrapperFactory { + ServletInputStream wrap(Request request, ServletInputStream servletInputStream); + } + + public static class GetInputStreamAdvice { + + @VisibleForAdvice + public static final ThreadLocal nestedThreadLocal = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onReadEnter(@Advice.This Object thiz, + @Advice.Local("transaction") Transaction transaction, + @Advice.Local("nested") boolean nested) { + nested = nestedThreadLocal.get(); + nestedThreadLocal.set(Boolean.TRUE); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void afterGetInputStream(@Advice.Return(readOnly = false) ServletInputStream inputStream, + @Advice.Local("nested") boolean nested) { + try { + if (nested || tracer == null || wrapperHelperClassManager == null) { + return; + } + final Transaction transaction = tracer.currentTransaction(); + // only wrap if the body buffer has been initialized via ServletTransactionHelper.startCaptureBody + if (transaction != null && transaction.getContext().getRequest().getBodyBuffer() != null) { + inputStream = wrapperHelperClassManager.getForClassLoaderOfClass(inputStream.getClass()).wrap(transaction.getContext().getRequest(), inputStream); + } + } finally { + nestedThreadLocal.set(Boolean.FALSE); + } + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index c08e2a2fd9..dfe6898c18 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -116,7 +116,7 @@ public static void onEnterServletService(@Advice.Argument(0) ServletRequest serv servletTransactionHelper.fillRequestContext(transaction, request.getProtocol(), request.getMethod(), request.isSecure(), request.getScheme(), request.getServerName(), request.getServerPort(), request.getRequestURI(), request.getQueryString(), - request.getRemoteAddr()); + request.getRemoteAddr(), request.getHeader("Content-Type")); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index 0941c8f962..65e7d64687 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -19,19 +19,6 @@ */ package co.elastic.apm.agent.servlet; -import static co.elastic.apm.agent.web.WebConfiguration.EventType.OFF; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.Map; -import java.util.Objects; -import java.util.Set; - -import javax.annotation.Nullable; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import co.elastic.apm.agent.bci.VisibleForAdvice; import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; @@ -45,6 +32,18 @@ import co.elastic.apm.agent.web.ClientIpUtils; import co.elastic.apm.agent.web.ResultUtil; import co.elastic.apm.agent.web.WebConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import static co.elastic.apm.agent.web.WebConfiguration.EventType.OFF; /** * This class must not import classes from {@code javax.servlet} due to class loader issues. @@ -64,6 +63,12 @@ public class ServletTransactionHelper { private final ElasticApmTracer tracer; private final CoreConfiguration coreConfiguration; private final WebConfiguration webConfiguration; + // TODO make configurable + private List recordedContentTypes = Arrays.asList( + WildcardMatcher.valueOf("text/*"), + WildcardMatcher.valueOf("application/json"), + WildcardMatcher.valueOf("application/xml") + ); @VisibleForAdvice public ServletTransactionHelper(ElasticApmTracer tracer) { @@ -113,12 +118,25 @@ public Transaction onBefore(String servletPath, @Nullable String pathInfo, @VisibleForAdvice public void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, String scheme, String serverName, int serverPort, String requestURI, String queryString, - String remoteAddr) { + String remoteAddr, @Nullable String contentTypeHeader) { final Request request = transaction.getContext().getRequest(); + startCaptureBody(transaction, method, contentTypeHeader); fillRequest(request, protocol, method, secure, scheme, serverName, serverPort, requestURI, queryString, remoteAddr); } + + private void startCaptureBody(Transaction transaction, String method, @Nullable String contentTypeHeader) { + Request request = transaction.getContext().getRequest(); + if (hasBody(contentTypeHeader, method)) { + if (webConfiguration.getCaptureBody() != OFF && WildcardMatcher.isAnyMatch(recordedContentTypes, contentTypeHeader)) { + request.withBodyBuffer(); + } else { + request.redactBody(); + } + } + } + @VisibleForAdvice public static void setUsernameIfUnset(@Nullable String userName, TransactionContext context) { // only set username if not manually set @@ -184,9 +202,7 @@ private void fillRequestParameters(Transaction transaction, String method, @Null Request request = transaction.getContext().getRequest(); if (hasBody(contentTypeHeader, method)) { if (webConfiguration.getCaptureBody() != OFF && parameterMap != null) { - captureBody(request, parameterMap, contentTypeHeader); - } else { - request.redactBody(); + captureParameters(request, parameterMap, contentTypeHeader); } } } @@ -244,14 +260,11 @@ private boolean hasBody(@Nullable String contentTypeHeader, String method) { return METHODS_WITH_BODY.contains(method) && contentTypeHeader != null; } - private void captureBody(Request request, Map params, @Nullable String contentTypeHeader) { + private void captureParameters(Request request, Map params, @Nullable String contentTypeHeader) { if (contentTypeHeader != null && contentTypeHeader.startsWith("application/x-www-form-urlencoded")) { for (Map.Entry param : params.entrySet()) { request.addFormUrlEncodedParameters(param.getKey(), param.getValue()); } - } else { - // this content-type is not supported (yet) - request.redactBody(); } } @@ -329,4 +342,4 @@ public static void setTransactionNameByServletClass(String method, @Nullable Cla public boolean isCaptureHeaders() { return webConfiguration.isCaptureHeaders(); } -} \ No newline at end of file +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java new file mode 100644 index 0000000000..170074997c --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java @@ -0,0 +1,32 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.servlet.helper; + +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation; + +import javax.servlet.ServletInputStream; + +public class InputStreamFactoryHelperImpl implements RequestStreamRecordingInstrumentation.InputStreamWrapperFactory { + @Override + public ServletInputStream wrap(Request request, ServletInputStream servletInputStream) { + return new RecordingServletInputStreamWrapper(request, servletInputStream); + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java new file mode 100644 index 0000000000..4311179960 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java @@ -0,0 +1,197 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.servlet.helper; + +import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.util.IOUtils; + +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import java.io.IOException; +import java.nio.CharBuffer; +import java.nio.charset.CoderResult; + +@VisibleForAdvice +public class RecordingServletInputStreamWrapper extends ServletInputStream { + + private final Request request; + private final ServletInputStream servletInputStream; + + @VisibleForAdvice + public RecordingServletInputStreamWrapper(Request request, ServletInputStream servletInputStream) { + this.request = request; + this.servletInputStream = servletInputStream; + } + + @Override + public int read() throws IOException { + try { + final int b = servletInputStream.read(); + decode(b); + return b; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public int read(byte[] b) throws IOException { + try { + final int read; + read = servletInputStream.read(b); + decode(b, 0, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + try { + final int read = servletInputStream.read(b, off, len); + decode(b, off, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + // not @Override'ing in order to be able to compile against Java 7 + public int readNBytes(byte[] b, int off, int len) throws IOException { + try { + final int read = servletInputStream.readNBytes(b, off, len); + decode(b, off, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public int readLine(byte[] b, int off, int len) throws IOException { + try { + final int read = servletInputStream.readLine(b, off, len); + decode(b, off, read); + return read; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public byte[] readAllBytes() throws IOException { + try { + final byte[] bytes = servletInputStream.readAllBytes(); + decode(bytes, 0, bytes.length); + return bytes; + } catch (IOException e) { + request.endOfBufferInput(); + throw e; + } + } + + @Override + public void close() throws IOException { + try { + servletInputStream.close(); + } finally { + request.endOfBufferInput(); + } + } + + @Override + public long skip(long n) throws IOException { + return servletInputStream.skip(n); + } + + @Override + public int available() throws IOException { + return servletInputStream.available(); + } + + @Override + public void mark(int readlimit) { + servletInputStream.mark(readlimit); + } + + @Override + public void reset() throws IOException { + servletInputStream.reset(); + } + + @Override + public boolean markSupported() { + return servletInputStream.markSupported(); + } + + @Override + public boolean isFinished() { + return servletInputStream.isFinished(); + } + + @Override + public boolean isReady() { + return servletInputStream.isReady(); + } + + @Override + public void setReadListener(ReadListener readListener) { + servletInputStream.setReadListener(readListener); + } + + private void decode(byte[] b, int off, int read) { + if (read == -1) { + request.endOfBufferInput(); + } else { + final CharBuffer bodyBuffer = request.getBodyBuffer(); + if (bodyBuffer != null) { + final CoderResult coderResult = IOUtils.decodeUtf8Bytes(b, off, read, bodyBuffer); + handleCoderResult(coderResult); + } + } + } + + private void decode(int b) { + if (b == -1) { + request.endOfBufferInput(); + } else { + final CharBuffer bodyBuffer = request.getBodyBuffer(); + if (bodyBuffer != null) { + final CoderResult coderResult = IOUtils.decodeUtf8Byte((byte) b, bodyBuffer); + handleCoderResult(coderResult); + } + } + } + + private void handleCoderResult(CoderResult coderResult) { + if (coderResult.isError()) { + request.setRawBody("[Non UTF-8 data]"); + } else if (coderResult.isOverflow()) { + request.endOfBufferInput(); + } + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation index 2a278185cf..dfc00a3a7b 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation @@ -2,4 +2,4 @@ co.elastic.apm.agent.servlet.ServletInstrumentation co.elastic.apm.agent.servlet.FilterChainInstrumentation co.elastic.apm.agent.servlet.AsyncInstrumentation$StartAsyncInstrumentation co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation -co.elastic.apm.agent.servlet.InputStreamInstrumentation \ No newline at end of file +co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java new file mode 100644 index 0000000000..576e41e425 --- /dev/null +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -0,0 +1,230 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.apm.agent.servlet; + +import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.impl.error.ErrorCapture; +import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.report.serialize.DslJsonSerializer; +import co.elastic.apm.agent.web.WebConfiguration; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.mock.web.MockFilterChain; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import javax.annotation.Nonnull; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.stream.Stream; + +import static org.assertj.core.api.Java6Assertions.assertThat; +import static org.mockito.Mockito.when; + +class TestRequestBodyCapturing extends AbstractInstrumentationTest { + + private static final byte[] BUFFER = new byte[1024]; + private InputStreamConsumer streamConsumer; + private InputStreamCloser streamCloser; + private WebConfiguration webConfiguration; + private MockFilterChain filterChain; + + static Stream streamConsumers() { + return Stream.of( + InputStream::read, + is -> is.read(BUFFER), + is -> is.read(BUFFER, 0, BUFFER.length), + is -> is.readLine(BUFFER, 0, BUFFER.length), + is -> { + is.readNBytes(BUFFER, 0, BUFFER.length); + return -1; + }, + is -> { + is.readAllBytes(); + return -1; + } + ); + } + + @BeforeEach + void setUp() { + webConfiguration = tracer.getConfig(WebConfiguration.class); + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); + filterChain = new MockFilterChain(new HttpServlet() { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + final ServletInputStream is = req.getInputStream(); + int read; + do { + read = streamConsumer.read(is); + } while (read != -1); + streamCloser.close(is); + } + }); + streamConsumer = is -> is.readLine(BUFFER, 0, BUFFER.length); + streamCloser = InputStream::close; + + } + + @ParameterizedTest + @MethodSource("streamConsumers") + void testReadTextPlain(InputStreamConsumer consumer) throws Exception { + streamConsumer = consumer; + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("foo"); + } + + @Test + void testCaptureBodyOff() throws Exception { + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.OFF); + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("[REDACTED]"); + } + + @ParameterizedTest + @ValueSource(strings = {"ALL", "TRANSACTIONS", "ERRORS"}) + void testCaptureBodyNotOff(WebConfiguration.EventType eventType) throws Exception { + streamCloser = is -> { throw new RuntimeException(); }; + + when(webConfiguration.getCaptureBody()).thenReturn(eventType); + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Transaction transaction = reporter.getFirstTransaction(); + final Object body = transaction.getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + // this is not [REDACTED] in this test as the BodyProcessor is not active in MockReporter + assertThat(body.toString()).isEqualTo("foo"); + + final ErrorCapture error = reporter.getFirstError(); + assertThat(error).isNotNull(); + assertThat(error.getContext().getRequest().getBody().toString()).isEqualTo("foo"); + } + + @Test + void testReadWithoutClose() throws Exception { + streamCloser = is -> {}; + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("foo"); + } + + @Test + void testReadLongText() throws Exception { + final byte[] longBody = RandomStringUtils.randomAlphanumeric(DslJsonSerializer.MAX_LONG_STRING_VALUE_LENGTH * 2).getBytes(StandardCharsets.UTF_8); + executeRequest(filterChain, longBody, "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString().length()).isEqualTo(DslJsonSerializer.MAX_LONG_STRING_VALUE_LENGTH); + } + + @Test + void testReadTextPlainNonUtf8() throws Exception { + executeRequest(filterChain, "foo{}".getBytes(StandardCharsets.UTF_16), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("[Non UTF-8 data]"); + } + + @Test + void testReadUnknownContentType() throws Exception { + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "application/unknown"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo("[REDACTED]"); + } + + @Test + void testNotReading() throws Exception { + MockFilterChain filterChain = new MockFilterChain(new HttpServlet() { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) { + } + }); + + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo(""); + } + + @Test + void testReadOtherStream() throws Exception { + MockFilterChain filterChain = new MockFilterChain(new HttpServlet() { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + new ByteArrayInputStream("foo".getBytes(StandardCharsets.UTF_8)).read(new byte[42]); + } + }); + + executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + + final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(body).isNotNull(); + assertThat(body.toString()).isEqualTo(""); + } + + private void executeRequest(MockFilterChain filterChain, byte[] bytes, String contentType) throws IOException, ServletException { + try { + filterChain.doFilter(createMockRequest(bytes, contentType), new MockHttpServletResponse()); + } catch (Exception e) { + e.printStackTrace(); + } + } + + @Nonnull + private HttpServletRequest createMockRequest(byte[] bytes, String contentType) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo"); + request.setContent(bytes); + request.setContentType(contentType); + return new HttpServletRequestWrapper(request); + } + + interface InputStreamConsumer { + int read(ServletInputStream inputStream) throws IOException; + } + + interface InputStreamCloser { + void close(ServletInputStream inputStream) throws IOException; + } +} diff --git a/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java b/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java index 62fa398c3a..85917d6d2f 100644 --- a/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java +++ b/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java @@ -120,6 +120,7 @@ protected AbstractServletContainerIntegrationTest(GenericContainer servletCon .withEnv("ELASTIC_APM_IGNORE_URLS", "/status*,/favicon.ico") .withEnv("ELASTIC_APM_REPORT_SYNC", "true") .withEnv("ELASTIC_APM_LOGGING_LOG_LEVEL", "DEBUG") + .withEnv("ELASTIC_APM_CAPTURE_BODY", "all") .withLogConsumer(new StandardOutLogConsumer().withPrefix(containerName)) .withExposedPorts(webPort) .withFileSystemBind(pathToJavaagent, "/elastic-apm-agent.jar"); @@ -314,7 +315,7 @@ protected boolean isExpectedStacktrace(String path) { return !path.equals("/async-start-servlet"); } - private String getBaseUrl() { + public String getBaseUrl() { return "http://" + servletContainer.getContainerIpAddress() + ":" + servletContainer.getMappedPort(webPort); } @@ -388,4 +389,8 @@ private void waitFor(String path) { .withStartupTimeout(Duration.ofMinutes(ENABLE_DEBUGGING ? 1_000 : 5)) .waitUntilReady(servletContainer); } + + public OkHttpClient getHttpClient() { + return httpClient; + } } diff --git a/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/ServletApiTestApp.java b/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/ServletApiTestApp.java index bc32d881e4..2e6e3b64c5 100644 --- a/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/ServletApiTestApp.java +++ b/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/ServletApiTestApp.java @@ -20,6 +20,10 @@ package co.elastic.apm.servlet; import com.fasterxml.jackson.databind.JsonNode; +import okhttp3.MediaType; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; import java.io.IOException; import java.util.List; @@ -39,6 +43,21 @@ void test(AbstractServletContainerIntegrationTest test) throws Exception { testSpanErrorReporting(test); testExecutorService(test); testHttpUrlConnection(test); + testCaptureBody(test); + } + + private void testCaptureBody(AbstractServletContainerIntegrationTest test) throws Exception { + test.clearMockServerLog(); + final Response response = test.getHttpClient().newCall(new Request.Builder() + .post(RequestBody.create(MediaType.parse("text/plain"), "{foo}")) + .url(test.getBaseUrl() + "/simple-webapp/echo") + .build()) + .execute(); + assertThat(response.code()).isEqualTo(200); + assertThat(response.body().string()).isEqualTo("{foo}"); + + final JsonNode transaction = test.assertTransactionReported("/simple-webapp/echo", 200); + assertThat(transaction.get("context").get("request").get("body").textValue()).isEqualTo("{foo}"); } private void testExecutorService(AbstractServletContainerIntegrationTest test) throws Exception { diff --git a/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/WebSphereIT.java b/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/WebSphereIT.java index 85b565ae5e..9f880e1c02 100644 --- a/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/WebSphereIT.java +++ b/integration-tests/simple-webapp-integration-test/src/test/java/co/elastic/apm/servlet/WebSphereIT.java @@ -48,11 +48,6 @@ public static Iterable data() { return Arrays.asList(new Object[][]{{"8.5.5"}, {"webProfile7"}}); } - @Override - protected void enableDebugging(GenericContainer servletContainer) { - servletContainer.withEnv("JVM_ARGS", "-javaagent:/elastic-apm-agent.jar -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"); - } - @Override protected boolean isExpectedStacktrace(String path) { return true; diff --git a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java new file mode 100644 index 0000000000..92dad12f0c --- /dev/null +++ b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java @@ -0,0 +1,38 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * 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. + * #L% + */ +package co.elastic.webapp; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +public class EchoServlet extends HttpServlet { + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { + byte[] buffer = new byte[1024]; + int read; + while ((read = req.getInputStream().read(buffer)) > 0) { + resp.getOutputStream().write(buffer, 0, read); + } + resp.setContentType(req.getContentType()); + } +} diff --git a/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml b/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml index af0430b321..446ab6850a 100644 --- a/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml +++ b/integration-tests/simple-webapp/src/main/webapp/WEB-INF/web.xml @@ -60,4 +60,14 @@ /http-url-connection + + EchoServlet + co.elastic.webapp.EchoServlet + 1 + + + EchoServlet + /echo + + From 00b3544d56b83f8c8a909f5095a0c4519d85d57f Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 21 Feb 2019 10:17:54 +0100 Subject: [PATCH 09/15] Make capture_body content types configurable --- .../servlet/ServletTransactionHelper.java | 22 ++++++------- .../apm/agent/servlet/ApmFilterTest.java | 15 --------- .../servlet/TestRequestBodyCapturing.java | 33 +++++++++++++++++++ .../apm/agent/web/WebConfiguration.java | 32 +++++++++++++++--- 4 files changed, 72 insertions(+), 30 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index 65e7d64687..5b65e84342 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -38,7 +38,6 @@ import javax.annotation.Nullable; import java.util.Arrays; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -56,6 +55,7 @@ public class ServletTransactionHelper { public static final String TRANSACTION_ATTRIBUTE = ServletApiAdvice.class.getName() + ".transaction"; @VisibleForAdvice public static final String ASYNC_ATTRIBUTE = ServletApiAdvice.class.getName() + ".async"; + private static final String CONTENT_TYPE_FROM_URLENCODED = "application/x-www-form-urlencoded"; private final Logger logger = LoggerFactory.getLogger(ServletTransactionHelper.class); @@ -63,12 +63,6 @@ public class ServletTransactionHelper { private final ElasticApmTracer tracer; private final CoreConfiguration coreConfiguration; private final WebConfiguration webConfiguration; - // TODO make configurable - private List recordedContentTypes = Arrays.asList( - WildcardMatcher.valueOf("text/*"), - WildcardMatcher.valueOf("application/json"), - WildcardMatcher.valueOf("application/xml") - ); @VisibleForAdvice public ServletTransactionHelper(ElasticApmTracer tracer) { @@ -129,7 +123,12 @@ public void fillRequestContext(Transaction transaction, String protocol, String private void startCaptureBody(Transaction transaction, String method, @Nullable String contentTypeHeader) { Request request = transaction.getContext().getRequest(); if (hasBody(contentTypeHeader, method)) { - if (webConfiguration.getCaptureBody() != OFF && WildcardMatcher.isAnyMatch(recordedContentTypes, contentTypeHeader)) { + if (webConfiguration.getCaptureBody() != OFF + && contentTypeHeader != null + // form parameters are recorded via ServletRequest.getParameterMap + // as the container might not call ServletRequest.getInputStream + && !contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED) + && WildcardMatcher.isAnyMatch(webConfiguration.getCaptureContentTypes(), contentTypeHeader)) { request.withBodyBuffer(); } else { request.redactBody(); @@ -210,9 +209,10 @@ private void fillRequestParameters(Transaction transaction, String method, @Null @VisibleForAdvice public boolean captureParameters(String method, @Nullable String contentTypeHeader) { return contentTypeHeader != null - && contentTypeHeader.startsWith("application/x-www-form-urlencoded") + && contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED) && hasBody(contentTypeHeader, method) - && webConfiguration.getCaptureBody() != OFF; + && webConfiguration.getCaptureBody() != OFF + && WildcardMatcher.isAnyMatch(webConfiguration.getCaptureContentTypes(), contentTypeHeader); } private boolean isExcluded(String servletPath, @Nullable String pathInfo, @Nullable String userAgentHeader) { @@ -261,7 +261,7 @@ private boolean hasBody(@Nullable String contentTypeHeader, String method) { } private void captureParameters(Request request, Map params, @Nullable String contentTypeHeader) { - if (contentTypeHeader != null && contentTypeHeader.startsWith("application/x-www-form-urlencoded")) { + if (contentTypeHeader != null && contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED)) { for (Map.Entry param : params.entrySet()) { request.addFormUrlEncodedParameters(param.getKey(), param.getValue()); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java index b5302e4a15..a61fae6ca6 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/ApmFilterTest.java @@ -94,21 +94,6 @@ void testURLTransaction() throws IOException, ServletException { assertThat(url.getFull().toString()).isEqualTo("http://localhost/foo/bar?foo=bar"); } - @Test - void testTrackPostParams() throws IOException, ServletException { - when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo/bar"); - request.addParameter("foo", "bar"); - request.addParameter("baz", "qux", "quux"); - request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=uft-8"); - - filterChain.doFilter(request, new MockHttpServletResponse()); - assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isInstanceOf(PotentiallyMultiValuedMap.class); - PotentiallyMultiValuedMap params = (PotentiallyMultiValuedMap) reporter.getFirstTransaction().getContext().getRequest().getBody(); - assertThat(params.get("foo")).isEqualTo("bar"); - assertThat(params.get("baz")).isEqualTo(Arrays.asList("qux", "quux")); - } - @Test void captureException() { // we can't use mock(Servlet.class) here as the agent would instrument the created mock which confuses mockito diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java index 576e41e425..691ec40790 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -23,6 +23,7 @@ import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.impl.transaction.Transaction; import co.elastic.apm.agent.report.serialize.DslJsonSerializer; +import co.elastic.apm.agent.util.PotentiallyMultiValuedMap; import co.elastic.apm.agent.web.WebConfiguration; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.BeforeEach; @@ -30,6 +31,8 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -45,6 +48,8 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; import java.util.stream.Stream; import static org.assertj.core.api.Java6Assertions.assertThat; @@ -204,6 +209,34 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I assertThat(body.toString()).isEqualTo(""); } + @Test + void testTrackPostParams() throws IOException, ServletException { + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo/bar"); + request.addParameter("foo", "bar"); + request.addParameter("baz", "qux", "quux"); + request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=uft-8"); + + filterChain.doFilter(request, new MockHttpServletResponse()); + assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isInstanceOf(PotentiallyMultiValuedMap.class); + PotentiallyMultiValuedMap params = (PotentiallyMultiValuedMap) reporter.getFirstTransaction().getContext().getRequest().getBody(); + assertThat(params.get("foo")).isEqualTo("bar"); + assertThat(params.get("baz")).isEqualTo(Arrays.asList("qux", "quux")); + } + + @Test + void testTrackPostParamsDisabled() throws IOException, ServletException { + when(webConfiguration.getCaptureBody()).thenReturn(WebConfiguration.EventType.ALL); + when(webConfiguration.getCaptureContentTypes()).thenReturn(Collections.emptyList()); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/foo/bar"); + request.addParameter("foo", "bar"); + request.addParameter("baz", "qux", "quux"); + request.addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=uft-8"); + + filterChain.doFilter(request, new MockHttpServletResponse()); + assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isEqualTo("[REDACTED]"); + } + private void executeRequest(MockFilterChain filterChain, byte[] bytes, String contentType) throws IOException, ServletException { try { filterChain.doFilter(createMockRequest(bytes, contentType), new MockHttpServletResponse()); diff --git a/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java b/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java index 4026bbeb60..e0d3ef80fa 100644 --- a/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java +++ b/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java @@ -43,14 +43,34 @@ public class WebConfiguration extends ConfigurationOptionProvider { "\n" + "This option is case-insensitive.\n" + "\n" + - "NOTE: Currently, only `application/x-www-form-urlencoded` (form parameters) are supported.\n" + - "Forms which include a file upload (`multipart/form-data`) are not supported.\n" + + "NOTE: Currently, only UTF-8 encoded plain text content types are supported.\n" + + "The option <> determines which content types are captured.\n" + "\n" + - "WARNING: request bodies often contain sensitive values like passwords, credit card numbers etc." + - "If your service handles data like this, we advise to only enable this feature with care.") + "WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc.\n" + + "If your service handles data like this, we advise to only enable this feature with care.\n" + + "Turning on body capturing can also significantly increase the overhead in terms of heap usage,\n" + + "network utilisation and Elasticsearch index size.") .dynamic(true) .buildWithDefault(EventType.OFF); + private final ConfigurationOption> captureContentTypes = ConfigurationOption + .builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class) + .key("capture_content_types") + .configurationCategory(HTTP_CATEGORY) + .tags("performance") + .description("Configures which content types should be recorded.\n" + + "\n" + + "The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well.\n" + + "\n" + + WildcardMatcher.DOCUMENTATION) + .dynamic(true) + .buildWithDefault(Arrays.asList( + WildcardMatcher.valueOf("application/x-www-form-urlencoded*"), + WildcardMatcher.valueOf("text/*"), + WildcardMatcher.valueOf("application/json*"), + WildcardMatcher.valueOf("application/xml*") + )); + private final ConfigurationOption captureHeaders = ConfigurationOption.booleanOption() .key("capture_headers") .configurationCategory(HTTP_CATEGORY) @@ -157,6 +177,10 @@ public boolean isCaptureHeaders() { return captureHeaders.get(); } + public List getCaptureContentTypes() { + return captureContentTypes.get(); + } + public enum EventType { /** * Request bodies will never be reported From d366064e0dc734c2011ea17674090a366c418e45 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 21 Feb 2019 11:48:25 +0100 Subject: [PATCH 10/15] End reading if reset is called Remove call to readNBytes as it might throw an error if eagerly linked on Java 7 --- .../RecordingServletInputStreamWrapper.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java index 4311179960..9db1eec842 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java @@ -78,18 +78,6 @@ public int read(byte[] b, int off, int len) throws IOException { } } - // not @Override'ing in order to be able to compile against Java 7 - public int readNBytes(byte[] b, int off, int len) throws IOException { - try { - final int read = servletInputStream.readNBytes(b, off, len); - decode(b, off, read); - return read; - } catch (IOException e) { - request.endOfBufferInput(); - throw e; - } - } - @Override public int readLine(byte[] b, int off, int len) throws IOException { try { @@ -140,7 +128,12 @@ public void mark(int readlimit) { @Override public void reset() throws IOException { - servletInputStream.reset(); + try { + servletInputStream.reset(); + } finally { + // don't read things twice from the stream, assume we have read everything by now + request.endOfBufferInput(); + } } @Override From 7d0e4533833cb88a1a99f9cf2854beff99fb372e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 27 Feb 2019 14:59:46 +0100 Subject: [PATCH 11/15] Apply review suggestions --- .../java/co/elastic/apm/agent/impl/context/Request.java | 2 +- .../java/co/elastic/apm/agent/impl/transaction/Db.java | 2 +- .../co/elastic/apm/agent/matcher/WildcardMatcher.java | 8 ++++---- .../servlet/RequestStreamRecordingInstrumentation.java | 8 ++++---- .../servlet/helper/InputStreamFactoryHelperImpl.java | 1 + .../apm/agent/servlet/TestRequestBodyCapturing.java | 6 +++--- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java index 1717cbff2a..869cb2d4eb 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java @@ -42,7 +42,7 @@ public class Request implements Recyclable { - private final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, + private static final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, new Allocator() { @Override public CharBuffer createInstance() { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java index 4a1ae4323b..bb1d7077ec 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Db.java @@ -38,7 +38,7 @@ */ public class Db implements Recyclable { - private final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, + private static final ObjectPool charBufferPool = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, new Allocator() { @Override public CharBuffer createInstance() { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java index 1d8ced99e1..6ea0eae734 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/matcher/WildcardMatcher.java @@ -129,11 +129,11 @@ public static WildcardMatcher valueOf(final String wildcardString) { } /** - * Returns {@code true}, if any of the matchers match the provided string. + * Returns the first {@link WildcardMatcher} {@linkplain WildcardMatcher#matches(String) matching} the provided string. * * @param matchers the matchers which should be used to match the provided string * @param s the string to match against - * @return {@code true}, if any of the matchers match the provided string + * @return the first matching {@link WildcardMatcher}, or {@code null} if none match. */ @Nullable public static boolean isAnyMatch(List matchers, @Nullable String s) { @@ -156,12 +156,12 @@ public static WildcardMatcher anyMatch(List matchers, @Nullable } /** - * Returns {@code true}, if any of the matchers match the provided partitioned string. + * Returns the first {@link WildcardMatcher} {@linkplain WildcardMatcher#matches(String) matching} the provided partitioned string. * * @param matchers the matchers which should be used to match the provided string * @param firstPart The first part of the string to match against. * @param secondPart The second part of the string to match against. - * @return {@code true}, if any of the matchers match the provided partitioned string + * @return the first matching {@link WildcardMatcher}, or {@code null} if none match. * @see #matches(String, String) */ @Nullable diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java index e4ad17294a..79a34c5625 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java @@ -48,7 +48,7 @@ public class RequestStreamRecordingInstrumentation extends ElasticApmInstrumenta @Nullable @VisibleForAdvice - // referring to AsyncContext is legal because of type erasure + // referring to InputStreamWrapperFactory is legal because of type erasure public static HelperClassManager wrapperHelperClassManager; @Override @@ -108,10 +108,10 @@ public static void onReadEnter(@Advice.This Object thiz, @Advice.OnMethodExit(suppress = Throwable.class) public static void afterGetInputStream(@Advice.Return(readOnly = false) ServletInputStream inputStream, @Advice.Local("nested") boolean nested) { + if (nested || tracer == null || wrapperHelperClassManager == null) { + return; + } try { - if (nested || tracer == null || wrapperHelperClassManager == null) { - return; - } final Transaction transaction = tracer.currentTransaction(); // only wrap if the body buffer has been initialized via ServletTransactionHelper.startCaptureBody if (transaction != null && transaction.getContext().getRequest().getBodyBuffer() != null) { diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java index 170074997c..f7bf34ed6d 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java @@ -27,6 +27,7 @@ public class InputStreamFactoryHelperImpl implements RequestStreamRecordingInstrumentation.InputStreamWrapperFactory { @Override public ServletInputStream wrap(Request request, ServletInputStream servletInputStream) { + System.out.println("wrap"); return new RecordingServletInputStreamWrapper(request, servletInputStream); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java index 691ec40790..955cfd8a6b 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -52,7 +52,7 @@ import java.util.Collections; import java.util.stream.Stream; -import static org.assertj.core.api.Java6Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; class TestRequestBodyCapturing extends AbstractInstrumentationTest { @@ -104,11 +104,11 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I @MethodSource("streamConsumers") void testReadTextPlain(InputStreamConsumer consumer) throws Exception { streamConsumer = consumer; - executeRequest(filterChain, "foo".getBytes(StandardCharsets.UTF_8), "text/plain"); + executeRequest(filterChain, "foo\nbar".getBytes(StandardCharsets.UTF_8), "text/plain"); final Object body = reporter.getFirstTransaction().getContext().getRequest().getBody(); assertThat(body).isNotNull(); - assertThat(body.toString()).isEqualTo("foo"); + assertThat(body.toString()).isEqualTo("foo\nbar"); } @Test From 309ebe395e2d9254aca0149c0e64354a62761175 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 27 Feb 2019 16:47:20 +0100 Subject: [PATCH 12/15] Add javadoc for setRawBody --- .../main/java/co/elastic/apm/agent/impl/context/Request.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java index 869cb2d4eb..d864dc958f 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java @@ -110,6 +110,11 @@ public String getRawBody() { return rawBody; } + /** + * Sets the body as a raw string and removes any previously set {@link #postParams} or {@link #bodyBuffer}. + * + * @param rawBody the body as a raw string + */ public void setRawBody(String rawBody) { postParams.resetState(); if (bodyBuffer != null) { From c65c9fdf3fabdfae6d89347e44dadd615ad99e76 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 28 Feb 2019 16:30:53 +0100 Subject: [PATCH 13/15] Add integration tests for different read methods fix recycling bug --- .../apm/agent/impl/context/Request.java | 1 + .../servlet/TestRequestBodyCapturing.java | 9 +++++ ...stractServletContainerIntegrationTest.java | 12 ++++--- .../java/co/elastic/apm/servlet/JettyIT.java | 1 - .../tests/JsfApplicationServerTestApp.java | 1 - .../apm/servlet/tests/ServletApiTestApp.java | 36 ++++++++++--------- .../java/co/elastic/webapp/EchoServlet.java | 29 +++++++++++++-- 7 files changed, 63 insertions(+), 26 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java index d864dc958f..ceb0edaaa5 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/Request.java @@ -284,6 +284,7 @@ public void resetState() { socket.resetState(); url.resetState(); cookies.resetState(); + bodyBufferFinished = false; if (bodyBuffer != null) { charBufferPool.recycle(bodyBuffer); bodyBuffer = null; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java index 955cfd8a6b..527a5c2852 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -68,6 +68,7 @@ static Stream streamConsumers() { InputStream::read, is -> is.read(BUFFER), is -> is.read(BUFFER, 0, BUFFER.length), + is -> is.read(BUFFER, 42, BUFFER.length / 2), is -> is.readLine(BUFFER, 0, BUFFER.length), is -> { is.readNBytes(BUFFER, 0, BUFFER.length); @@ -237,6 +238,14 @@ void testTrackPostParamsDisabled() throws IOException, ServletException { assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody()).isEqualTo("[REDACTED]"); } + @Test + void testNoExplicitEndOfInput() { + final Transaction transaction = tracer.startTransaction(); + transaction.getContext().getRequest().withBodyBuffer(); + transaction.end(); + assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody().toString()).isEqualTo(""); + } + private void executeRequest(MockFilterChain filterChain, byte[] bytes, String contentType) throws IOException, ServletException { try { filterChain.doFilter(createMockRequest(bytes, contentType), new MockHttpServletResponse()); diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java index e370042d94..93a756b5f6 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/AbstractServletContainerIntegrationTest.java @@ -219,7 +219,7 @@ public void clearMockServerLog() { } public JsonNode assertTransactionReported(String pathToTest, int expectedResponseCode) { - final List reportedTransactions = getAllReported(500, this::getReportedTransactions, 1); + final List reportedTransactions = getAllReported(this::getReportedTransactions, 1); JsonNode transaction = reportedTransactions.iterator().next(); assertThat(transaction.get("context").get("request").get("url").get("pathname").textValue()).isEqualTo(pathToTest); assertThat(transaction.get("context").get("response").get("status_code").intValue()).isEqualTo(expectedResponseCode); @@ -247,23 +247,25 @@ public Response executeRequest(String pathToTest) throws IOException { } @Nonnull - public List getAllReported(int timeoutMs, Supplier> supplier, int expected) { + public List getAllReported(Supplier> supplier, int expected) { + long timeout = ENABLE_DEBUGGING ? 600_000 : 500; long start = System.currentTimeMillis(); List reportedTransactions; do { reportedTransactions = supplier.get(); - } while (reportedTransactions.size() != expected && System.currentTimeMillis() - start < timeoutMs); + } while (reportedTransactions.size() != expected && System.currentTimeMillis() - start < timeout); assertThat(reportedTransactions).hasSize(expected); return reportedTransactions; } @Nonnull - public List assertSpansTransactionId(int timeoutMs, Supplier> supplier, String transactionId) { + public List assertSpansTransactionId(Supplier> supplier, String transactionId) { + long timeout = ENABLE_DEBUGGING ? 600_000 : 500; long start = System.currentTimeMillis(); List reportedSpans; do { reportedSpans = supplier.get(); - } while (reportedSpans.size() == 0 && System.currentTimeMillis() - start < timeoutMs); + } while (reportedSpans.size() == 0 && System.currentTimeMillis() - start < timeout); assertThat(reportedSpans.size()).isGreaterThanOrEqualTo(1); for (JsonNode span : reportedSpans) { assertThat(span.get("transaction_id").textValue()).isEqualTo(transactionId); diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java index aa9e8d070e..4f0f7e9ded 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/JettyIT.java @@ -42,7 +42,6 @@ public JettyIT(final String version) { .withEnv("ELASTIC_APM_IGNORE_URLS", "/status*,/favicon.ico") .withEnv("ELASTIC_APM_REPORT_SYNC", "true") .withEnv("ELASTIC_APM_LOGGING_LOG_LEVEL", "DEBUG") - .withLogConsumer(new StandardOutLogConsumer().withPrefix("jetty")) .withExposedPorts(8080), "jetty-application", "/var/lib/jetty/webapps", diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java index 72a6a0dcd0..aaf030f1b3 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/JsfApplicationServerTestApp.java @@ -51,7 +51,6 @@ private void testJsfRequest(AbstractServletContainerIntegrationTest containerInt assertThat(transaction.get("name").textValue()).isEqualTo(testedPath); String transactionId = transaction.get("id").textValue(); List spans = containerIntegrationTest.assertSpansTransactionId( - 500, containerIntegrationTest::getReportedSpans, transactionId); assertThat(spans.size()).isEqualTo(2); diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java index 1d2eddc335..339881bdde 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java @@ -48,17 +48,19 @@ public void test(AbstractServletContainerIntegrationTest test) throws Exception } private void testCaptureBody(AbstractServletContainerIntegrationTest test) throws Exception { - test.clearMockServerLog(); - final Response response = test.getHttpClient().newCall(new Request.Builder() - .post(RequestBody.create(MediaType.parse("text/plain"), "{foo}")) - .url(test.getBaseUrl() + "/simple-webapp/echo") - .build()) - .execute(); - assertThat(response.code()).isEqualTo(200); - assertThat(response.body().string()).isEqualTo("{foo}"); - - final JsonNode transaction = test.assertTransactionReported("/simple-webapp/echo", 200); - assertThat(transaction.get("context").get("request").get("body").textValue()).isEqualTo("{foo}"); + for (String readMethod : List.of(/*"read-byte", "read-bytes",*/ "read-offset", "read-line")) { + test.clearMockServerLog(); + final Response response = test.getHttpClient().newCall(new Request.Builder() + .post(RequestBody.create(MediaType.parse("text/plain"), "{foo}\n{bar}")) + .url(test.getBaseUrl() + "/simple-webapp/echo?read-method=" + readMethod) + .build()) + .execute(); + assertThat(response.code()).isEqualTo(200); + assertThat(response.body().string()).isEqualTo("{foo}\n{bar}"); + + final JsonNode transaction = test.assertTransactionReported("/simple-webapp/echo", 200); + assertThat(transaction.get("context").get("request").get("body").textValue()).isEqualTo("{foo}\n{bar}"); + } } private void testExecutorService(AbstractServletContainerIntegrationTest test) throws Exception { @@ -66,7 +68,7 @@ private void testExecutorService(AbstractServletContainerIntegrationTest test) t final String pathToTest = "/simple-webapp/executor-service-servlet"; test.executeAndValidateRequest(pathToTest, null, 200); String transactionId = test.assertTransactionReported(pathToTest, 200).get("id").textValue(); - final List spans = test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + final List spans = test.assertSpansTransactionId(test::getReportedSpans, transactionId); assertThat(spans).hasSize(1); } @@ -75,11 +77,11 @@ private void testHttpUrlConnection(AbstractServletContainerIntegrationTest test) final String pathToTest = "/simple-webapp/http-url-connection"; test.executeAndValidateRequest(pathToTest, "Hello World!", 200); - final List reportedTransactions = test.getAllReported(500, test::getReportedTransactions, 2); + final List reportedTransactions = test.getAllReported(test::getReportedTransactions, 2); final JsonNode innerTransaction = reportedTransactions.get(0); final JsonNode outerTransaction = reportedTransactions.get(1); - final List spans = test.assertSpansTransactionId(500, test::getReportedSpans, outerTransaction.get("id").textValue()); + final List spans = test.assertSpansTransactionId(test::getReportedSpans, outerTransaction.get("id").textValue()); assertThat(spans).hasSize(1); final JsonNode span = spans.get(0); @@ -98,7 +100,7 @@ private void testTransactionReporting(AbstractServletContainerIntegrationTest te test.executeAndValidateRequest(pathToTest, "Hello World", 200); JsonNode transaction = test.assertTransactionReported(pathToTest, 200); String transactionId = transaction.get("id").textValue(); - List spans = test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + List spans = test.assertSpansTransactionId(test::getReportedSpans, transactionId); for (JsonNode span : spans) { assertThat(span.get("type").textValue()).isEqualTo("db.h2.query"); } @@ -113,7 +115,7 @@ private void testSpanErrorReporting(AbstractServletContainerIntegrationTest test test.executeAndValidateRequest(pathToTest + "?cause_db_error=true", "DB Error", 200); JsonNode transaction = test.assertTransactionReported(pathToTest, 200); String transactionId = transaction.get("id").textValue(); - test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + test.assertSpansTransactionId(test::getReportedSpans, transactionId); test.assertErrorContent(500, test::getReportedErrors, transactionId, "Column \"NON_EXISTING_COLUMN\" not found"); } } @@ -126,7 +128,7 @@ private void testTransactionErrorReporting(AbstractServletContainerIntegrationTe test.executeAndValidateRequest(fullPathToTest + "?cause_transaction_error=true", "", null); JsonNode transaction = test.assertTransactionReported(fullPathToTest, 500); String transactionId = transaction.get("id").textValue(); - test.assertSpansTransactionId(500, test::getReportedSpans, transactionId); + test.assertSpansTransactionId(test::getReportedSpans, transactionId); // we currently only report errors when Exceptions are caught, still this test is relevant for response code capturing if (test.isExpectedStacktrace(pathToTest)) { test.assertErrorContent(500, test::getReportedErrors, transactionId, "Transaction failure"); diff --git a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java index 92dad12f0c..0e5b0d4a1e 100644 --- a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java +++ b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/EchoServlet.java @@ -19,6 +19,8 @@ */ package co.elastic.webapp; +import javax.servlet.ServletInputStream; +import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -30,8 +32,31 @@ public class EchoServlet extends HttpServlet { protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { byte[] buffer = new byte[1024]; int read; - while ((read = req.getInputStream().read(buffer)) > 0) { - resp.getOutputStream().write(buffer, 0, read); + final ServletOutputStream os = resp.getOutputStream(); + final ServletInputStream is = req.getInputStream(); + switch (req.getParameter("read-method")) { + case "read-byte": + while ((read = is.read()) > 0) { + os.write(read); + } + break; + case "read-bytes": + while ((read = is.read(buffer)) > 0) { + os.write(buffer, 0, read); + } + break; + case "read-offset": + while ((read = is.read(buffer, 42, buffer.length / 2)) > 0) { + os.write(buffer, 42, read); + } + break; + case "read-line": + while ((read = is.readLine(buffer, 0, buffer.length)) > 0) { + os.write(buffer, 0, read); + } + break; + default: + throw new IllegalArgumentException("Invalid read-method"); } resp.setContentType(req.getContentType()); } From 368a8eff0fd37d024b93315fb7a3f7b8c958acbc Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 4 Mar 2019 10:22:42 +0100 Subject: [PATCH 14/15] Remove leftovers --- .../apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java | 1 - .../java/co/elastic/apm/servlet/tests/ServletApiTestApp.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java index f7bf34ed6d..170074997c 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java @@ -27,7 +27,6 @@ public class InputStreamFactoryHelperImpl implements RequestStreamRecordingInstrumentation.InputStreamWrapperFactory { @Override public ServletInputStream wrap(Request request, ServletInputStream servletInputStream) { - System.out.println("wrap"); return new RecordingServletInputStreamWrapper(request, servletInputStream); } } diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java index 339881bdde..de4cf3c0be 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/tests/ServletApiTestApp.java @@ -48,7 +48,7 @@ public void test(AbstractServletContainerIntegrationTest test) throws Exception } private void testCaptureBody(AbstractServletContainerIntegrationTest test) throws Exception { - for (String readMethod : List.of(/*"read-byte", "read-bytes",*/ "read-offset", "read-line")) { + for (String readMethod : List.of("read-byte", "read-bytes", "read-offset", "read-line")) { test.clearMockServerLog(); final Response response = test.getHttpClient().newCall(new Request.Builder() .post(RequestBody.create(MediaType.parse("text/plain"), "{foo}\n{bar}")) From 77e1f4cb740cc12d159bd3711c98f8a6c3971990 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 22 Mar 2019 08:48:12 +0100 Subject: [PATCH 15/15] Rename capture_content_types to capture_body_content_types --- .../servlet/TestRequestBodyCapturing.java | 3 +- .../apm/agent/web/WebConfiguration.java | 6 +- docs/configuration.asciidoc | 65 ++++++++++++++++--- 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java index 527a5c2852..70f181f064 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/TestRequestBodyCapturing.java @@ -21,6 +21,7 @@ import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.impl.error.ErrorCapture; +import co.elastic.apm.agent.impl.transaction.TraceContext; import co.elastic.apm.agent.impl.transaction.Transaction; import co.elastic.apm.agent.report.serialize.DslJsonSerializer; import co.elastic.apm.agent.util.PotentiallyMultiValuedMap; @@ -240,7 +241,7 @@ void testTrackPostParamsDisabled() throws IOException, ServletException { @Test void testNoExplicitEndOfInput() { - final Transaction transaction = tracer.startTransaction(); + final Transaction transaction = tracer.startTransaction(TraceContext.asRoot(), null, getClass().getClassLoader()); transaction.getContext().getRequest().withBodyBuffer(); transaction.end(); assertThat(reporter.getFirstTransaction().getContext().getRequest().getBody().toString()).isEqualTo(""); diff --git a/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java b/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java index e0d3ef80fa..1d5d6def32 100644 --- a/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java +++ b/apm-agent-plugins/apm-web-plugin/src/main/java/co/elastic/apm/agent/web/WebConfiguration.java @@ -44,7 +44,7 @@ public class WebConfiguration extends ConfigurationOptionProvider { "This option is case-insensitive.\n" + "\n" + "NOTE: Currently, only UTF-8 encoded plain text content types are supported.\n" + - "The option <> determines which content types are captured.\n" + + "The option <> determines which content types are captured.\n" + "\n" + "WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc.\n" + "If your service handles data like this, we advise to only enable this feature with care.\n" + @@ -55,9 +55,9 @@ public class WebConfiguration extends ConfigurationOptionProvider { private final ConfigurationOption> captureContentTypes = ConfigurationOption .builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class) - .key("capture_content_types") + .key("capture_body_content_types") .configurationCategory(HTTP_CATEGORY) - .tags("performance") + .tags("added[1.5.0]", "performance") .description("Configures which content types should be recorded.\n" + "\n" + "The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well.\n" + diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 4a348c636a..2eeed79b61 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -72,6 +72,7 @@ Click on a key to get more information. ** <> * <> ** <> +** <> ** <> ** <> ** <> @@ -319,7 +320,7 @@ you should add an additional entry to this list (make sure to also include the d ==== `disable_instrumentations` A list of instrumentations which should be disabled. -Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. +Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. If you want to try out incubating features, set the value to an empty string. @@ -426,10 +427,13 @@ If the request has a body and this setting is disabled, the body will be shown a This option is case-insensitive. -NOTE: Currently, only `application/x-www-form-urlencoded` (form parameters) are supported. -Forms which include a file upload (`multipart/form-data`) are not supported. +NOTE: Currently, only UTF-8 encoded plain text content types are supported. +The option <> determines which content types are captured. -WARNING: request bodies often contain sensitive values like passwords, credit card numbers etc.If your service handles data like this, we advise to only enable this feature with care. +WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc. +If your service handles data like this, we advise to only enable this feature with care. +Turning on body capturing can also significantly increase the overhead in terms of heap usage, +network utilisation and Elasticsearch index size. Valid options: `off`, `errors`, `transactions`, `all` @@ -446,6 +450,33 @@ Valid options: `off`, `errors`, `transactions`, `all` | `elastic.apm.capture_body` | `capture_body` | `ELASTIC_APM_CAPTURE_BODY` |============ +[float] +[[config-capture-body-content-types]] +==== `capture_body_content_types` (added[1.5.0] performance) + +Configures which content types should be recorded. + +The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well. + +This option supports the wildcard `*`, which matches zero or more characters. +Examples: `/foo/*/bar/*/baz*`, `*foo*`. +Matching is case insensitive by default. +Prepending an element with `(?-i)` makes the matching case sensitive. + + +[options="header"] +|============ +| Default | Type | Dynamic +| `application/x-www-form-urlencoded*, text/*, application/json*, application/xml*` | List | true +|============ + + +[options="header"] +|============ +| Java System Properties | Property file | Environment +| `elastic.apm.capture_body_content_types` | `capture_body_content_types` | `ELASTIC_APM_CAPTURE_BODY_CONTENT_TYPES` +|============ + [float] [[config-capture-headers]] ==== `capture_headers` (performance) @@ -1173,7 +1204,7 @@ If the service name is set explicitly, it overrides all of the above. # sanitize_field_names=password,passwd,pwd,secret,*key,*token*,*session*,*credit*,*card*,authorization,set-cookie # A list of instrumentations which should be disabled. -# Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. +# Valid options are `annotations`, `apache-httpclient`, `concurrent`, `dispatcher-servlet`, `elasticsearch-restclient`, `executor`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jsf`, `okhttp`, `opentracing`, `public-api`, `render`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `servlet-service-name`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `urlconnection`. # If you want to try out incubating features, # set the value to an empty string. # @@ -1246,10 +1277,13 @@ If the service name is set explicitly, it overrides all of the above. # # This option is case-insensitive. # -# NOTE: Currently, only `application/x-www-form-urlencoded` (form parameters) are supported. -# Forms which include a file upload (`multipart/form-data`) are not supported. +# NOTE: Currently, only UTF-8 encoded plain text content types are supported. +# The option <> determines which content types are captured. # -# WARNING: request bodies often contain sensitive values like passwords, credit card numbers etc.If your service handles data like this, we advise to only enable this feature with care. +# WARNING: Request bodies often contain sensitive values like passwords, credit card numbers etc. +# If your service handles data like this, we advise to only enable this feature with care. +# Turning on body capturing can also significantly increase the overhead in terms of heap usage, +# network utilisation and Elasticsearch index size. # # Valid options: off, errors, transactions, all # This setting can be changed at runtime @@ -1258,6 +1292,21 @@ If the service name is set explicitly, it overrides all of the above. # # capture_body=OFF +# Configures which content types should be recorded. +# +# The defaults end with a wildcard so that content types like `text/plain; charset=utf-8` are captured as well. +# +# This option supports the wildcard `*`, which matches zero or more characters. +# Examples: `/foo/*/bar/*/baz*`, `*foo*`. +# Matching is case insensitive by default. +# Prepending an element with `(?-i)` makes the matching case sensitive. +# +# This setting can be changed at runtime +# Type: comma separated list +# Default value: application/x-www-form-urlencoded*,text/*,application/json*,application/xml* +# +# capture_body_content_types=application/x-www-form-urlencoded*,text/*,application/json*,application/xml* + # If set to `true`, # the agent will capture request and response headers, including cookies. #