From 1298b4693c0b25fae4cceea6acc4d6905135d479 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 17 May 2022 23:55:45 +0300 Subject: [PATCH] Fix redisson ClassCastException (#6054) --- .../redisson/CompletableFutureWrapper.java | 38 +++---------- ...disCommandAsyncServiceInstrumentation.java | 40 ------------- ...mpletableFutureWrapperInstrumentation.java | 56 ------------------- .../RedissonInstrumentationModule.java | 6 +- .../redisson/RedissonPromiseWrapper.java | 37 +++--------- .../groovy/RedissonAsyncClientTest.groovy | 18 ++++++ 6 files changed, 33 insertions(+), 162 deletions(-) delete mode 100644 instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedisCommandAsyncServiceInstrumentation.java delete mode 100644 instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonCompletableFutureWrapperInstrumentation.java diff --git a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/CompletableFutureWrapper.java b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/CompletableFutureWrapper.java index 9d9c696af82e..8020e2553690 100644 --- a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/CompletableFutureWrapper.java +++ b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/CompletableFutureWrapper.java @@ -14,16 +14,19 @@ public final class CompletableFutureWrapper extends CompletableFuture private volatile EndOperationListener endOperationListener; private CompletableFutureWrapper(CompletableFuture delegate) { + Context context = Context.current(); this.whenComplete( (result, error) -> { EndOperationListener endOperationListener = this.endOperationListener; if (endOperationListener != null) { endOperationListener.accept(result, error); } - if (error != null) { - delegate.completeExceptionally(error); - } else { - delegate.complete(result); + try (Scope ignored = context.makeCurrent()) { + if (error != null) { + delegate.completeExceptionally(error); + } else { + delegate.complete(result); + } } }); } @@ -40,33 +43,6 @@ public static CompletableFuture wrap(CompletableFuture delegate) { return new CompletableFutureWrapper<>(delegate); } - /** - * Wrap {@link CompletableFuture} to run callbacks with the context that was current at the time - * this method was called. - * - *

This method should be called on, or as close as possible to, the {@link CompletableFuture} - * that is returned to the user to ensure that the callbacks added by user are run in appropriate - * context. - */ - public static CompletableFuture wrapContext(CompletableFuture future) { - Context context = Context.current(); - // when input future is completed, complete result future with context that was current - // at the time when the future was wrapped - CompletableFuture result = new CompletableFuture<>(); - future.whenComplete( - (T value, Throwable throwable) -> { - try (Scope ignored = context.makeCurrent()) { - if (throwable != null) { - result.completeExceptionally(throwable); - } else { - result.complete(value); - } - } - }); - - return result; - } - @Override public void setEndOperationListener(EndOperationListener endOperationListener) { this.endOperationListener = endOperationListener; diff --git a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedisCommandAsyncServiceInstrumentation.java b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedisCommandAsyncServiceInstrumentation.java deleted file mode 100644 index a8ef7f54db52..000000000000 --- a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedisCommandAsyncServiceInstrumentation.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.redisson; - -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; -import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; -import org.redisson.misc.RPromise; - -public class RedisCommandAsyncServiceInstrumentation implements TypeInstrumentation { - @Override - public ElementMatcher typeMatcher() { - return named("org.redisson.command.CommandAsyncService"); - } - - @Override - public void transform(TypeTransformer transformer) { - // used before 3.16.8 - transformer.applyAdviceToMethod( - named("async").and(takesArgument(5, named("org.redisson.misc.RPromise"))), - this.getClass().getName() + "$WrapPromiseAdvice"); - } - - @SuppressWarnings("unused") - public static class WrapPromiseAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter(@Advice.Argument(value = 5, readOnly = false) RPromise promise) { - promise = RedissonPromiseWrapper.wrapContext(promise); - } - } -} diff --git a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonCompletableFutureWrapperInstrumentation.java b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonCompletableFutureWrapperInstrumentation.java deleted file mode 100644 index d2fb8150bafc..000000000000 --- a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonCompletableFutureWrapperInstrumentation.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.redisson; - -import static net.bytebuddy.matcher.ElementMatchers.isConstructor; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; -import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -public class RedissonCompletableFutureWrapperInstrumentation implements TypeInstrumentation { - @Override - public ElementMatcher typeMatcher() { - return named("org.redisson.misc.CompletableFutureWrapper"); - } - - @Override - public void transform(TypeTransformer transformer) { - // used since 3.16.8 - transformer.applyAdviceToMethod( - isConstructor().and(takesArgument(0, CompletionStage.class)), - this.getClass().getName() + "$WrapCompletionStageAdvice"); - transformer.applyAdviceToMethod( - isConstructor().and(takesArgument(0, CompletableFuture.class)), - this.getClass().getName() + "$WrapCompletableFutureAdvice"); - } - - @SuppressWarnings("unused") - public static class WrapCompletableFutureAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.Argument(value = 0, readOnly = false) CompletableFuture completableFuture) { - completableFuture = CompletableFutureWrapper.wrapContext(completableFuture); - } - } - - @SuppressWarnings("unused") - public static class WrapCompletionStageAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.Argument(value = 0, readOnly = false) CompletionStage completionStage) { - completionStage = CompletableFutureWrapper.wrapContext(completionStage.toCompletableFuture()); - } - } -} diff --git a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonInstrumentationModule.java b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonInstrumentationModule.java index e88506aec656..7fdff3f7ef6b 100644 --- a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonInstrumentationModule.java +++ b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonInstrumentationModule.java @@ -21,10 +21,6 @@ public RedissonInstrumentationModule() { @Override public List typeInstrumentations() { - return asList( - new RedisConnectionInstrumentation(), - new RedisCommandDataInstrumentation(), - new RedisCommandAsyncServiceInstrumentation(), - new RedissonCompletableFutureWrapperInstrumentation()); + return asList(new RedisConnectionInstrumentation(), new RedisCommandDataInstrumentation()); } } diff --git a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonPromiseWrapper.java b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonPromiseWrapper.java index ed3acc335840..51d5888b533c 100644 --- a/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonPromiseWrapper.java +++ b/instrumentation/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonPromiseWrapper.java @@ -14,16 +14,19 @@ public class RedissonPromiseWrapper extends RedissonPromise implements Pro private volatile EndOperationListener endOperationListener; private RedissonPromiseWrapper(RPromise delegate) { + Context context = Context.current(); this.whenComplete( (result, error) -> { EndOperationListener endOperationListener = this.endOperationListener; if (endOperationListener != null) { endOperationListener.accept(result, error); } - if (error != null) { - delegate.tryFailure(error); - } else { - delegate.trySuccess(result); + try (Scope ignored = context.makeCurrent()) { + if (error != null) { + delegate.tryFailure(error); + } else { + delegate.trySuccess(result); + } } }); } @@ -40,32 +43,6 @@ public static RPromise wrap(RPromise delegate) { return new RedissonPromiseWrapper<>(delegate); } - /** - * Wrap {@link RPromise} to run callbacks with the context that was current at the time this - * method was called. - * - *

This method should be called on, or as close as possible to, the {@link RPromise} that is - * returned to the user to ensure that the callbacks added by user are run in appropriate context. - */ - public static RPromise wrapContext(RPromise promise) { - Context context = Context.current(); - // when returned promise is completed, complete input promise with context that was current - // at the time when the promise was wrapped - RPromise result = new RedissonPromise(); - result.whenComplete( - (value, error) -> { - try (Scope ignored = context.makeCurrent()) { - if (error != null) { - promise.tryFailure(error); - } else { - promise.trySuccess(value); - } - } - }); - - return result; - } - @Override public void setEndOperationListener(EndOperationListener endOperationListener) { this.endOperationListener = endOperationListener; diff --git a/instrumentation/redisson-3.0/javaagent/src/test/groovy/RedissonAsyncClientTest.groovy b/instrumentation/redisson-3.0/javaagent/src/test/groovy/RedissonAsyncClientTest.groovy index a4f38142bac4..9c9dd0710b63 100644 --- a/instrumentation/redisson-3.0/javaagent/src/test/groovy/RedissonAsyncClientTest.groovy +++ b/instrumentation/redisson-3.0/javaagent/src/test/groovy/RedissonAsyncClientTest.groovy @@ -6,10 +6,12 @@ import io.opentelemetry.api.trace.Span import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification import io.opentelemetry.semconv.trace.attributes.SemanticAttributes +import java.util.concurrent.Callable import java.util.concurrent.CompletionStage import org.redisson.Redisson import org.redisson.api.RBucket import org.redisson.api.RFuture +import org.redisson.api.RScheduledExecutorService import org.redisson.api.RSet import org.redisson.api.RedissonClient import org.redisson.config.Config @@ -129,5 +131,21 @@ class RedissonAsyncClientTest extends AgentInstrumentationSpecification { } } + // regression test for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/6033 + def "test schedule"() { + RScheduledExecutorService executorService = redisson.getExecutorService("EXECUTOR") + def taskId = executorService.schedule(new MyCallable(), 0, TimeUnit.SECONDS) + .getTaskId() + expect: + taskId != null + } + + private static class MyCallable implements Callable, Serializable { + + @Override + Object call() throws Exception { + return null + } + } }