From 9e29d659f5c9c0ea69958c1a28ac675f22f2de98 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 15:08:33 +0100 Subject: [PATCH 1/9] Add JAX-RS 2 client body capture Signed-off-by: Pavol Loffay --- .../v4_0/InputStreamUtils.java | 3 +- instrumentation/build.gradle.kts | 1 + .../jaxrs-client-2.0/build.gradle.kts | 42 ++++++ .../v2_0/JaxrsClientBodyCaptureFilter.java | 110 ++++++++++++++++ .../JaxrsClientBodyInstrumentationModule.java | 88 +++++++++++++ .../JaxrsClientBodyInstrumentationName.java | 24 ++++ .../v2_0/JaxrsClientEntityInterceptor.java | 78 +++++++++++ .../JaxrsClientBodyInstrumentationTest.java | 123 ++++++++++++++++++ .../core/HypertraceSemanticAttributes.java | 8 ++ settings.gradle.kts | 2 + 10 files changed, 478 insertions(+), 1 deletion(-) create mode 100644 instrumentation/jaxrs-client-2.0/build.gradle.kts create mode 100644 instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java create mode 100644 instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationModule.java create mode 100644 instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationName.java create mode 100644 instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientEntityInterceptor.java create mode 100644 instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java diff --git a/instrumentation/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/InputStreamUtils.java b/instrumentation/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/InputStreamUtils.java index 9a8bcb133..df08103f6 100644 --- a/instrumentation/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/InputStreamUtils.java +++ b/instrumentation/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/InputStreamUtils.java @@ -28,6 +28,7 @@ import java.nio.charset.Charset; import org.hypertrace.agent.core.GlobalObjectRegistry; import org.hypertrace.agent.core.GlobalObjectRegistry.SpanAndBuffer; +import org.hypertrace.agent.core.HypertraceSemanticAttributes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,7 +51,7 @@ public static void addAttribute(Span span, AttributeKey attributeKey, St span.setAttribute(attributeKey, value); } else { TRACER - .spanBuilder("additional-data") + .spanBuilder(HypertraceSemanticAttributes.ADDITIONAL_DATA_SPAN_NAME) .setParent(Context.root().with(span)) .setAttribute(attributeKey, value) .startSpan() diff --git a/instrumentation/build.gradle.kts b/instrumentation/build.gradle.kts index edf25b150..15ac6067c 100644 --- a/instrumentation/build.gradle.kts +++ b/instrumentation/build.gradle.kts @@ -37,6 +37,7 @@ dependencies{ implementation(project(":instrumentation:grpc-1.5")) implementation(project(":instrumentation:okhttp:okhttp-3.0")) implementation(project(":instrumentation:apache-httpclient-4.0")) + implementation(project(":instrumentation:jaxrs-client-2.0")) implementation(project(":otel-extensions")) } diff --git a/instrumentation/jaxrs-client-2.0/build.gradle.kts b/instrumentation/jaxrs-client-2.0/build.gradle.kts new file mode 100644 index 000000000..4cb0f13bf --- /dev/null +++ b/instrumentation/jaxrs-client-2.0/build.gradle.kts @@ -0,0 +1,42 @@ +plugins { + `java-library` + id("com.google.protobuf") version "0.8.13" + id("net.bytebuddy.byte-buddy") + id("io.opentelemetry.instrumentation.auto-instrumentation") + muzzle +} + +muzzle { + pass { + group = "javax.ws.rs" + module = "javax.ws.rs-api" + versions = "[2.0,)" + } + pass { + // We want to support the dropwizard clients too. + group = "io.dropwizard" + module = "dropwizard-client" + versions = "[0.8.0,)" + assertInverse = true + } +} + +afterEvaluate{ + io.opentelemetry.instrumentation.gradle.bytebuddy.ByteBuddyPluginConfigurator(project, + sourceSets.main.get(), + "io.opentelemetry.javaagent.tooling.muzzle.collector.MuzzleCodeGenerationPlugin", + project(":javaagent-tooling").configurations["instrumentationMuzzle"] + configurations.runtimeClasspath + ).configure() +} + +val versions: Map by extra + +dependencies { + api("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-jaxrs-client-2.0-common:${versions["opentelemetry_java_agent"]}") + + compileOnly("javax.ws.rs:javax.ws.rs-api:2.0.1") + + testImplementation(project(":testing-common")) + testImplementation("org.glassfish.jersey.core:jersey-client:2.27") + testImplementation("org.glassfish.jersey.inject:jersey-hk2:2.27") +} diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java new file mode 100644 index 000000000..ff6970194 --- /dev/null +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java @@ -0,0 +1,110 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.jaxrs.v2_0; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0.ClientTracingFilter; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.client.ClientRequestFilter; +import javax.ws.rs.client.ClientResponseContext; +import javax.ws.rs.client.ClientResponseFilter; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedMap; +import org.hypertrace.agent.config.Config.AgentConfig; +import org.hypertrace.agent.core.ContentTypeUtils; +import org.hypertrace.agent.core.HypertraceConfig; +import org.hypertrace.agent.core.HypertraceSemanticAttributes; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class JaxrsClientBodyCaptureFilter implements ClientRequestFilter, ClientResponseFilter { + + private static final Logger log = LoggerFactory.getLogger(JaxrsClientBodyCaptureFilter.class); + + @Override + public void filter(ClientRequestContext requestContext) { + Object spanObj = requestContext.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); + if (!(spanObj instanceof Span)) { + return; + } + + Span currentSpan = (Span) spanObj; + AgentConfig agentConfig = HypertraceConfig.get(); + + try { + if (agentConfig.getDataCapture().getHttpHeaders().getRequest().getValue()) { + captureHeaders( + currentSpan, + HypertraceSemanticAttributes::httpRequestHeader, + requestContext.getStringHeaders()); + } + if (requestContext.hasEntity() + && agentConfig.getDataCapture().getHttpBody().getRequest().getValue()) { + MediaType mediaType = requestContext.getMediaType(); + if (mediaType == null || !ContentTypeUtils.shouldCapture(mediaType.toString())) { + return; + } + + Object entity = requestContext.getEntity(); + if (entity != null) { + currentSpan.setAttribute( + HypertraceSemanticAttributes.HTTP_REQUEST_BODY, entity.toString()); + } + } + requestContext.getEntity(); + } catch (Exception ex) { + log.error("Exception while getting request entity or headers", ex); + } + } + + @Override + public void filter(ClientRequestContext requestContext, ClientResponseContext responseContext) { + Object spanObj = requestContext.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); + if (!(spanObj instanceof Span)) { + return; + } + + Span currentSpan = (Span) spanObj; + AgentConfig agentConfig = HypertraceConfig.get(); + + try { + if (agentConfig.getDataCapture().getHttpHeaders().getResponse().getValue()) { + captureHeaders( + currentSpan, + HypertraceSemanticAttributes::httpResponseHeader, + responseContext.getHeaders()); + } + } catch (Exception ex) { + log.error("Exception while getting response entity or headers", ex); + } + } + + private void captureHeaders( + Span span, + Function> keySupplier, + MultivaluedMap headers) { + for (Map.Entry> entry : headers.entrySet()) { + for (Object value : entry.getValue()) { + span.setAttribute(keySupplier.apply(entry.getKey()), value.toString()); + } + } + } +} diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationModule.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationModule.java new file mode 100644 index 000000000..1901cbde7 --- /dev/null +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationModule.java @@ -0,0 +1,88 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.jaxrs.v2_0; + +import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed; +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.extendsClass; +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.hasInterface; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.tooling.InstrumentationModule; +import io.opentelemetry.javaagent.tooling.TypeInstrumentation; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.ws.rs.client.Client; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.implementation.bytecode.assign.Assigner; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class JaxrsClientBodyInstrumentationModule extends InstrumentationModule { + + @Override + public int getOrder() { + return 1; + } + + public JaxrsClientBodyInstrumentationModule() { + super(JaxrsClientBodyInstrumentationName.PRIMARY, JaxrsClientBodyInstrumentationName.OTHER); + } + + @Override + public List typeInstrumentations() { + return Collections.singletonList(new JaxrsClientBuilderInstrumentation()); + } + + class JaxrsClientBuilderInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("javax.ws.rs.client.ClientBuilder"); + } + + @Override + public ElementMatcher typeMatcher() { + return extendsClass(named("javax.ws.rs.client.ClientBuilder")); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("build").and(returns(hasInterface(named("javax.ws.rs.client.Client")))), + ClientBuilder_build_Advice.class.getName()); + } + } + + static class ClientBuilder_build_Advice { + @Advice.OnMethodExit + public static void registerFeature( + @Advice.Return(typing = Assigner.Typing.DYNAMIC) Client client) { + // Register on the generated client instead of the builder + // The build() can be called multiple times and is not thread safe + // A client is only created once + // Use lowest priority to run after OTEL filter that controls lifecycle of span + client.register(JaxrsClientBodyCaptureFilter.class, Integer.MIN_VALUE); + client.register(JaxrsClientEntityInterceptor.class); + } + } +} diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationName.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationName.java new file mode 100644 index 000000000..158d61f78 --- /dev/null +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationName.java @@ -0,0 +1,24 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.jaxrs.v2_0; + +public class JaxrsClientBodyInstrumentationName { + public static final String PRIMARY = "jaxrs-client"; + public static final String[] OTHER = { + "jaxrs-client-2.0", "ht", "jaxrs-client-ht", "jaxrs-client-2.0-ht" + }; +} diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientEntityInterceptor.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientEntityInterceptor.java new file mode 100644 index 000000000..dde6e4555 --- /dev/null +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientEntityInterceptor.java @@ -0,0 +1,78 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.jaxrs.v2_0; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Context; +import io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0.ClientTracingFilter; +import java.io.IOException; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.ext.ReaderInterceptor; +import javax.ws.rs.ext.ReaderInterceptorContext; +import org.hypertrace.agent.config.Config.AgentConfig; +import org.hypertrace.agent.core.ContentTypeUtils; +import org.hypertrace.agent.core.HypertraceConfig; +import org.hypertrace.agent.core.HypertraceSemanticAttributes; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class JaxrsClientEntityInterceptor implements ReaderInterceptor { + + private static final Logger log = LoggerFactory.getLogger(JaxrsClientEntityInterceptor.class); + + private static final Tracer TRACER = + OpenTelemetry.getGlobalTracer("org.hypertrace.java.jaxrs.client"); + + @Override + public Object aroundReadFrom(ReaderInterceptorContext context) + throws IOException, WebApplicationException { + + Object entity = context.proceed(); + + Object spanObj = context.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME); + if (!(spanObj instanceof Span)) { + log.error( + "Span object is not present in the context properties, response object will not be captured"); + return entity; + } + Span currentSpan = (Span) spanObj; + + MediaType mediaType = context.getMediaType(); + AgentConfig agentConfig = HypertraceConfig.get(); + if (mediaType == null + || !ContentTypeUtils.shouldCapture(mediaType.toString()) + || !agentConfig.getDataCapture().getHttpBody().getResponse().getValue()) { + return entity; + } + + if (currentSpan.isRecording()) { + currentSpan.setAttribute(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY, entity.toString()); + } else { + TRACER + .spanBuilder(HypertraceSemanticAttributes.ADDITIONAL_DATA_SPAN_NAME) + .setParent(Context.root().with(currentSpan)) + .setAttribute(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY, entity.toString()) + .startSpan() + .end(); + } + + return entity; + } +} diff --git a/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java b/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java new file mode 100644 index 000000000..2eeecf83b --- /dev/null +++ b/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java @@ -0,0 +1,123 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.jaxrs.v2_0; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.sdk.trace.data.SpanData; +import java.util.List; +import java.util.concurrent.TimeoutException; +import javax.ws.rs.client.Client; +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.hypertrace.agent.core.HypertraceSemanticAttributes; +import org.hypertrace.agent.testing.AbstractInstrumenterTest; +import org.hypertrace.agent.testing.TestHttpServer; +import org.hypertrace.agent.testing.TestHttpServer.GetJsonHandler; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +public class JaxrsClientBodyInstrumentationTest extends AbstractInstrumenterTest { + + private static final String JSON = "{\"id\":1,\"name\":\"John\"}"; + private static final TestHttpServer testHttpServer = new TestHttpServer(); + + @BeforeAll + public static void startServer() throws Exception { + testHttpServer.start(); + } + + @AfterAll + public static void closeServer() throws Exception { + testHttpServer.close(); + } + + @Test + public void getJson() throws TimeoutException, InterruptedException { + ClientBuilder clientBuilder = ClientBuilder.newBuilder(); + Client client = clientBuilder.build(); + + Response response = + client + .target(String.format("http://localhost:%d/get_json", testHttpServer.port())) + .request() + .header("test-request-header", "test-header-value") + .get(); + Assertions.assertEquals(200, response.getStatus()); + // read entity has to happen before response.close() + String entity = response.readEntity(String.class); + Assertions.assertEquals(GetJsonHandler.RESPONSE_BODY, entity); + Assertions.assertEquals(false, Span.current().isRecording()); + response.close(); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(2, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + Assertions.assertEquals( + "test-value", + clientSpan + .getAttributes() + .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); + Assertions.assertEquals( + "test-header-value", + clientSpan + .getAttributes() + .get(HypertraceSemanticAttributes.httpRequestHeader("test-request-header"))); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + SpanData responseBodySpan = traces.get(0).get(1); + Assertions.assertEquals( + GetJsonHandler.RESPONSE_BODY, + responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } + + @Test + public void postJson() throws TimeoutException, InterruptedException { + ClientBuilder clientBuilder = ClientBuilder.newBuilder(); + Client client = clientBuilder.build(); + + Response response = + client + .target(String.format("http://localhost:%d/post", testHttpServer.port())) + .request() + .header("test-request-header", "test-header-value") + .post(Entity.entity(JSON, MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals(204, response.getStatus()); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + Assertions.assertEquals( + "test-value", + clientSpan + .getAttributes() + .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); + Assertions.assertEquals( + JSON, clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } +} diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/HypertraceSemanticAttributes.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/HypertraceSemanticAttributes.java index 391cd13af..972c60f4c 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/HypertraceSemanticAttributes.java +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/HypertraceSemanticAttributes.java @@ -21,6 +21,14 @@ public class HypertraceSemanticAttributes { private HypertraceSemanticAttributes() {} + /** + * Span name used for span that carries additional data (e.g. attributes) that should belong to + * its parent. The parent span cannot carry additional data because it has been already finished. + * This usually happens when capturing response body in RPC client instrumentations - the body is + * read after the client span is finished. + */ + public static final String ADDITIONAL_DATA_SPAN_NAME = "additional-data"; + public static AttributeKey httpRequestHeader(String header) { return AttributeKey.stringKey("http.request.header." + header); } diff --git a/settings.gradle.kts b/settings.gradle.kts index caeafcfad..f74f4b1a9 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -39,3 +39,5 @@ findProject(":instrumentation:okhttp:okhttp-3.0")?.name = "okhttp-3.0" include("filter-custom-opa") include("otel-extensions") include("testing-bootstrap") +include("instrumentation:jaxrs-client-2.0") +findProject(":instrumentation:jaxrs-client-2.0")?.name = "jaxrs-client-2.0" From 08cb872cfc9b6916323af6f66546543d5a163bc4 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 15:22:52 +0100 Subject: [PATCH 2/9] Add to readme Signed-off-by: Pavol Loffay --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ca172835b..c1789046d 100644 --- a/README.md +++ b/README.md @@ -14,10 +14,12 @@ and adds following capabilities: List of supported frameworks with additional capabilities: | Library/Framework | Versions | |--------------------------------------------------------------------------------------------------------|-----------------| +| [Apache HttpClient](https://hc.apache.org/index.html) | 4.0+ | +| [gRPC](https://github.com/grpc/grpc-java) | 1.5+, | +| [JAX-RS Client](https://javaee.github.io/javaee-spec/javadocs/javax/ws/rs/client/package-summary.html) | 2.0+ | +| [OkHttp](https://github.com/square/okhttp/) | 3.0+ | | [Servlet](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/package-summary.html) | 2.3+ | | [Spark Web Framework](https://github.com/perwendel/spark) | 2.3+ | -| [gRPC](https://github.com/grpc/grpc-java) | 1.5+ | -| [OkHttp](https://github.com/square/okhttp/) | 3.0+ | ### Adding custom filter implementation From 29aa0f41182c75788af8067992e6c068c2d6eceb Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 15:23:55 +0100 Subject: [PATCH 3/9] typo Signed-off-by: Pavol Loffay --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c1789046d..2ff7d088f 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ The following instrumentation names disable only Hypertrace instrumentations, no * `ht` - all Hypertrace instrumentations * `servlet-ht` - Servlet, Spark Web * `okhttp-ht` - Okhttp -* `grpc-ht` - Okhttp +* `grpc-ht` - gRPC The Hypertrace instrumentations use also the core OpenTelemetry instrumentation names so for example `-Dotel.instrumentation.servlet.enabled=false` disables all servlet instrumentations including core From a7c1dc40f0abf9534ae0580292c02dc4d507220c Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 16:11:38 +0100 Subject: [PATCH 4/9] Form Signed-off-by: Pavol Loffay --- .../v2_0/JaxrsClientBodyCaptureFilter.java | 14 ++++++-- .../JaxrsClientBodyInstrumentationTest.java | 34 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java index ff6970194..2798c8392 100644 --- a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java @@ -26,6 +26,7 @@ import javax.ws.rs.client.ClientRequestFilter; import javax.ws.rs.client.ClientResponseContext; import javax.ws.rs.client.ClientResponseFilter; +import javax.ws.rs.core.Form; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.MultivaluedMap; import org.hypertrace.agent.config.Config.AgentConfig; @@ -65,8 +66,17 @@ public void filter(ClientRequestContext requestContext) { Object entity = requestContext.getEntity(); if (entity != null) { - currentSpan.setAttribute( - HypertraceSemanticAttributes.HTTP_REQUEST_BODY, entity.toString()); + if (entity instanceof Form) { + Form form = (Form) entity; + MultivaluedMap formMap = form.asMap(); + if (formMap != null) { + currentSpan.setAttribute( + HypertraceSemanticAttributes.HTTP_REQUEST_BODY, formMap.toString()); + } + } else { + currentSpan.setAttribute( + HypertraceSemanticAttributes.HTTP_REQUEST_BODY, entity.toString()); + } } } requestContext.getEntity(); diff --git a/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java b/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java index 2eeecf83b..e18ceaa8d 100644 --- a/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java +++ b/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java @@ -23,7 +23,10 @@ import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.client.Entity; +import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedHashMap; +import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import org.hypertrace.agent.core.HypertraceSemanticAttributes; import org.hypertrace.agent.testing.AbstractInstrumenterTest; @@ -120,4 +123,35 @@ public void postJson() throws TimeoutException, InterruptedException { Assertions.assertNull( clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } + + @Test + public void postUrlEncoded() throws TimeoutException, InterruptedException { + ClientBuilder clientBuilder = ClientBuilder.newBuilder(); + Client client = clientBuilder.build(); + + WebTarget webTarget = + client.target(String.format("http://localhost:%d/post", testHttpServer.port())); + MultivaluedMap formData = new MultivaluedHashMap<>(); + formData.add("key1", "value1"); + formData.add("key2", "value2"); + Response response = webTarget.request().post(Entity.form(formData)); + Assertions.assertEquals(204, response.getStatus()); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + Assertions.assertEquals( + "test-value", + clientSpan + .getAttributes() + .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); + Assertions.assertEquals( + "{key1=[value1], key2=[value2]}", + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } } From 8bf2b72ed0080e3c93ffd98d2e295acd64bd8b59 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 16:13:14 +0100 Subject: [PATCH 5/9] Update readme Signed-off-by: Pavol Loffay --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2ff7d088f..ffc07cd0e 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ List of supported frameworks with additional capabilities: | Library/Framework | Versions | |--------------------------------------------------------------------------------------------------------|-----------------| | [Apache HttpClient](https://hc.apache.org/index.html) | 4.0+ | -| [gRPC](https://github.com/grpc/grpc-java) | 1.5+, | +| [gRPC](https://github.com/grpc/grpc-java) | 1.5+ to [1.32.0](https://github.com/hypertrace/javaagent/issues/70) | | [JAX-RS Client](https://javaee.github.io/javaee-spec/javadocs/javax/ws/rs/client/package-summary.html) | 2.0+ | | [OkHttp](https://github.com/square/okhttp/) | 3.0+ | | [Servlet](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/package-summary.html) | 2.3+ | From f710ea95957cc090b21a7546a25e5c9a9b76cb6b Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 16:16:10 +0100 Subject: [PATCH 6/9] Fix Signed-off-by: Pavol Loffay --- .../hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java index 4545ec44d..32e5102ad 100644 --- a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java +++ b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java @@ -113,7 +113,7 @@ public void getJson() throws Exception { @Test public void postUrlEncoded() throws Exception { - FormBody formBody = new FormBody.Builder().add("foo", "bar").build(); + FormBody formBody = new FormBody.Builder().add("foo", "bar").add("baz", "far").build(); Request request = new Builder() .url(String.format("http://localhost:%d/post", testHttpServer.port())) @@ -134,7 +134,8 @@ public void postUrlEncoded() throws Exception { .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); Assertions.assertEquals( - "foo=bar", clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + "foo=bar&baz=far", + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertNull( clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } From e50ba76c50aee335431508e6f701c0a989bd783a Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 16:27:05 +0100 Subject: [PATCH 7/9] unify across clients Signed-off-by: Pavol Loffay --- .../v4_0/ApacheHttpClientInstrumentationTest.java | 6 ++++-- .../okhttp/v3_0/OkHttpTracingInterceptorTest.java | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java index 9eb8b2593..939e4bddd 100644 --- a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java +++ b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java @@ -105,7 +105,8 @@ public void getJson() throws IOException, TimeoutException, InterruptedException @Test public void postUrlEncoded() throws IOException, TimeoutException, InterruptedException { List nvps = new ArrayList<>(); - nvps.add(new BasicNameValuePair("code", "22")); + nvps.add(new BasicNameValuePair("key1", "value1")); + nvps.add(new BasicNameValuePair("key2", "value2")); HttpPost postRequest = new HttpPost(); postRequest.setEntity(new UrlEncodedFormEntity(nvps, HTTP.UTF_8)); @@ -126,7 +127,8 @@ public void postUrlEncoded() throws IOException, TimeoutException, InterruptedEx .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); Assertions.assertEquals( - "code=22", clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + "key1=value1&key2=value2", + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertNull( clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } diff --git a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java index 32e5102ad..d19f3737a 100644 --- a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java +++ b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java @@ -113,7 +113,7 @@ public void getJson() throws Exception { @Test public void postUrlEncoded() throws Exception { - FormBody formBody = new FormBody.Builder().add("foo", "bar").add("baz", "far").build(); + FormBody formBody = new FormBody.Builder().add("key1", "value1").add("key2", "value2").build(); Request request = new Builder() .url(String.format("http://localhost:%d/post", testHttpServer.port())) @@ -134,7 +134,7 @@ public void postUrlEncoded() throws Exception { .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); Assertions.assertEquals( - "foo=bar&baz=far", + "key1=value1&key2=value2", clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertNull( clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); From 4879f02f72de96e4ac4db0f9dac9154ff41041e0 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 16:40:46 +0100 Subject: [PATCH 8/9] Use the right content Signed-off-by: Pavol Loffay --- .../v2_0/JaxrsClientBodyCaptureFilter.java | 25 +++++++++++++++---- .../JaxrsClientBodyInstrumentationTest.java | 2 +- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java index 2798c8392..df562bc29 100644 --- a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java @@ -68,11 +68,8 @@ public void filter(ClientRequestContext requestContext) { if (entity != null) { if (entity instanceof Form) { Form form = (Form) entity; - MultivaluedMap formMap = form.asMap(); - if (formMap != null) { - currentSpan.setAttribute( - HypertraceSemanticAttributes.HTTP_REQUEST_BODY, formMap.toString()); - } + String content = getContent(form); + currentSpan.setAttribute(HypertraceSemanticAttributes.HTTP_REQUEST_BODY, content); } else { currentSpan.setAttribute( HypertraceSemanticAttributes.HTTP_REQUEST_BODY, entity.toString()); @@ -107,6 +104,24 @@ public void filter(ClientRequestContext requestContext, ClientResponseContext re } } + private String getContent(Form form) { + MultivaluedMap formMap = form.asMap(); + StringBuilder sb = new StringBuilder(); + if (formMap != null) { + for (Map.Entry> entry : formMap.entrySet()) { + if (sb.length() > 0) { + sb.append("&"); + } + for (String value : entry.getValue()) { + sb.append(entry.getKey()); + sb.append("="); + sb.append(value); + } + } + } + return sb.toString(); + } + private void captureHeaders( Span span, Function> keySupplier, diff --git a/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java b/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java index e18ceaa8d..177297c15 100644 --- a/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java +++ b/instrumentation/jaxrs-client-2.0/src/test/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyInstrumentationTest.java @@ -149,7 +149,7 @@ public void postUrlEncoded() throws TimeoutException, InterruptedException { .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); Assertions.assertEquals( - "{key1=[value1], key2=[value2]}", + "key1=value1&key2=value2", clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertNull( clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); From b0646c24370d8255b83fc64dd559e0bb295aeacc Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 16:44:25 +0100 Subject: [PATCH 9/9] use static and remove junk Signed-off-by: Pavol Loffay --- instrumentation/jaxrs-client-2.0/build.gradle.kts | 1 - .../hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/instrumentation/jaxrs-client-2.0/build.gradle.kts b/instrumentation/jaxrs-client-2.0/build.gradle.kts index 4cb0f13bf..905a84af0 100644 --- a/instrumentation/jaxrs-client-2.0/build.gradle.kts +++ b/instrumentation/jaxrs-client-2.0/build.gradle.kts @@ -1,6 +1,5 @@ plugins { `java-library` - id("com.google.protobuf") version "0.8.13" id("net.bytebuddy.byte-buddy") id("io.opentelemetry.instrumentation.auto-instrumentation") muzzle diff --git a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java index df562bc29..44fcaf99c 100644 --- a/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java +++ b/instrumentation/jaxrs-client-2.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/jaxrs/v2_0/JaxrsClientBodyCaptureFilter.java @@ -68,7 +68,7 @@ public void filter(ClientRequestContext requestContext) { if (entity != null) { if (entity instanceof Form) { Form form = (Form) entity; - String content = getContent(form); + String content = getUrlEncodedContent(form); currentSpan.setAttribute(HypertraceSemanticAttributes.HTTP_REQUEST_BODY, content); } else { currentSpan.setAttribute( @@ -104,7 +104,7 @@ public void filter(ClientRequestContext requestContext, ClientResponseContext re } } - private String getContent(Form form) { + private static String getUrlEncodedContent(Form form) { MultivaluedMap formMap = form.asMap(); StringBuilder sb = new StringBuilder(); if (formMap != null) { @@ -122,7 +122,7 @@ private String getContent(Form form) { return sb.toString(); } - private void captureHeaders( + private static void captureHeaders( Span span, Function> keySupplier, MultivaluedMap headers) {