From ed26a8a69b91cd0149f1208ab8d50e93017be00c Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Thu, 20 Jun 2024 10:49:09 +0200 Subject: [PATCH 01/12] Implemented request body capturing for apache httpclient 4.x --- .../apm/agent/impl/context/HttpImpl.java | 13 +++ .../AbstractApacheHttpClientAdvice.java | 2 +- .../v4/ApacheHttpClientInstrumentation.java | 10 ++- ...heHttpEntityGetContentInstrumentation.java | 76 ++++++++++++++++ ...pacheHttpEntityWriteToInstrumentation.java | 81 +++++++++++++++++ ...LegacyApacheHttpClientInstrumentation.java | 2 + .../HttpAsyncRequestProducerWrapper.java | 1 + .../v4/helper/RequestBodyCaptureRegistry.java | 64 +++++++++++++ ...ic.apm.agent.sdk.ElasticApmInstrumentation | 2 + ...cheHttpAsyncClientInstrumentationTest.java | 46 ++++++++++ .../ApacheHttpClientInstrumentationTest.java | 32 +++++++ ...ntBasicHttpRequestInstrumentationTest.java | 2 + ...ientHttpUriRequestInstrumentationTest.java | 44 ++++++++- .../RequestBodyRecordingInputStream.java | 87 ++++++++++++++++++ .../RequestBodyRecordingOutputStream.java | 90 +++++++++++++++++++ ...AbstractHttpClientInstrumentationTest.java | 17 ++++ .../configuration/WebConfiguration.java | 13 +++ .../apm/agent/tracer/metadata/Http.java | 3 + 18 files changed, 582 insertions(+), 3 deletions(-) create mode 100644 apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java create mode 100644 apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java create mode 100644 apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java create mode 100644 apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java create mode 100644 apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java index 4995e3ef51..4c123d2bce 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java @@ -36,6 +36,9 @@ public class HttpImpl implements Recyclable, Http { @Nullable private String method; + @Nullable + private StringBuilder requestBody; + /** * Status code of the response */ @@ -65,6 +68,15 @@ public int getStatusCode() { return statusCode; } + @Override + @Nullable + public StringBuilder getRequestBody(boolean initialize) { + if (initialize && requestBody == null) { + requestBody = new StringBuilder(); + } + return requestBody; + } + @Override public HttpImpl withUrl(@Nullable String url) { if (url != null) { @@ -90,6 +102,7 @@ public void resetState() { url.resetState(); method = null; statusCode = 0; + requestBody = null; } public boolean hasContent() { diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAdvice.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAdvice.java index 3a866b8aa9..1998180c3b 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAdvice.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAdvice.java @@ -34,7 +34,7 @@ public abstract class AbstractApacheHttpClientAdvice { public static & - TextHeaderGetter> Object startSpan(final Tracer tracer, + TextHeaderGetter> Span startSpan(final Tracer tracer, final ApacheHttpClientApiAdapter adapter, final WRAPPER request, @Nullable final HTTPHOST httpHost, diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java index 1d60900984..06b2c70c6f 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java @@ -20,7 +20,11 @@ import co.elastic.apm.agent.httpclient.common.AbstractApacheHttpClientAdvice; import co.elastic.apm.agent.httpclient.v4.helper.ApacheHttpClient4ApiAdapter; +import co.elastic.apm.agent.httpclient.v4.helper.RequestBodyCaptureRegistry; import co.elastic.apm.agent.httpclient.v4.helper.RequestHeaderAccessor; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.tracer.Span; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -47,13 +51,17 @@ public class ApacheHttpClientInstrumentation extends BaseApacheHttpClientInstrumentation { public static class ApacheHttpClient4Advice extends AbstractApacheHttpClientAdvice { + private static final Logger logger = LoggerFactory.getLogger(ApacheHttpClient4Advice.class); + private static final ApacheHttpClient4ApiAdapter adapter = ApacheHttpClient4ApiAdapter.get(); @Nullable @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static Object onBeforeExecute(@Advice.Argument(0) HttpRoute route, @Advice.Argument(1) HttpRequestWrapper request) throws URISyntaxException { - return startSpan(tracer, adapter, request, route.getTargetHost(), RequestHeaderAccessor.INSTANCE); + Span span = startSpan(tracer, adapter, request, route.getTargetHost(), RequestHeaderAccessor.INSTANCE); + RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, span); + return span; } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java new file mode 100644 index 0000000000..a87d6c4b3f --- /dev/null +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java @@ -0,0 +1,76 @@ +package co.elastic.apm.agent.httpclient.v4; + +import co.elastic.apm.agent.httpclient.RequestBodyRecordingInputStream; +import co.elastic.apm.agent.httpclient.RequestBodyRecordingOutputStream; +import co.elastic.apm.agent.httpclient.v4.helper.RequestBodyCaptureRegistry; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.tracer.Span; +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 org.apache.http.HttpEntity; + +import java.io.InputStream; +import java.io.OutputStream; +import java.net.URISyntaxException; + +import static co.elastic.apm.agent.sdk.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +public class ApacheHttpEntityGetContentInstrumentation extends BaseApacheHttpClientInstrumentation { + + public static class ApacheHttpEntityGetContentAdvice { + + private static final Logger logger = LoggerFactory.getLogger(ApacheHttpEntityGetContentAdvice.class); + + @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToReturned + public static InputStream onExit(@Advice.This HttpEntity thiz, @Advice.Return InputStream content) throws URISyntaxException { + Span clientSpan = RequestBodyCaptureRegistry.removeSpanFor(thiz); + if (clientSpan != null) { + logger.debug("Wrapping input stream for request body capture for HttpEntity {} ({}) for span {}", thiz.getClass().getName(), System.identityHashCode(thiz), clientSpan); + return new RequestBodyRecordingInputStream(content, clientSpan); + } + return content; + } + } + + @Override + public String getAdviceClassName() { + return "co.elastic.apm.agent.httpclient.v4.ApacheHttpEntityGetContentInstrumentation$ApacheHttpEntityGetContentAdvice"; + } + + @Override + public ElementMatcher.Junction getClassLoaderMatcher() { + return not(isBootstrapClassLoader()) + .and(classLoaderCanLoadClass("org.apache.http.HttpEntity")); + } + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameStartsWith("org.apache.http").and(nameContains("Entity")); + } + + @Override + public ElementMatcher getTypeMatcher() { + return hasSuperType(named("org.apache.http.HttpEntity")); + } + + @Override + public ElementMatcher getMethodMatcher() { + return named("getContent") + .and(takesArguments(0)); + } + +} diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java new file mode 100644 index 0000000000..a17015958a --- /dev/null +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java @@ -0,0 +1,81 @@ +package co.elastic.apm.agent.httpclient.v4; + +import co.elastic.apm.agent.httpclient.RequestBodyRecordingOutputStream; +import co.elastic.apm.agent.httpclient.v4.helper.RequestBodyCaptureRegistry; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.tracer.Span; +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 org.apache.http.HttpEntity; + +import java.io.OutputStream; +import java.net.URISyntaxException; + +import static co.elastic.apm.agent.sdk.bytebuddy.CustomElementMatchers.classLoaderCanLoadClass; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader; +import static net.bytebuddy.matcher.ElementMatchers.nameContains; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +public class ApacheHttpEntityWriteToInstrumentation extends BaseApacheHttpClientInstrumentation { + + public static class ApacheHttpEntityWriteToAdvice { + + private static final Logger logger = LoggerFactory.getLogger(ApacheHttpEntityWriteToAdvice.class); + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToArguments(@Advice.AssignReturned.ToArguments.ToArgument(0)) + public static OutputStream onEnter(@Advice.This HttpEntity thiz, @Advice.Argument(0) OutputStream drain) throws URISyntaxException { + Span clientSpan = RequestBodyCaptureRegistry.removeSpanFor(thiz); + if (clientSpan != null) { + logger.debug("Wrapping output stream for request body capture for HttpEntity {} ({}) for span {}", thiz.getClass().getName(), System.identityHashCode(thiz), clientSpan); + return new RequestBodyRecordingOutputStream(drain, clientSpan); + } + return drain; + } + + @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + public static void onExit(@Advice.Enter OutputStream potentiallyWrappedStream) throws URISyntaxException { + if (potentiallyWrappedStream instanceof RequestBodyRecordingOutputStream) { + ((RequestBodyRecordingOutputStream) potentiallyWrappedStream).releaseSpan(); + } + } + } + + @Override + public String getAdviceClassName() { + return "co.elastic.apm.agent.httpclient.v4.ApacheHttpEntityWriteToInstrumentation$ApacheHttpEntityWriteToAdvice"; + } + + @Override + public ElementMatcher.Junction getClassLoaderMatcher() { + return not(isBootstrapClassLoader()) + .and(classLoaderCanLoadClass("org.apache.http.HttpEntity")); + } + + @Override + public ElementMatcher getTypeMatcherPreFilter() { + return nameStartsWith("org.apache.http").and(nameContains("Entity")); + } + + @Override + public ElementMatcher getTypeMatcher() { + return hasSuperType(named("org.apache.http.HttpEntity")); + } + + @Override + public ElementMatcher getMethodMatcher() { + return named("writeTo") + .and(takesArguments(1)) + .and(takesArgument(0, OutputStream.class)); + } + +} diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java index 8097b84d1c..9c20886e38 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java @@ -19,6 +19,7 @@ package co.elastic.apm.agent.httpclient.v4; import co.elastic.apm.agent.httpclient.HttpClientHelper; +import co.elastic.apm.agent.httpclient.v4.helper.RequestBodyCaptureRegistry; import co.elastic.apm.agent.httpclient.v4.helper.RequestHeaderAccessor; import co.elastic.apm.agent.tracer.TraceState; import co.elastic.apm.agent.tracer.Outcome; @@ -101,6 +102,7 @@ public static Object onBeforeExecute(@Advice.Argument(0) @Nullable HttpHost host span = HttpClientHelper.startHttpClientSpan(activeContext, method, uri, hostName); if (span != null) { + RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, span); span.activate(); } } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/HttpAsyncRequestProducerWrapper.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/HttpAsyncRequestProducerWrapper.java index 6d874c13fc..a8648894ba 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/HttpAsyncRequestProducerWrapper.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/HttpAsyncRequestProducerWrapper.java @@ -85,6 +85,7 @@ public HttpRequest generateRequest() throws IOException, HttpException { // trace context propagation if (request != null) { if (span != null) { + RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, span); RequestLine requestLine = request.getRequestLine(); if (requestLine != null) { String method = requestLine.getMethod(); diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java new file mode 100644 index 0000000000..2314192411 --- /dev/null +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java @@ -0,0 +1,64 @@ +package co.elastic.apm.agent.httpclient.v4.helper; + +import co.elastic.apm.agent.httpclient.v4.ApacheHttpClientInstrumentation; +import co.elastic.apm.agent.httpclient.v4.BaseApacheHttpClientInstrumentation; +import co.elastic.apm.agent.sdk.logging.Logger; +import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.sdk.state.GlobalState; +import co.elastic.apm.agent.tracer.GlobalTracer; +import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.Tracer; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; +import co.elastic.apm.agent.tracer.reference.ReferenceCountedMap; +import org.apache.http.HttpEntity; +import org.apache.http.HttpEntityEnclosingRequest; +import org.apache.http.HttpRequest; +import org.apache.http.client.methods.HttpRequestWrapper; + +import javax.annotation.Nullable; + +public class RequestBodyCaptureRegistry { + + private static final Logger logger = LoggerFactory.getLogger(RequestBodyCaptureRegistry.class); + + private static final Tracer tracer = GlobalTracer.get(); + + @GlobalState + public static class MapHolder { + private static final ReferenceCountedMap> entityToClientSpan = GlobalTracer.get().newReferenceCountedMap(); + + + public static void captureBodyFor(Object entity, Span httpClientSpan) { + entityToClientSpan.put(entity, httpClientSpan); + } + + @Nullable + public static Span removeSpanFor(Object entity) { + return entityToClientSpan.remove(entity); + } + } + + + public static void potentiallyCaptureRequestBody(HttpRequest request, @Nullable Span span) { + if (span != null && span.isSampled() && !tracer.getConfig(WebConfiguration.class).getCaptureClientRequestContentTypes().isEmpty()) { + if (request instanceof HttpEntityEnclosingRequest) { + HttpEntity entity = ((HttpEntityEnclosingRequest) request).getEntity(); + if (entity != null) { + logger.debug("Enabling request capture for entity {}() for span {}", entity.getClass().getName(), System.identityHashCode(entity), span); + MapHolder.captureBodyFor(entity, span); + } else { + logger.debug("HttpEntity is null for span {}", span); + } + + } else { + logger.debug("Not capturing request body because {} is not an HttpEntityEnclosingRequest", request.getClass().getName()); + } + } + } + + @Nullable + public static Span removeSpanFor(HttpEntity entity) { + return MapHolder.removeSpanFor(entity); + } + +} diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation index 753c7c4605..8d04c1a373 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation @@ -2,3 +2,5 @@ co.elastic.apm.agent.httpclient.v4.ApacheHttpClientInstrumentation co.elastic.apm.agent.httpclient.v4.ApacheHttpAsyncClientInstrumentation co.elastic.apm.agent.httpclient.v4.ApacheHttpAsyncClientRedirectInstrumentation co.elastic.apm.agent.httpclient.v4.LegacyApacheHttpClientInstrumentation +co.elastic.apm.agent.httpclient.v4.ApacheHttpEntityWriteToInstrumentation +co.elastic.apm.agent.httpclient.v4.ApacheHttpEntityGetContentInstrumentation diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java index 20543c1073..c6d5302add 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java @@ -18,15 +18,19 @@ */ package co.elastic.apm.agent.httpclient.v4; +import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest; import co.elastic.apm.agent.impl.transaction.SpanImpl; import co.elastic.apm.agent.tracer.Outcome; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import org.apache.http.HttpResponse; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.concurrent.FutureCallback; import org.apache.http.conn.UnsupportedSchemeException; +import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; import org.apache.http.impl.nio.client.HttpAsyncClients; import org.assertj.core.api.Assertions; @@ -34,11 +38,16 @@ import org.junit.BeforeClass; import org.junit.Test; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.doReturn; public class ApacheHttpAsyncClientInstrumentationTest extends AbstractHttpClientInstrumentationTest { @@ -125,5 +134,42 @@ public void testSpanWithIllegalProtocol() throws Exception { Assertions.assertThat(reporter.getSpans()).hasSize(1); } + @Test + public void testPostBodyCapture() throws IOException, ExecutionException, InterruptedException { + doReturn(Collections.singletonList(WildcardMatcher.matchAll())) + .when(getConfig().getConfig(WebConfiguration.class)).getCaptureClientRequestContentTypes(); + + StringBuilder longString = new StringBuilder(); + for (int i = 0; i < 200; i++) { + longString.append(String.format("line %1$4d\n", i)); + } + HttpPost request = new HttpPost(getBaseUrl() + "/"); + request.setEntity(new InputStreamEntity(new ByteArrayInputStream(longString.toString().getBytes(StandardCharsets.UTF_8)))); + + final CompletableFuture responseFuture = new CompletableFuture<>(); + + client.execute(request, new FutureCallback<>() { + @Override + public void completed(HttpResponse result) { + responseFuture.complete(result); + } + + @Override + public void failed(Exception ex) { + responseFuture.completeExceptionally(ex); + } + + @Override + public void cancelled() { + responseFuture.cancel(true); + } + }); + responseFuture.get(); + + expectSpan("/") + .withRequestBodySatisfying(body -> { + // assertThat(body).endsWith("line 101\nline"); + }).verify(); + } } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java index 91189c295d..bf72bf795f 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java @@ -18,15 +18,27 @@ */ package co.elastic.apm.agent.httpclient.v4; +import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.junit.AfterClass; import org.junit.BeforeClass; +import org.junit.Test; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doReturn; public class ApacheHttpClientInstrumentationTest extends AbstractHttpClientInstrumentationTest { @@ -49,4 +61,24 @@ protected void performGet(String path) throws Exception { response.close(); } + @Test + public void testPostBodyCapture() throws IOException { + doReturn(Collections.singletonList(WildcardMatcher.matchAll())) + .when(getConfig().getConfig(WebConfiguration.class)).getCaptureClientRequestContentTypes(); + + StringBuilder longString = new StringBuilder(); + for (int i = 0; i < 200; i++) { + longString.append(String.format("line %1$4d\n", i)); + } + HttpPost request = new HttpPost(getBaseUrl() + "/"); + request.setEntity(new InputStreamEntity(new ByteArrayInputStream(longString.toString().getBytes(StandardCharsets.UTF_8)))); + + client.execute(request); + + expectSpan("/") + .withRequestBodySatisfying(body -> { + assertThat(body).endsWith("line 101\nline"); + }).verify(); + } + } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientBasicHttpRequestInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientBasicHttpRequestInstrumentationTest.java index 72a3828ac3..62e8088ce7 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientBasicHttpRequestInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientBasicHttpRequestInstrumentationTest.java @@ -59,4 +59,6 @@ protected void performGet(String path) throws Exception { throw (Exception) e.getTargetException(); } } + + } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java index 499f1d9032..4fdd3904cf 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java @@ -18,17 +18,30 @@ */ package co.elastic.apm.agent.httpclient.v4; +import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; +import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.client.DefaultHttpClient; import org.apache.http.util.EntityUtils; import org.junit.AfterClass; import org.junit.BeforeClass; +import org.junit.Test; +import java.io.ByteArrayInputStream; +import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.util.Collections; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doReturn; public class LegacyApacheHttpClientHttpUriRequestInstrumentationTest extends AbstractHttpClientInstrumentationTest { @@ -48,12 +61,41 @@ public static void close() { @Override protected void performGet(String path) throws Exception { + HttpGet request = new HttpGet(path); + performRequest(request); + } + + private static void performRequest(HttpRequest request) throws Exception { Method execute = client.getClass().getMethod("execute", HttpUriRequest.class); try { - HttpResponse response = (HttpResponse) execute.invoke(client, new HttpGet(path)); + HttpResponse response = (HttpResponse) execute.invoke(client, request); EntityUtils.consume(response.getEntity()); } catch (InvocationTargetException e) { throw (Exception) e.getTargetException(); } } + + @Test + public void testPostBodyCapture() throws Exception { + doReturn(Collections.singletonList(WildcardMatcher.matchAll())) + .when(getConfig().getConfig(WebConfiguration.class)).getCaptureClientRequestContentTypes(); + + StringBuilder longString = new StringBuilder(); + for (int i = 0; i < 200; i++) { + longString.append(String.format("line %1$4d\n", i)); + } + HttpPost request = new HttpPost(getBaseUrl() + "/"); + request.setEntity(new InputStreamEntity(new ByteArrayInputStream(longString.toString().getBytes(StandardCharsets.UTF_8)))); + + ClassLoader cl1 = InputStreamEntity.class.getClassLoader(); + ClassLoader cl = request.getClass().getClassLoader(); + + performRequest(request); + + expectSpan("/") + .withRequestBodySatisfying(body -> { + assertThat(body).endsWith("line 101\nline"); + }).verify(); + } + } diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java new file mode 100644 index 0000000000..745881a515 --- /dev/null +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java @@ -0,0 +1,87 @@ +package co.elastic.apm.agent.httpclient; + +import co.elastic.apm.agent.tracer.Span; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +public class RequestBodyRecordingInputStream extends InputStream { + + public static final int MAX_LENGTH = 1024; + + private final InputStream delegate; + + @Nullable + private Span clientSpan; + + public RequestBodyRecordingInputStream(InputStream delegate, Span clientSpan) { + this.delegate = delegate; + clientSpan.incrementReferences(); + this.clientSpan = clientSpan; + } + + + protected void appendToBody(char b) { + if (clientSpan != null) { + StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); + if (body.length() < MAX_LENGTH) { + body.append(b); + } + } + } + + protected void appendToBody(byte[] b, int off, int len) { + if (clientSpan != null) { + StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); + int remaining = Math.min(MAX_LENGTH - body.length(), len); + for (int i = 0; i < remaining; i++) { + body.append((char) b[off + i]); + } + } + } + + public void releaseSpan() { + if (clientSpan != null) { + clientSpan.decrementReferences(); + clientSpan = null; + } + } + + @Override + public int read() throws IOException { + int character = delegate.read(); + if (character == -1) { + releaseSpan(); + } else { + appendToBody((char) character); + } + return character; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int readBytes = delegate.read(b, off, len); + if (readBytes == -1) { + releaseSpan(); + } else { + appendToBody(b, off, readBytes); + } + return readBytes; + } + + @Override + public int available() throws IOException { + return delegate.available(); + } + + @Override + public void close() throws IOException { + try { + releaseSpan(); + } finally { + delegate.close(); + } + } +} diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java new file mode 100644 index 0000000000..a695313945 --- /dev/null +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java @@ -0,0 +1,90 @@ +package co.elastic.apm.agent.httpclient; + +import co.elastic.apm.agent.tracer.Span; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.io.OutputStream; + +public class RequestBodyRecordingOutputStream extends OutputStream { + + public static final int MAX_LENGTH = 1024; + + private final OutputStream delegate; + + @Nullable + private Span clientSpan; + + public RequestBodyRecordingOutputStream(OutputStream delegate, Span clientSpan) { + this.delegate = delegate; + clientSpan.incrementReferences(); + this.clientSpan = clientSpan; + } + + @Override + public void write(int b) throws IOException { + try { + appendToBody((char) b); + } finally { + delegate.write(b); + } + } + + protected void appendToBody(char b) { + if (clientSpan != null) { + StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); + if (body.length() < MAX_LENGTH) { + body.append(b); + } + } + } + + @Override + public void write(byte[] b) throws IOException { + try { + appendToBody(b, 0, b.length); + } finally { + delegate.write(b); + } + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + try { + appendToBody(b, off, len); + } finally { + delegate.write(b, off, len); + } + } + + protected void appendToBody(byte[] b, int off, int len) { + if (clientSpan != null) { + StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); + int remaining = Math.min(MAX_LENGTH - body.length(), len); + for (int i = 0; i < remaining; i++) { + body.append((char) b[off + i]); + } + } + } + + public void releaseSpan() { + if (clientSpan != null) { + clientSpan.decrementReferences(); + clientSpan = null; + } + } + + @Override + public void close() throws IOException { + try { + releaseSpan(); + } finally { + delegate.close(); + } + } + + @Override + public void flush() throws IOException { + delegate.flush(); + } +} diff --git a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java index 7d0f33372f..3b3ce3f148 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java @@ -26,6 +26,7 @@ import co.elastic.apm.agent.impl.transaction.SpanImpl; import co.elastic.apm.agent.impl.transaction.TraceContextImpl; import co.elastic.apm.agent.impl.transaction.TransactionImpl; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import co.elastic.apm.agent.tracer.util.ResultUtil; import co.elastic.apm.agent.tracer.Outcome; import co.elastic.apm.agent.tracer.TraceState; @@ -52,6 +53,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import static co.elastic.apm.agent.impl.transaction.TraceContextImpl.W3C_TRACE_PARENT_TEXTUAL_HEADER_NAME; import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat; @@ -292,6 +294,8 @@ protected class VerifyBuilder { private boolean traceContextHeaders = true; private boolean requestExecuted = true; + private Consumer requestBodyVerification = null; + private VerifyBuilder(String path) { this.path = path; } @@ -323,6 +327,11 @@ public VerifyBuilder withoutRequestExecuted() { return this; } + public VerifyBuilder withRequestBodySatisfying(Consumer requestBodyVerification) { + this.requestBodyVerification = requestBodyVerification; + return this; + } + public SpanImpl verify() { assertThat(reporter.getFirstSpan(500)).isNotNull(); assertThat(reporter.getSpans()).hasSize(1); @@ -351,6 +360,14 @@ public SpanImpl verify() { assertThat(span).hasOutcome(Outcome.FAILURE); } + StringBuilder reqBody = span.getContext().getHttp().getRequestBody(false); + if (reqBody != null) { + assertThat(reqBody).hasSizeLessThan(1025); //IntakeV2 max allowed is 1024 + } + if (requestBodyVerification != null) { + requestBodyVerification.accept(reqBody == null ? null : reqBody.toString()); + } + if (isAsync()) { assertThat(span).isAsync(); } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java index d7fdb00f91..c88194f9ea 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java @@ -121,6 +121,15 @@ public class WebConfiguration extends ConfigurationOptionProvider { .dynamic(true) .buildWithDefault(Collections.emptyList()); + private final ConfigurationOption> captureClientRequestContentTypes = ConfigurationOption + .builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class) + .key("capture_http_client_request_content_types") + .configurationCategory(HTTP_CATEGORY) + .tags("added[1.50.0]", "internal") + .description("Configures for which content types the HTTP request bodies should be recorded.") + .dynamic(true) + .buildWithDefault(Collections.emptyList()); + public List getIgnoreUrls() { return ignoreUrls.get(); } @@ -141,4 +150,8 @@ public List getCaptureContentTypes() { return captureContentTypes.get(); } + public List getCaptureClientRequestContentTypes() { + return captureClientRequestContentTypes.get(); + } + } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java index d4d614c039..845a315a7e 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java @@ -22,6 +22,9 @@ public interface Http { + @Nullable + StringBuilder getRequestBody(boolean initialize); + /** * URL used for the outgoing HTTP call */ From 6314a66447fab8977e3eef4923e9f9298a1dc84b Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Thu, 20 Jun 2024 10:56:18 +0200 Subject: [PATCH 02/12] added hacked serialization of http request body as otel attribute --- .../agent/report/serialize/DslJsonSerializer.java | 15 +++++++++++---- .../report/serialize/DslJsonSerializerTest.java | 11 +++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) 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 89759bed42..460dec8025 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 @@ -1028,21 +1028,21 @@ private void serializeSpanLinks(List spanLinks) { } private void serializeOTel(SpanImpl span) { - serializeOtel(span, Collections.emptyList()); + serializeOtel(span, Collections.emptyList(), span.getContext().getHttp().getRequestBody(false)); } private void serializeOTel(TransactionImpl transaction) { List profilingCorrelationStackTraceIds = transaction.getProfilingCorrelationStackTraceIds(); synchronized (profilingCorrelationStackTraceIds) { - serializeOtel(transaction, profilingCorrelationStackTraceIds); + serializeOtel(transaction, profilingCorrelationStackTraceIds, null); } } - private void serializeOtel(AbstractSpanImpl span, List profilingStackTraceIds) { + private void serializeOtel(AbstractSpanImpl span, List profilingStackTraceIds, @Nullable CharSequence httpRequestBody) { OTelSpanKind kind = span.getOtelKind(); Map attributes = span.getOtelAttributes(); - boolean hasAttributes = !attributes.isEmpty() || !profilingStackTraceIds.isEmpty(); + boolean hasAttributes = !attributes.isEmpty() || !profilingStackTraceIds.isEmpty() || httpRequestBody != null; boolean hasKind = kind != null; if (hasKind || hasAttributes) { writeFieldName("otel"); @@ -1092,6 +1092,13 @@ private void serializeOtel(AbstractSpanImpl span, List profilingStack } jw.writeByte(ARRAY_END); } + if (httpRequestBody != null) { + if (!isFirstAttrib) { + jw.writeByte(COMMA); + } + writeFieldName("http_client_request_body"); + jw.writeString(httpRequestBody); + } jw.writeByte(OBJECT_END); } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index 34caa3ba05..bd66dec475 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -425,6 +425,17 @@ void testSpanHttpContextSerialization() { assertThat(http.get("status_code").intValue()).isEqualTo(523); } + @Test + void testSpanHttpRequestBodySerialization() { + SpanImpl span = new SpanImpl(tracer); + span.getContext().getHttp().getRequestBody(true).append("foobar"); + + JsonNode spanJson = readJsonString(writer.toJsonString(span)); + JsonNode otel = spanJson.get("otel"); + JsonNode attribs = otel.get("attributes"); + assertThat(attribs.get("http_client_request_body").textValue()).isEqualTo("foobar"); + } + public static boolean[][] getContentCombinations() { return new boolean[][]{ {true, true, true, true}, From 9494c0be113b699373d544ddc3aed9613f66b670 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Thu, 20 Jun 2024 11:47:11 +0200 Subject: [PATCH 03/12] License headers, fix compilation --- ...cheHttpEntityGetContentInstrumentation.java | 18 ++++++++++++++++++ ...ApacheHttpEntityWriteToInstrumentation.java | 18 ++++++++++++++++++ .../v4/helper/RequestBodyCaptureRegistry.java | 18 ++++++++++++++++++ .../RequestBodyRecordingInputStream.java | 18 ++++++++++++++++++ .../RequestBodyRecordingOutputStream.java | 18 ++++++++++++++++++ 5 files changed, 90 insertions(+) diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java index a87d6c4b3f..b08ea36da9 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityGetContentInstrumentation.java @@ -1,3 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package co.elastic.apm.agent.httpclient.v4; import co.elastic.apm.agent.httpclient.RequestBodyRecordingInputStream; diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java index a17015958a..0b06c8b9fc 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpEntityWriteToInstrumentation.java @@ -1,3 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package co.elastic.apm.agent.httpclient.v4; import co.elastic.apm.agent.httpclient.RequestBodyRecordingOutputStream; diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java index 2314192411..e972e3209d 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java @@ -1,3 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package co.elastic.apm.agent.httpclient.v4.helper; import co.elastic.apm.agent.httpclient.v4.ApacheHttpClientInstrumentation; diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java index 745881a515..a3f3a43187 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java @@ -1,3 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package co.elastic.apm.agent.httpclient; import co.elastic.apm.agent.tracer.Span; diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java index a695313945..fb25a70e11 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java @@ -1,3 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package co.elastic.apm.agent.httpclient; import co.elastic.apm.agent.tracer.Span; From 418baf0724b009d8b89cfdbd061b46e1cf4b7764 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Thu, 20 Jun 2024 12:23:25 +0200 Subject: [PATCH 04/12] fix compilation --- .../apm/agent/tracer/configuration/WebConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java index c88194f9ea..14f5c71d90 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java @@ -128,7 +128,7 @@ public class WebConfiguration extends ConfigurationOptionProvider { .tags("added[1.50.0]", "internal") .description("Configures for which content types the HTTP request bodies should be recorded.") .dynamic(true) - .buildWithDefault(Collections.emptyList()); + .buildWithDefault(Collections.emptyList()); public List getIgnoreUrls() { return ignoreUrls.get(); From 1181ced5180594e4db715d86bdc1105486131a10 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Mon, 24 Jun 2024 09:22:50 +0200 Subject: [PATCH 05/12] Fix apache http async client test --- .../httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java index c6d5302add..ba5a43a361 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java @@ -169,7 +169,7 @@ public void cancelled() { expectSpan("/") .withRequestBodySatisfying(body -> { - // assertThat(body).endsWith("line 101\nline"); + assertThat(body).endsWith("line 101\nline"); }).verify(); } } From b0f1e094e60ca917b5bcf139e2940b6a7ae12a2c Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Tue, 16 Jul 2024 13:05:36 +0200 Subject: [PATCH 06/12] Added API for enabling request body capture. --- .../agent/impl/context/BodyCaptureImpl.java | 136 ++++++++++++++++++ .../apm/agent/impl/context/HttpImpl.java | 15 +- .../configuration/WebConfiguration.java | 2 + .../agent/tracer/metadata/BodyCapture.java | 47 ++++++ .../apm/agent/tracer/metadata/Http.java | 5 +- 5 files changed, 191 insertions(+), 14 deletions(-) create mode 100644 apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java create mode 100644 apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/BodyCapture.java diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java new file mode 100644 index 0000000000..443d17cce5 --- /dev/null +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java @@ -0,0 +1,136 @@ +package co.elastic.apm.agent.impl.context; + +import co.elastic.apm.agent.objectpool.Resetter; +import co.elastic.apm.agent.objectpool.impl.QueueBasedObjectPool; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; +import co.elastic.apm.agent.tracer.metadata.BodyCapture; +import co.elastic.apm.agent.tracer.pooling.Allocator; +import co.elastic.apm.agent.tracer.pooling.ObjectPool; +import co.elastic.apm.agent.tracer.pooling.Recyclable; +import org.jctools.queues.atomic.MpmcAtomicArrayQueue; + +import javax.annotation.Nullable; +import java.nio.ByteBuffer; + +public class BodyCaptureImpl implements BodyCapture, Recyclable { + private static final ObjectPool BYTE_BUFFER_POOL = QueueBasedObjectPool.of(new MpmcAtomicArrayQueue(128), false, + new Allocator() { + @Override + public ByteBuffer createInstance() { + return ByteBuffer.allocate(WebConfiguration.MAX_BODY_CAPTURE_BYTES); + } + }, + new Resetter() { + @Override + public void recycle(ByteBuffer object) { + object.clear(); + } + }); + + private enum CaptureState { + NOT_ELIGIBLE, + ELIGIBLE, + STARTED + } + + private CaptureState state; + + private StringBuilder charset; + + /** + * The number of bytes to capture. + */ + private int numBytesToCapture; + + @Nullable + private ByteBuffer bodyBuffer; + + BodyCaptureImpl() { + charset = new StringBuilder(); + resetState(); + } + + @Override + public void resetState() { + state = CaptureState.NOT_ELIGIBLE; + charset.setLength(0); + if (bodyBuffer != null) { + BYTE_BUFFER_POOL.recycle(bodyBuffer); + } + } + + @Override + public void markEligibleForCapturing() { + if (state == CaptureState.NOT_ELIGIBLE) { + state = CaptureState.ELIGIBLE; + } + } + + @Override + public boolean isEligibleForCapturing() { + return state != CaptureState.NOT_ELIGIBLE; + } + + @Override + public boolean startCapture(@Nullable String charset, int numBytesToCapture) { + if (numBytesToCapture > WebConfiguration.MAX_BODY_CAPTURE_BYTES) { + throw new IllegalArgumentException("Capturing " + numBytesToCapture + " bytes is not supported"); + } + if (state == CaptureState.ELIGIBLE) { + if (charset != null) { + this.charset.append(charset); + } + this.numBytesToCapture = numBytesToCapture; + state = CaptureState.STARTED; + return true; + } + return false; + } + + private void acquireBodyBufferIfRequired() { + if (state != CaptureState.STARTED) { + throw new IllegalStateException("Capturing has not been started!"); + } + if (bodyBuffer == null) { + bodyBuffer = BYTE_BUFFER_POOL.createInstance(); + } + } + + @Override + public void append(byte b) { + acquireBodyBufferIfRequired(); + if (!isFull()) { + bodyBuffer.put(b); + } + } + + @Override + public void append(byte[] b, int offset, int len) { + acquireBodyBufferIfRequired(); + int remaining = numBytesToCapture - bodyBuffer.position(); + if (remaining > 0) { + bodyBuffer.put(b, offset, Math.min(len, remaining)); + } + } + + @Override + public boolean isFull() { + if (bodyBuffer == null) { + return false; + } + return bodyBuffer.position() < numBytesToCapture; + } + + @Nullable + public CharSequence getCharset() { + if (charset.length() == 0) { + return null; + } + return charset; + } + + @Nullable + public ByteBuffer getBody() { + return bodyBuffer; + } +} diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java index 4c123d2bce..9b6ca5f0a1 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java @@ -30,6 +30,8 @@ public class HttpImpl implements Recyclable, Http { */ private final UrlImpl url = new UrlImpl(); + private final BodyCaptureImpl requestBody = new BodyCaptureImpl(); + /** * HTTP method used by this HTTP outgoing span */ @@ -69,11 +71,7 @@ public int getStatusCode() { } @Override - @Nullable - public StringBuilder getRequestBody(boolean initialize) { - if (initialize && requestBody == null) { - requestBody = new StringBuilder(); - } + public BodyCaptureImpl getRequestBody() { return requestBody; } @@ -100,9 +98,9 @@ public HttpImpl withStatusCode(int statusCode) { @Override public void resetState() { url.resetState(); + requestBody.resetState(); method = null; statusCode = 0; - requestBody = null; } public boolean hasContent() { @@ -111,9 +109,4 @@ public boolean hasContent() { statusCode > 0; } - public void copyFrom(HttpImpl other) { - url.copyFrom(other.url); - method = other.method; - statusCode = other.statusCode; - } } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java index 14f5c71d90..38fa5b0451 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java @@ -29,6 +29,8 @@ public class WebConfiguration extends ConfigurationOptionProvider { + public static final int MAX_BODY_CAPTURE_BYTES = 1024; + private static final String HTTP_CATEGORY = "HTTP"; private final ConfigurationOption> captureContentTypes = ConfigurationOption diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/BodyCapture.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/BodyCapture.java new file mode 100644 index 0000000000..ef8df98de0 --- /dev/null +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/BodyCapture.java @@ -0,0 +1,47 @@ +package co.elastic.apm.agent.tracer.metadata; + +import javax.annotation.Nullable; + +public interface BodyCapture { + + /** + * Requests that the body for this span may be captured. + * Whether it is actually captured may depend on further details not known yet when this method is called + * (e.g. the Content-Type header). + */ + void markEligibleForCapturing(); + + /** + * @return true, if {@link #markEligibleForCapturing()} was called for this span. + */ + boolean isEligibleForCapturing(); + + /** + * This method acts as a protection mechanism so that only one instrumentation tries to capture the body. + * It returns true, if the calling instrumentation shall start adding body byte via {@link #append(byte)}. + *

+ * For this to happen, {@link #markEligibleForCapturing()} must have been called first. + *

+ * After {@link #startCapture(String, int)} has returned true once, subsequent calls will return false. + * So for example if instrumentation A and B are active for the same span, only the first one will actually be capturing the body, + * because {@link #startCapture(String, int)} only returns true once. + * + * @param charset the charset (if available) with which the request-body is encoded. + * @param numBytesToCapture the number of bytes to capture, to configure the limit of the internal buffer + * + * @return true, if the calling instrumentation should be capturing the body (by calling {@link #append(byte)} + */ + boolean startCapture(@Nullable String charset, int numBytesToCapture); + + void append(byte b); + + void append(byte[] b, int offset, int len); + + /** + * Checks if the limit number of bytes to capture has been reached. In this case future append calls would be a no-op. + * If this is the case, the caller can consider releasing the reference to the span to prevent potential memory leaks. + * + * @return true, if the maximum number of bytes supported has already been captured + */ + boolean isFull(); +} diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java index 845a315a7e..aad5ad3d85 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/metadata/Http.java @@ -22,9 +22,6 @@ public interface Http { - @Nullable - StringBuilder getRequestBody(boolean initialize); - /** * URL used for the outgoing HTTP call */ @@ -42,4 +39,6 @@ public interface Http { @Nullable String getMethod(); + + BodyCapture getRequestBody(); } From 417a7353d68da329441ec4c925ef302fc0457943 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 17 Jul 2024 10:36:51 +0200 Subject: [PATCH 07/12] Centralized logic and tests to be reusable across clients --- .../apm/agent/impl/context/HttpImpl.java | 3 - .../report/serialize/DslJsonSerializer.java | 51 ++++++++++-- .../serialize/DslJsonSerializerTest.java | 14 +++- .../AbstractApacheHttpClientAsyncAdvice.java | 3 +- .../v4/ApacheHttpClientInstrumentation.java | 2 +- ...LegacyApacheHttpClientInstrumentation.java | 4 +- .../v4/helper/RequestBodyCaptureRegistry.java | 16 ++-- ...cheHttpAsyncClientInstrumentationTest.java | 77 ++++++++----------- .../ApacheHttpClientInstrumentationTest.java | 33 +++----- ...ientHttpUriRequestInstrumentationTest.java | 38 +++------ .../agent/httpclient/HttpClientHelper.java | 51 +++++++++++- .../RequestBodyRecordingInputStream.java | 23 +++--- .../RequestBodyRecordingOutputStream.java | 22 +++--- ...AbstractHttpClientInstrumentationTest.java | 52 ++++++++++--- .../httpclient/HttpClientHelperTest.java | 13 ++++ .../configuration/WebConfiguration.java | 18 +++-- 16 files changed, 257 insertions(+), 163 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java index 9b6ca5f0a1..67a7169682 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/HttpImpl.java @@ -38,9 +38,6 @@ public class HttpImpl implements Recyclable, Http { @Nullable private String method; - @Nullable - private StringBuilder requestBody; - /** * Status code of the response */ 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 460dec8025..bf67ac56f4 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 @@ -18,8 +18,23 @@ */ package co.elastic.apm.agent.report.serialize; -import co.elastic.apm.agent.impl.context.*; +import co.elastic.apm.agent.impl.context.AbstractContextImpl; +import co.elastic.apm.agent.impl.context.BodyCaptureImpl; +import co.elastic.apm.agent.impl.context.CloudOriginImpl; +import co.elastic.apm.agent.impl.context.DbImpl; +import co.elastic.apm.agent.impl.context.DestinationImpl; +import co.elastic.apm.agent.impl.context.Headers; import co.elastic.apm.agent.impl.context.HttpImpl; +import co.elastic.apm.agent.impl.context.MessageImpl; +import co.elastic.apm.agent.impl.context.RequestImpl; +import co.elastic.apm.agent.impl.context.ResponseImpl; +import co.elastic.apm.agent.impl.context.ServiceOriginImpl; +import co.elastic.apm.agent.impl.context.ServiceTargetImpl; +import co.elastic.apm.agent.impl.context.SocketImpl; +import co.elastic.apm.agent.impl.context.SpanContextImpl; +import co.elastic.apm.agent.impl.context.TransactionContextImpl; +import co.elastic.apm.agent.impl.context.UrlImpl; +import co.elastic.apm.agent.impl.context.UserImpl; import co.elastic.apm.agent.impl.error.ErrorCaptureImpl; import co.elastic.apm.agent.impl.metadata.Agent; import co.elastic.apm.agent.impl.metadata.CloudProviderInfo; @@ -33,16 +48,26 @@ import co.elastic.apm.agent.impl.metadata.ServiceImpl; import co.elastic.apm.agent.impl.metadata.SystemInfo; import co.elastic.apm.agent.impl.stacktrace.StacktraceConfigurationImpl; -import co.elastic.apm.agent.impl.transaction.*; +import co.elastic.apm.agent.impl.transaction.AbstractSpanImpl; +import co.elastic.apm.agent.impl.transaction.Composite; +import co.elastic.apm.agent.impl.transaction.DroppedSpanStats; +import co.elastic.apm.agent.impl.transaction.FaasImpl; +import co.elastic.apm.agent.impl.transaction.FaasTriggerImpl; +import co.elastic.apm.agent.impl.transaction.IdImpl; +import co.elastic.apm.agent.impl.transaction.OTelSpanKind; +import co.elastic.apm.agent.impl.transaction.SpanCount; import co.elastic.apm.agent.impl.transaction.SpanImpl; -import co.elastic.apm.agent.tracer.metrics.Labels; +import co.elastic.apm.agent.impl.transaction.StackFrame; +import co.elastic.apm.agent.impl.transaction.TraceContextImpl; +import co.elastic.apm.agent.impl.transaction.TransactionImpl; import co.elastic.apm.agent.report.ApmServerClient; import co.elastic.apm.agent.sdk.internal.collections.LongList; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.tracer.metadata.PotentiallyMultiValuedMap; -import co.elastic.apm.agent.tracer.pooling.Recyclable; import co.elastic.apm.agent.tracer.metrics.DslJsonUtil; +import co.elastic.apm.agent.tracer.metrics.Labels; +import co.elastic.apm.agent.tracer.pooling.Recyclable; import com.dslplatform.json.BoolConverter; import com.dslplatform.json.DslJson; import com.dslplatform.json.JsonWriter; @@ -54,6 +79,7 @@ import java.io.File; import java.io.IOException; import java.io.OutputStream; +import java.nio.ByteBuffer; import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -1028,7 +1054,22 @@ private void serializeSpanLinks(List spanLinks) { } private void serializeOTel(SpanImpl span) { - serializeOtel(span, Collections.emptyList(), span.getContext().getHttp().getRequestBody(false)); + serializeOtel(span, Collections.emptyList(), requestBodyToString(span.getContext().getHttp().getRequestBody())); + } + + @Nullable + private CharSequence requestBodyToString(BodyCaptureImpl requestBody) { + //TODO: perform proper, charset aware conversion to string + ByteBuffer buffer = requestBody.getBody(); + if (buffer == null || buffer.position() == 0) { + return null; + } + buffer.flip(); + StringBuilder result = new StringBuilder(); + while (buffer.hasRemaining()) { + result.append((char) buffer.get()); + } + return result; } private void serializeOTel(TransactionImpl transaction) { diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index bd66dec475..820ed201da 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -27,6 +27,7 @@ import co.elastic.apm.agent.impl.TextHeaderMapAccessor; import co.elastic.apm.agent.impl.baggage.BaggageImpl; import co.elastic.apm.agent.impl.context.AbstractContextImpl; +import co.elastic.apm.agent.impl.context.BodyCaptureImpl; import co.elastic.apm.agent.impl.context.DestinationImpl; import co.elastic.apm.agent.impl.context.Headers; import co.elastic.apm.agent.impl.context.RequestImpl; @@ -46,8 +47,13 @@ import co.elastic.apm.agent.impl.sampling.ConstantSampler; import co.elastic.apm.agent.impl.sampling.Sampler; import co.elastic.apm.agent.impl.stacktrace.StacktraceConfigurationImpl; -import co.elastic.apm.agent.impl.transaction.*; +import co.elastic.apm.agent.impl.transaction.AbstractSpanImpl; +import co.elastic.apm.agent.impl.transaction.IdImpl; +import co.elastic.apm.agent.impl.transaction.OTelSpanKind; import co.elastic.apm.agent.impl.transaction.SpanImpl; +import co.elastic.apm.agent.impl.transaction.StackFrame; +import co.elastic.apm.agent.impl.transaction.TraceContextImpl; +import co.elastic.apm.agent.impl.transaction.TransactionImpl; import co.elastic.apm.agent.report.ApmServerClient; import co.elastic.apm.agent.sdk.internal.collections.LongList; import co.elastic.apm.agent.sdk.internal.util.IOUtils; @@ -428,7 +434,11 @@ void testSpanHttpContextSerialization() { @Test void testSpanHttpRequestBodySerialization() { SpanImpl span = new SpanImpl(tracer); - span.getContext().getHttp().getRequestBody(true).append("foobar"); + + BodyCaptureImpl bodyCapture = span.getContext().getHttp().getRequestBody(); + bodyCapture.markEligibleForCapturing(); + bodyCapture.startCapture("utf-8", 50); + bodyCapture.append("foobar".getBytes(StandardCharsets.UTF_8), 0, 6); JsonNode spanJson = readJsonString(writer.toJsonString(span)); JsonNode otel = spanJson.get("otel"); diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAsyncAdvice.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAsyncAdvice.java index f3b0f234f8..a4fd57dbff 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAsyncAdvice.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/src/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpClientAsyncAdvice.java @@ -20,8 +20,8 @@ import co.elastic.apm.agent.httpclient.HttpClientHelper; -import co.elastic.apm.agent.tracer.TraceState; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.TraceState; import co.elastic.apm.agent.tracer.Tracer; public abstract class AbstractApacheHttpClientAsyncAdvice { @@ -40,6 +40,7 @@ public static activeContext = tracer.currentContext(); Span span = activeContext.createExitSpan(); if (span != null) { + span.getContext().getHttp().getRequestBody().markEligibleForCapturing(); span.withType(HttpClientHelper.EXTERNAL_TYPE) .withSubtype(HttpClientHelper.HTTP_SUBTYPE) .withSync(false) diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java index 06b2c70c6f..2bc725c95b 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentation.java @@ -60,7 +60,7 @@ public static class ApacheHttpClient4Advice extends AbstractApacheHttpClientAdvi public static Object onBeforeExecute(@Advice.Argument(0) HttpRoute route, @Advice.Argument(1) HttpRequestWrapper request) throws URISyntaxException { Span span = startSpan(tracer, adapter, request, route.getTargetHost(), RequestHeaderAccessor.INSTANCE); - RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, span); + RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, tracer.getActive()); return span; } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java index 9c20886e38..21fe4ad545 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java @@ -21,9 +21,9 @@ import co.elastic.apm.agent.httpclient.HttpClientHelper; import co.elastic.apm.agent.httpclient.v4.helper.RequestBodyCaptureRegistry; import co.elastic.apm.agent.httpclient.v4.helper.RequestHeaderAccessor; -import co.elastic.apm.agent.tracer.TraceState; import co.elastic.apm.agent.tracer.Outcome; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.TraceState; import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; @@ -102,11 +102,11 @@ public static Object onBeforeExecute(@Advice.Argument(0) @Nullable HttpHost host span = HttpClientHelper.startHttpClientSpan(activeContext, method, uri, hostName); if (span != null) { - RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, span); span.activate(); } } + RequestBodyCaptureRegistry.potentiallyCaptureRequestBody(request, tracer.getActive()); tracer.currentContext().propagateContext(request, RequestHeaderAccessor.INSTANCE, RequestHeaderAccessor.INSTANCE); return span; } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java index e972e3209d..009cc1fe9b 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/RequestBodyCaptureRegistry.java @@ -18,20 +18,17 @@ */ package co.elastic.apm.agent.httpclient.v4.helper; -import co.elastic.apm.agent.httpclient.v4.ApacheHttpClientInstrumentation; -import co.elastic.apm.agent.httpclient.v4.BaseApacheHttpClientInstrumentation; +import co.elastic.apm.agent.httpclient.HttpClientHelper; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.sdk.state.GlobalState; +import co.elastic.apm.agent.tracer.AbstractSpan; import co.elastic.apm.agent.tracer.GlobalTracer; import co.elastic.apm.agent.tracer.Span; -import co.elastic.apm.agent.tracer.Tracer; -import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import co.elastic.apm.agent.tracer.reference.ReferenceCountedMap; import org.apache.http.HttpEntity; import org.apache.http.HttpEntityEnclosingRequest; import org.apache.http.HttpRequest; -import org.apache.http.client.methods.HttpRequestWrapper; import javax.annotation.Nullable; @@ -39,8 +36,6 @@ public class RequestBodyCaptureRegistry { private static final Logger logger = LoggerFactory.getLogger(RequestBodyCaptureRegistry.class); - private static final Tracer tracer = GlobalTracer.get(); - @GlobalState public static class MapHolder { private static final ReferenceCountedMap> entityToClientSpan = GlobalTracer.get().newReferenceCountedMap(); @@ -57,17 +52,16 @@ public static Span removeSpanFor(Object entity) { } - public static void potentiallyCaptureRequestBody(HttpRequest request, @Nullable Span span) { - if (span != null && span.isSampled() && !tracer.getConfig(WebConfiguration.class).getCaptureClientRequestContentTypes().isEmpty()) { + public static void potentiallyCaptureRequestBody(HttpRequest request, @Nullable AbstractSpan span) { + if (HttpClientHelper.startRequestBodyCapture(span, request, RequestHeaderAccessor.INSTANCE)) { if (request instanceof HttpEntityEnclosingRequest) { HttpEntity entity = ((HttpEntityEnclosingRequest) request).getEntity(); if (entity != null) { logger.debug("Enabling request capture for entity {}() for span {}", entity.getClass().getName(), System.identityHashCode(entity), span); - MapHolder.captureBodyFor(entity, span); + MapHolder.captureBodyFor(entity, (Span) span); } else { logger.debug("HttpEntity is null for span {}", span); } - } else { logger.debug("Not capturing request body because {} is not an HttpEntityEnclosingRequest", request.getClass().getName()); } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java index ba5a43a361..b2698ebc2e 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpAsyncClientInstrumentationTest.java @@ -18,11 +18,9 @@ */ package co.elastic.apm.agent.httpclient.v4; -import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest; import co.elastic.apm.agent.impl.transaction.SpanImpl; import co.elastic.apm.agent.tracer.Outcome; -import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import org.apache.http.HttpResponse; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpGet; @@ -40,14 +38,10 @@ import java.io.ByteArrayInputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.doReturn; public class ApacheHttpAsyncClientInstrumentationTest extends AbstractHttpClientInstrumentationTest { @@ -98,6 +92,39 @@ public void cancelled() { responseFuture.get(); } + @Override + protected boolean isBodyCapturingSupported() { + return true; + } + + @Override + protected void performPost(String path, byte[] data, String contentTypeHeader) throws Exception { + final CompletableFuture responseFuture = new CompletableFuture<>(); + + HttpClientContext httpClientContext = HttpClientContext.create(); + HttpPost request = new HttpPost(path); + request.setEntity(new InputStreamEntity(new ByteArrayInputStream(data))); + request.setHeader("Content-Type", contentTypeHeader); + client.execute(request, httpClientContext, new FutureCallback<>() { + @Override + public void completed(HttpResponse result) { + responseFuture.complete(result); + } + + @Override + public void failed(Exception ex) { + responseFuture.completeExceptionally(ex); + } + + @Override + public void cancelled() { + responseFuture.cancel(true); + } + }); + + responseFuture.get(); + } + @Test public void testSpanFinishOnEarlyException() throws Exception { @@ -134,42 +161,4 @@ public void testSpanWithIllegalProtocol() throws Exception { Assertions.assertThat(reporter.getSpans()).hasSize(1); } - @Test - public void testPostBodyCapture() throws IOException, ExecutionException, InterruptedException { - doReturn(Collections.singletonList(WildcardMatcher.matchAll())) - .when(getConfig().getConfig(WebConfiguration.class)).getCaptureClientRequestContentTypes(); - - StringBuilder longString = new StringBuilder(); - for (int i = 0; i < 200; i++) { - longString.append(String.format("line %1$4d\n", i)); - } - HttpPost request = new HttpPost(getBaseUrl() + "/"); - request.setEntity(new InputStreamEntity(new ByteArrayInputStream(longString.toString().getBytes(StandardCharsets.UTF_8)))); - - final CompletableFuture responseFuture = new CompletableFuture<>(); - - client.execute(request, new FutureCallback<>() { - @Override - public void completed(HttpResponse result) { - responseFuture.complete(result); - } - - @Override - public void failed(Exception ex) { - responseFuture.completeExceptionally(ex); - } - - @Override - public void cancelled() { - responseFuture.cancel(true); - } - }); - responseFuture.get(); - - - expectSpan("/") - .withRequestBodySatisfying(body -> { - assertThat(body).endsWith("line 101\nline"); - }).verify(); - } } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java index bf72bf795f..cfa20bd6a0 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/ApacheHttpClientInstrumentationTest.java @@ -18,9 +18,7 @@ */ package co.elastic.apm.agent.httpclient.v4; -import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest; -import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; @@ -29,16 +27,9 @@ import org.apache.http.impl.client.HttpClients; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Test; import java.io.ByteArrayInputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.Collections; -import java.util.stream.Collectors; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.doReturn; public class ApacheHttpClientInstrumentationTest extends AbstractHttpClientInstrumentationTest { @@ -61,24 +52,18 @@ protected void performGet(String path) throws Exception { response.close(); } - @Test - public void testPostBodyCapture() throws IOException { - doReturn(Collections.singletonList(WildcardMatcher.matchAll())) - .when(getConfig().getConfig(WebConfiguration.class)).getCaptureClientRequestContentTypes(); + @Override + protected boolean isBodyCapturingSupported() { + return true; + } - StringBuilder longString = new StringBuilder(); - for (int i = 0; i < 200; i++) { - longString.append(String.format("line %1$4d\n", i)); - } - HttpPost request = new HttpPost(getBaseUrl() + "/"); - request.setEntity(new InputStreamEntity(new ByteArrayInputStream(longString.toString().getBytes(StandardCharsets.UTF_8)))); + @Override + protected void performPost(String path, byte[] content, String contentTypeHeader) throws Exception { + HttpPost request = new HttpPost(path); + request.setEntity(new InputStreamEntity(new ByteArrayInputStream(content))); + request.setHeader("Content-Type", contentTypeHeader); client.execute(request); - - expectSpan("/") - .withRequestBodySatisfying(body -> { - assertThat(body).endsWith("line 101\nline"); - }).verify(); } } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java index 4fdd3904cf..16c56da974 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/test/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientHttpUriRequestInstrumentationTest.java @@ -18,9 +18,7 @@ */ package co.elastic.apm.agent.httpclient.v4; -import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest; -import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpGet; @@ -31,17 +29,10 @@ import org.apache.http.util.EntityUtils; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Test; import java.io.ByteArrayInputStream; -import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.nio.charset.StandardCharsets; -import java.util.Collections; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.doReturn; public class LegacyApacheHttpClientHttpUriRequestInstrumentationTest extends AbstractHttpClientInstrumentationTest { @@ -75,27 +66,16 @@ private static void performRequest(HttpRequest request) throws Exception { } } - @Test - public void testPostBodyCapture() throws Exception { - doReturn(Collections.singletonList(WildcardMatcher.matchAll())) - .when(getConfig().getConfig(WebConfiguration.class)).getCaptureClientRequestContentTypes(); - - StringBuilder longString = new StringBuilder(); - for (int i = 0; i < 200; i++) { - longString.append(String.format("line %1$4d\n", i)); - } - HttpPost request = new HttpPost(getBaseUrl() + "/"); - request.setEntity(new InputStreamEntity(new ByteArrayInputStream(longString.toString().getBytes(StandardCharsets.UTF_8)))); - - ClassLoader cl1 = InputStreamEntity.class.getClassLoader(); - ClassLoader cl = request.getClass().getClassLoader(); + @Override + protected boolean isBodyCapturingSupported() { + return true; + } + @Override + protected void performPost(String path, byte[] content, String contentTypeHeader) throws Exception { + HttpPost request = new HttpPost(path); + request.setEntity(new InputStreamEntity(new ByteArrayInputStream(content))); + request.setHeader("Content-Type", contentTypeHeader); performRequest(request); - - expectSpan("/") - .withRequestBodySatisfying(body -> { - assertThat(body).endsWith("line 101\nline"); - }).verify(); } - } diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/HttpClientHelper.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/HttpClientHelper.java index 2d62702437..61bb7e7e66 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/HttpClientHelper.java +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/HttpClientHelper.java @@ -18,16 +18,26 @@ */ package co.elastic.apm.agent.httpclient; +import co.elastic.apm.agent.common.util.WildcardMatcher; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; -import co.elastic.apm.agent.tracer.TraceState; +import co.elastic.apm.agent.tracer.AbstractSpan; +import co.elastic.apm.agent.tracer.GlobalTracer; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.TraceState; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; +import co.elastic.apm.agent.tracer.dispatch.TextHeaderGetter; import javax.annotation.Nullable; import java.net.URI; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public class HttpClientHelper { + private static final Pattern CHARSET_EXTRACTOR = Pattern.compile(";\\s*charset\\s*=\\s*((\"[^\"]+)|([^;\\s]+))"); + private static final Logger logger = LoggerFactory.getLogger(HttpClientHelper.class); public static final String EXTERNAL_TYPE = "external"; @@ -54,6 +64,9 @@ public static Span startHttpClientSpan(TraceState activeContext, String me @Nullable String scheme, @Nullable CharSequence hostName, int port) { Span span = activeContext.createExitSpan(); if (span != null) { + if (span.isSampled()) { + span.getContext().getHttp().getRequestBody().markEligibleForCapturing(); + } updateHttpSpanNameAndContext(span, method, uri, scheme, hostName, port); } if (logger.isTraceEnabled()) { @@ -62,6 +75,42 @@ public static Span startHttpClientSpan(TraceState activeContext, String me return span; } + public static boolean startRequestBodyCapture(@Nullable AbstractSpan abstractSpan, R request, TextHeaderGetter headerGetter) { + if (!(abstractSpan instanceof Span)) { + return false; + } + Span span = (Span) abstractSpan; + if (!span.getContext().getHttp().getRequestBody().isEligibleForCapturing()) { + return false; + } + WebConfiguration webConfig = GlobalTracer.get().getConfig(WebConfiguration.class); + int byteCount = webConfig.getCaptureClientRequestBytes(); + if (byteCount == 0) { + return false; + } + List contentTypes = webConfig.getCaptureContentTypes(); + String contentTypeHeader = headerGetter.getFirstHeader("Content-Type", request); + if (contentTypeHeader == null) { + contentTypeHeader = ""; + } + if (WildcardMatcher.anyMatch(contentTypes, contentTypeHeader) == null) { + return false; + } + String charset = extractCharsetFromContentType(contentTypeHeader); + return span.getContext().getHttp().getRequestBody().startCapture(charset, byteCount); + } + + //Visible for testing + @Nullable + static String extractCharsetFromContentType(String contentTypeHeader) { + Matcher matcher = CHARSET_EXTRACTOR.matcher(contentTypeHeader); + if (matcher.find()) { + String potentiallyQuotedCharset = matcher.group(1); + return potentiallyQuotedCharset.replace("\"", ""); + } + return null; + } + public static void updateHttpSpanNameAndContext(Span span, String method, @Nullable String uri, String scheme, CharSequence hostName, int port) { span.withType(EXTERNAL_TYPE) .withSubtype(HTTP_SUBTYPE) diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java index a3f3a43187..f8e228eaf8 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingInputStream.java @@ -19,16 +19,14 @@ package co.elastic.apm.agent.httpclient; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.metadata.BodyCapture; import javax.annotation.Nullable; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; public class RequestBodyRecordingInputStream extends InputStream { - public static final int MAX_LENGTH = 1024; - private final InputStream delegate; @Nullable @@ -41,21 +39,22 @@ public RequestBodyRecordingInputStream(InputStream delegate, Span clientSpan) } - protected void appendToBody(char b) { + protected void appendToBody(byte b) { if (clientSpan != null) { - StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); - if (body.length() < MAX_LENGTH) { - body.append(b); + BodyCapture requestBody = clientSpan.getContext().getHttp().getRequestBody(); + requestBody.append(b); + if (requestBody.isFull()) { + releaseSpan(); } } } protected void appendToBody(byte[] b, int off, int len) { if (clientSpan != null) { - StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); - int remaining = Math.min(MAX_LENGTH - body.length(), len); - for (int i = 0; i < remaining; i++) { - body.append((char) b[off + i]); + BodyCapture requestBody = clientSpan.getContext().getHttp().getRequestBody(); + requestBody.append(b, off, len); + if (requestBody.isFull()) { + releaseSpan(); } } } @@ -73,7 +72,7 @@ public int read() throws IOException { if (character == -1) { releaseSpan(); } else { - appendToBody((char) character); + appendToBody((byte) character); } return character; } diff --git a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java index fb25a70e11..aaf640c1d3 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java +++ b/apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/agent/httpclient/RequestBodyRecordingOutputStream.java @@ -19,6 +19,7 @@ package co.elastic.apm.agent.httpclient; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.metadata.BodyCapture; import javax.annotation.Nullable; import java.io.IOException; @@ -26,8 +27,6 @@ public class RequestBodyRecordingOutputStream extends OutputStream { - public static final int MAX_LENGTH = 1024; - private final OutputStream delegate; @Nullable @@ -42,17 +41,18 @@ public RequestBodyRecordingOutputStream(OutputStream delegate, Span clientSpa @Override public void write(int b) throws IOException { try { - appendToBody((char) b); + appendToBody((byte) b); } finally { delegate.write(b); } } - protected void appendToBody(char b) { + protected void appendToBody(byte b) { if (clientSpan != null) { - StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); - if (body.length() < MAX_LENGTH) { - body.append(b); + BodyCapture body = clientSpan.getContext().getHttp().getRequestBody(); + body.append(b); + if (body.isFull()) { + releaseSpan(); } } } @@ -77,10 +77,10 @@ public void write(byte[] b, int off, int len) throws IOException { protected void appendToBody(byte[] b, int off, int len) { if (clientSpan != null) { - StringBuilder body = clientSpan.getContext().getHttp().getRequestBody(true); - int remaining = Math.min(MAX_LENGTH - body.length(), len); - for (int i = 0; i < remaining; i++) { - body.append((char) b[off + i]); + BodyCapture body = clientSpan.getContext().getHttp().getRequestBody(); + body.append(b, off, len); + if (body.isFull()) { + releaseSpan(); } } } diff --git a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java index 3b3ce3f148..788757662f 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java +++ b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/AbstractHttpClientInstrumentationTest.java @@ -21,17 +21,18 @@ import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.configuration.CoreConfigurationImpl; import co.elastic.apm.agent.impl.TextHeaderMapAccessor; +import co.elastic.apm.agent.impl.context.BodyCaptureImpl; import co.elastic.apm.agent.impl.context.DestinationImpl; import co.elastic.apm.agent.impl.context.HttpImpl; import co.elastic.apm.agent.impl.transaction.SpanImpl; import co.elastic.apm.agent.impl.transaction.TraceContextImpl; import co.elastic.apm.agent.impl.transaction.TransactionImpl; -import co.elastic.apm.agent.tracer.configuration.WebConfiguration; -import co.elastic.apm.agent.tracer.util.ResultUtil; import co.elastic.apm.agent.tracer.Outcome; -import co.elastic.apm.agent.tracer.TraceState; import co.elastic.apm.agent.tracer.Scope; +import co.elastic.apm.agent.tracer.TraceState; +import co.elastic.apm.agent.tracer.configuration.WebConfiguration; import co.elastic.apm.agent.tracer.dispatch.TextHeaderGetter; +import co.elastic.apm.agent.tracer.util.ResultUtil; import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; import com.github.tomakehurst.wiremock.http.HttpHeader; @@ -46,6 +47,8 @@ import org.junit.Test; import javax.annotation.Nullable; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -116,6 +119,29 @@ public void testHttpCall() { expectSpan(path).verify(); } + @Test + public void testPostBodyCapture() throws Exception { + if (!isBodyCapturingSupported()) { + return; + } + doReturn(5).when(config.getConfig(WebConfiguration.class)).getCaptureClientRequestBytes(); + byte[] content = "Hello World!".getBytes(StandardCharsets.UTF_8); + String path = "/"; + performPost(getBaseUrl() + path, content, "text/plain; charset=utf-8"); + expectSpan(path) + .withRequestBodySatisfying(body -> { + ByteBuffer buffer = body.getBody(); + assertThat(buffer).isNotNull(); + int numBytes = buffer.position(); + buffer.position(0); + byte[] contentBytes = new byte[numBytes]; + buffer.get(contentBytes); + assertThat(contentBytes).isEqualTo("Hello".getBytes(StandardCharsets.UTF_8)); + assertThat(Objects.toString(body.getCharset())).isEqualTo("utf-8"); + }) + .verify(); + } + @Test public void testDisabledOutgoingHeaders() { doReturn(true).when(config.getConfig(CoreConfigurationImpl.class)).isOutgoingTraceContextHeadersInjectionDisabled(); @@ -249,6 +275,11 @@ protected boolean isRedirectFollowingSupported() { return true; } + // assumption + protected boolean isBodyCapturingSupported() { + return false; + } + // assumption protected boolean isErrorOnCircularRedirectSupported() { return isRedirectFollowingSupported(); @@ -282,6 +313,10 @@ public boolean isAsync() { protected abstract void performGet(String path) throws Exception; + protected void performPost(String path, byte[] content, String contentTypeHeader) throws Exception { + throw new UnsupportedOperationException(); + } + protected VerifyBuilder expectSpan(String path) { return new VerifyBuilder(path); } @@ -294,7 +329,7 @@ protected class VerifyBuilder { private boolean traceContextHeaders = true; private boolean requestExecuted = true; - private Consumer requestBodyVerification = null; + private Consumer requestBodyVerification = null; private VerifyBuilder(String path) { this.path = path; @@ -327,7 +362,7 @@ public VerifyBuilder withoutRequestExecuted() { return this; } - public VerifyBuilder withRequestBodySatisfying(Consumer requestBodyVerification) { + public VerifyBuilder withRequestBodySatisfying(Consumer requestBodyVerification) { this.requestBodyVerification = requestBodyVerification; return this; } @@ -360,12 +395,9 @@ public SpanImpl verify() { assertThat(span).hasOutcome(Outcome.FAILURE); } - StringBuilder reqBody = span.getContext().getHttp().getRequestBody(false); - if (reqBody != null) { - assertThat(reqBody).hasSizeLessThan(1025); //IntakeV2 max allowed is 1024 - } + BodyCaptureImpl reqBody = span.getContext().getHttp().getRequestBody(); if (requestBodyVerification != null) { - requestBodyVerification.accept(reqBody == null ? null : reqBody.toString()); + requestBodyVerification.accept(reqBody); } if (isAsync()) { diff --git a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/HttpClientHelperTest.java b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/HttpClientHelperTest.java index e40199c27f..9242d167ed 100644 --- a/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/HttpClientHelperTest.java +++ b/apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/agent/httpclient/HttpClientHelperTest.java @@ -123,4 +123,17 @@ private void createSpanWithUrl(String s) throws URISyntaxException { HttpClientHelper.startHttpClientSpan(tracer.getActive(), "GET", new URI(s), null) .end(); } + + @Test + void testContentTypeCharsetExtraction() { + assertThat(HttpClientHelper.extractCharsetFromContentType("multipart/form-data; boundary=---------------------------974767299852498929531610575")) + .isNull(); + assertThat(HttpClientHelper.extractCharsetFromContentType("Content-Type: text/html; charset=utf-8")) + .isEqualTo("utf-8"); + assertThat(HttpClientHelper.extractCharsetFromContentType("Content-Type: text/html; charset = foobar;baz")) + .isEqualTo("foobar"); + assertThat(HttpClientHelper.extractCharsetFromContentType("Content-Type: application/json; charset = \"foo bar\";baz")) + .isEqualTo("foo bar"); + + } } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java index 38fa5b0451..54f62b7bf8 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java @@ -27,6 +27,8 @@ import java.util.Collections; import java.util.List; +import static co.elastic.apm.agent.tracer.configuration.RangeValidator.isInRange; + public class WebConfiguration extends ConfigurationOptionProvider { public static final int MAX_BODY_CAPTURE_BYTES = 1024; @@ -123,14 +125,16 @@ public class WebConfiguration extends ConfigurationOptionProvider { .dynamic(true) .buildWithDefault(Collections.emptyList()); - private final ConfigurationOption> captureClientRequestContentTypes = ConfigurationOption - .builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class) - .key("capture_http_client_request_content_types") + private final ConfigurationOption captureClientRequestBytes = ConfigurationOption.integerOption() + .addValidator(isInRange(0, MAX_BODY_CAPTURE_BYTES)) + .key("capture_http_client_request_body") .configurationCategory(HTTP_CATEGORY) .tags("added[1.50.0]", "internal") - .description("Configures for which content types the HTTP request bodies should be recorded.") + .description("Configures how many bytes of http-client request bodies shall be captured. " + + "Note that only request bodies will be captured for content types matching the capture_body_content_types configuration. " + + " The maximum allowed value is " + MAX_BODY_CAPTURE_BYTES + " , a value of 0 disables body capturing") .dynamic(true) - .buildWithDefault(Collections.emptyList()); + .buildWithDefault(0); public List getIgnoreUrls() { return ignoreUrls.get(); @@ -152,8 +156,8 @@ public List getCaptureContentTypes() { return captureContentTypes.get(); } - public List getCaptureClientRequestContentTypes() { - return captureClientRequestContentTypes.get(); + public int getCaptureClientRequestBytes() { + return captureClientRequestBytes.get(); } } From 07dd59f68be5b2209064a958349c90d6b6a0a758 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 17 Jul 2024 10:46:30 +0200 Subject: [PATCH 08/12] Added tests for BodyCapture context --- .../agent/impl/context/BodyCaptureImpl.java | 2 +- .../agent/impl/context/BodyCaptureTest.java | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 apm-agent-core/src/test/java/co/elastic/apm/agent/impl/context/BodyCaptureTest.java diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java index 443d17cce5..a9e23736ae 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java @@ -118,7 +118,7 @@ public boolean isFull() { if (bodyBuffer == null) { return false; } - return bodyBuffer.position() < numBytesToCapture; + return bodyBuffer.position() >= numBytesToCapture; } @Nullable diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/context/BodyCaptureTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/context/BodyCaptureTest.java new file mode 100644 index 0000000000..2c53ef665b --- /dev/null +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/context/BodyCaptureTest.java @@ -0,0 +1,56 @@ +package co.elastic.apm.agent.impl.context; + +import org.junit.jupiter.api.Test; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; + +import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class BodyCaptureTest { + + @Test + public void testAppendTruncation() { + BodyCaptureImpl capture = new BodyCaptureImpl(); + capture.markEligibleForCapturing(); + capture.startCapture("foobar", 10); + assertThat(capture.isFull()).isFalse(); + + capture.append("123Hello World!".getBytes(StandardCharsets.UTF_8), 3, 5); + assertThat(capture.isFull()).isFalse(); + + capture.append(" from the other side".getBytes(StandardCharsets.UTF_8), 0, 20); + assertThat(capture.isFull()).isTrue(); + + ByteBuffer content = capture.getBody(); + int size = content.position(); + byte[] contentBytes = new byte[size]; + content.position(0); + content.get(contentBytes); + + assertThat(contentBytes).isEqualTo("Hello from".getBytes(StandardCharsets.UTF_8)); + } + + @Test + public void testLifecycle() { + BodyCaptureImpl capture = new BodyCaptureImpl(); + + assertThat(capture.isEligibleForCapturing()).isFalse(); + assertThat(capture.startCapture("foobar", 42)) + .isFalse(); + assertThatThrownBy(() -> capture.append((byte) 42)).isInstanceOf(IllegalStateException.class); + + capture.markEligibleForCapturing(); + assertThat(capture.isEligibleForCapturing()).isTrue(); + assertThatThrownBy(() -> capture.append((byte) 42)).isInstanceOf(IllegalStateException.class); + + assertThat(capture.startCapture("foobar", 42)) + .isTrue(); + capture.append((byte) 42); //ensure no exception thrown + + // startCapture should return true only once + assertThat(capture.startCapture("foobar", 42)) + .isFalse(); + } +} From 2b7d07c28fb5af46f3a8e43a5c33caeb6f59b1d2 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 17 Jul 2024 11:33:27 +0200 Subject: [PATCH 09/12] fixes, renamed output otel attribute --- .../co/elastic/apm/agent/impl/context/BodyCaptureImpl.java | 4 ++-- .../elastic/apm/agent/report/serialize/DslJsonSerializer.java | 2 +- .../apm/agent/report/serialize/DslJsonSerializerTest.java | 2 +- .../apm/agent/tracer/configuration/WebConfiguration.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java index a9e23736ae..52a40cfa37 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java @@ -35,10 +35,10 @@ private enum CaptureState { private CaptureState state; - private StringBuilder charset; + private final StringBuilder charset; /** - * The number of bytes to capture. + * The maximum number of bytes to capture, if the body is longer remaining bytes will be dropped. */ private int numBytesToCapture; 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 bf67ac56f4..141a987f6f 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 @@ -1137,7 +1137,7 @@ private void serializeOtel(AbstractSpanImpl span, List profilingStack if (!isFirstAttrib) { jw.writeByte(COMMA); } - writeFieldName("http_client_request_body"); + writeFieldName("http.request.body.content"); jw.writeString(httpRequestBody); } jw.writeByte(OBJECT_END); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index 820ed201da..7006fd0fa8 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -443,7 +443,7 @@ void testSpanHttpRequestBodySerialization() { JsonNode spanJson = readJsonString(writer.toJsonString(span)); JsonNode otel = spanJson.get("otel"); JsonNode attribs = otel.get("attributes"); - assertThat(attribs.get("http_client_request_body").textValue()).isEqualTo("foobar"); + assertThat(attribs.get("http.request.body.content").textValue()).isEqualTo("foobar"); } public static boolean[][] getContentCombinations() { diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java index 54f62b7bf8..2a35facaf5 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/WebConfiguration.java @@ -127,7 +127,7 @@ public class WebConfiguration extends ConfigurationOptionProvider { private final ConfigurationOption captureClientRequestBytes = ConfigurationOption.integerOption() .addValidator(isInRange(0, MAX_BODY_CAPTURE_BYTES)) - .key("capture_http_client_request_body") + .key("capture_http_client_request_body_size") .configurationCategory(HTTP_CATEGORY) .tags("added[1.50.0]", "internal") .description("Configures how many bytes of http-client request bodies shall be captured. " + From 250ee184becc83b19d3dc09ac6e60900a8552e4a Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 17 Jul 2024 11:35:40 +0200 Subject: [PATCH 10/12] Added changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ca3c30bcd8..622822f22a 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -40,6 +40,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: [float] ===== Features * Added option to make routing-key part of RabbitMQ transaction/span names - {pull}3636[#3636] +* Added internal option for capturing request bodies for apache httpclient v4 - {pull}3692[#3692] [[release-notes-1.x]] === Java Agent version 1.x From c8dcb9f1bcb1555f4082d61c11d342c9433f3589 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 17 Jul 2024 15:10:52 +0200 Subject: [PATCH 11/12] Added synchronization for body capture state --- .../agent/impl/context/BodyCaptureImpl.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java index 52a40cfa37..cac434d536 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java @@ -33,7 +33,7 @@ private enum CaptureState { STARTED } - private CaptureState state; + private volatile CaptureState state; private final StringBuilder charset; @@ -62,7 +62,11 @@ public void resetState() { @Override public void markEligibleForCapturing() { if (state == CaptureState.NOT_ELIGIBLE) { - state = CaptureState.ELIGIBLE; + synchronized (this) { + if (state == CaptureState.NOT_ELIGIBLE) { + state = CaptureState.ELIGIBLE; + } + } } } @@ -72,17 +76,21 @@ public boolean isEligibleForCapturing() { } @Override - public boolean startCapture(@Nullable String charset, int numBytesToCapture) { + public boolean startCapture(@Nullable String requestCharset, int numBytesToCapture) { if (numBytesToCapture > WebConfiguration.MAX_BODY_CAPTURE_BYTES) { throw new IllegalArgumentException("Capturing " + numBytesToCapture + " bytes is not supported"); } if (state == CaptureState.ELIGIBLE) { - if (charset != null) { - this.charset.append(charset); + synchronized (this) { + if (state == CaptureState.ELIGIBLE) { + if (requestCharset != null) { + this.charset.append(requestCharset); + } + this.numBytesToCapture = numBytesToCapture; + state = CaptureState.STARTED; + return true; + } } - this.numBytesToCapture = numBytesToCapture; - state = CaptureState.STARTED; - return true; } return false; } From 19f3949f6382e6560a4f11b07be2d85e48a23ce2 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 17 Jul 2024 15:13:15 +0200 Subject: [PATCH 12/12] Update apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java Co-authored-by: jackshirazi --- .../java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java index cac434d536..289a5f6fd2 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/context/BodyCaptureImpl.java @@ -78,7 +78,7 @@ public boolean isEligibleForCapturing() { @Override public boolean startCapture(@Nullable String requestCharset, int numBytesToCapture) { if (numBytesToCapture > WebConfiguration.MAX_BODY_CAPTURE_BYTES) { - throw new IllegalArgumentException("Capturing " + numBytesToCapture + " bytes is not supported"); + throw new IllegalArgumentException("Capturing " + numBytesToCapture + " bytes is not supported, maximum is " + WebConfiguration.MAX_BODY_CAPTURE_BYTES + " bytes"); } if (state == CaptureState.ELIGIBLE) { synchronized (this) {