From cdfe8e0afbc39faf3c90c0d4face21e67b3daa6a Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 19 Jan 2021 14:02:18 +0200 Subject: [PATCH] Fix integration with jersey-client 2.30 (#2071) * Fix integration with jetty-client 2.30 * make class final and add private constructor * Update instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-common/javaagent/jaxrs-client-2.0-common-javaagent.gradle Co-authored-by: Trask Stalnaker Co-authored-by: Trask Stalnaker --- .../jaxrs-client-2.0-common-javaagent.gradle | 4 +- .../JerseyClientInstrumentationModule.java | 11 +---- .../jaxrsclient/v2_0/JerseyClientUtil.java | 47 +++++++++++++++++++ .../jaxrsclient/v2_0/WrappedFuture.java | 13 ++--- 4 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientUtil.java diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-common/javaagent/jaxrs-client-2.0-common-javaagent.gradle b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-common/javaagent/jaxrs-client-2.0-common-javaagent.gradle index a5228542cc..57b9ac190c 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-common/javaagent/jaxrs-client-2.0-common-javaagent.gradle +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-common/javaagent/jaxrs-client-2.0-common-javaagent.gradle @@ -35,8 +35,8 @@ dependencies { testInstrumentation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent') - latestDepTestLibrary group: 'org.glassfish.jersey.inject', name: 'jersey-hk2', version: '2.27' - latestDepTestLibrary group: 'org.glassfish.jersey.core', name: 'jersey-client', version: '2.27' + latestDepTestLibrary group: 'org.glassfish.jersey.inject', name: 'jersey-hk2', version: '2.+' + latestDepTestLibrary group: 'org.glassfish.jersey.core', name: 'jersey-client', version: '2.+' latestDepTestLibrary group: 'org.apache.cxf', name: 'cxf-rt-rs-client', version: '3.2.6' latestDepTestLibrary group: 'org.jboss.resteasy', name: 'resteasy-client', version: '3.0.26.Final' } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientInstrumentationModule.java b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientInstrumentationModule.java index f0a6e18f54..d2e35155da 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientInstrumentationModule.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientInstrumentationModule.java @@ -5,14 +5,12 @@ package io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0; -import static io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0.JaxRsClientTracer.tracer; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; import com.google.auto.service.AutoService; -import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.tooling.InstrumentationModule; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; import java.util.Collections; @@ -71,10 +69,7 @@ public class JerseyClientInstrumentationModule extends InstrumentationModule { @Advice.FieldValue("requestContext") ClientRequest context, @Advice.Thrown Throwable throwable) { if (throwable != null) { - Object prop = context.getProperty(ClientTracingFilter.CONTEXT_PROPERTY_NAME); - if (prop instanceof Context) { - tracer().endExceptionally((Context) prop, throwable); - } + JerseyClientUtil.handleException(context, throwable); } } } @@ -85,9 +80,7 @@ public class JerseyClientInstrumentationModule extends InstrumentationModule { public static void handleError( @Advice.FieldValue("requestContext") ClientRequest context, @Advice.Return(readOnly = false) Future future) { - if (!(future instanceof WrappedFuture)) { - future = new WrappedFuture<>(future, context); - } + future = JerseyClientUtil.addErrorReporting(context, future); } } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientUtil.java b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientUtil.java new file mode 100644 index 0000000000..36b8c64266 --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/JerseyClientUtil.java @@ -0,0 +1,47 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0; + +import static io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0.JaxRsClientTracer.tracer; + +import io.opentelemetry.context.Context; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Future; +import org.glassfish.jersey.client.ClientRequest; + +public final class JerseyClientUtil { + + public static Future addErrorReporting(ClientRequest context, Future future) { + // since jersey 2.30 jersey internally uses CompletableFuture + // we can't wrap it with WrappedFuture as it causes ClassCastException when casting + // to CompletableFuture + if (future instanceof CompletableFuture) { + future = + ((CompletableFuture) future) + .whenComplete( + (result, exception) -> { + if (exception != null) { + handleException(context, exception); + } + }); + } else { + if (!(future instanceof WrappedFuture)) { + future = new WrappedFuture<>(future, context); + } + } + + return future; + } + + public static void handleException(ClientRequest context, Throwable exception) { + Object prop = context.getProperty(ClientTracingFilter.CONTEXT_PROPERTY_NAME); + if (prop instanceof Context) { + tracer().endExceptionally((Context) prop, exception); + } + } + + private JerseyClientUtil() {} +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/WrappedFuture.java b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/WrappedFuture.java index 60c1c5c4c4..88d29dcb03 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/WrappedFuture.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0/jaxrs-client-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/v2_0/WrappedFuture.java @@ -5,9 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0; -import static io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0.JaxRsClientTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.jaxrsclient.v2_0.JerseyClientUtil.handleException; -import io.opentelemetry.context.Context; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -44,10 +43,7 @@ public class WrappedFuture implements Future { try { return wrapped.get(); } catch (ExecutionException e) { - Object prop = context.getProperty(ClientTracingFilter.CONTEXT_PROPERTY_NAME); - if (prop instanceof Context) { - tracer().endExceptionally((Context) prop, e.getCause()); - } + handleException(context, e.getCause()); throw e; } } @@ -58,10 +54,7 @@ public class WrappedFuture implements Future { try { return wrapped.get(timeout, unit); } catch (ExecutionException e) { - Object prop = context.getProperty(ClientTracingFilter.CONTEXT_PROPERTY_NAME); - if (prop instanceof Context) { - tracer().endExceptionally((Context) prop, e.getCause()); - } + handleException(context, e.getCause()); throw e; } }