From 26dc106399b7d442781ac2f906d4081616a205d9 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 26 Jul 2021 07:12:29 +0300 Subject: [PATCH] Okhttp3: fix concurrency test with callback (#3669) --- .../OkHttp3DispatcherInstrumentation.java | 54 +++++++++++++++++++ .../v3_0/OkHttp3InstrumentationModule.java | 4 +- .../okhttp/v3_0/OkHttp3Test.groovy | 5 ++ .../okhttp/v3_0/AbstractOkHttp3Test.groovy | 7 +-- .../test/base/HttpClientTest.groovy | 5 ++ 5 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3DispatcherInstrumentation.java diff --git a/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3DispatcherInstrumentation.java b/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3DispatcherInstrumentation.java new file mode 100644 index 0000000000..e85b9a9803 --- /dev/null +++ b/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3DispatcherInstrumentation.java @@ -0,0 +1,54 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0; + +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 io.opentelemetry.javaagent.instrumentation.api.ContextStore; +import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.api.concurrent.ExecutorInstrumentationUtils; +import io.opentelemetry.javaagent.instrumentation.api.concurrent.State; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class OkHttp3DispatcherInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return named("okhttp3.Dispatcher"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("enqueue").and(takesArgument(0, named("okhttp3.RealCall$AsyncCall"))), + OkHttp3DispatcherInstrumentation.class.getName() + "$AttachStateAdvice"); + } + + @SuppressWarnings("unused") + public static class AttachStateAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static State onEnter(@Advice.Argument(0) Runnable call) { + if (ExecutorInstrumentationUtils.shouldAttachStateToTask(call)) { + ContextStore contextStore = + InstrumentationContext.get(Runnable.class, State.class); + return ExecutorInstrumentationUtils.setupState( + contextStore, call, Java8BytecodeBridge.currentContext()); + } + return null; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit(@Advice.Enter State state, @Advice.Thrown Throwable throwable) { + ExecutorInstrumentationUtils.cleanUpOnMethodExit(state, throwable); + } + } +} diff --git a/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3InstrumentationModule.java b/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3InstrumentationModule.java index a28865a436..afc5d61aa2 100644 --- a/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3InstrumentationModule.java +++ b/instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3InstrumentationModule.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0; -import static java.util.Collections.singletonList; +import static java.util.Arrays.asList; import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; @@ -21,6 +21,6 @@ public class OkHttp3InstrumentationModule extends InstrumentationModule { @Override public List typeInstrumentations() { - return singletonList(new OkHttp3Instrumentation()); + return asList(new OkHttp3Instrumentation(), new OkHttp3DispatcherInstrumentation()); } } diff --git a/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy b/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy index eae7e8caf5..e30047b457 100644 --- a/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy +++ b/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy @@ -24,4 +24,9 @@ class OkHttp3Test extends AbstractOkHttp3Test implements LibraryTestTrait { boolean testWithClientParent() { false } + + @Override + boolean testCausalityWithCallback() { + false + } } diff --git a/instrumentation/okhttp/okhttp-3.0/testing/src/main/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.groovy b/instrumentation/okhttp/okhttp-3.0/testing/src/main/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.groovy index 4a9c5c050d..fde3bda8a8 100644 --- a/instrumentation/okhttp/okhttp-3.0/testing/src/main/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.groovy +++ b/instrumentation/okhttp/okhttp-3.0/testing/src/main/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.groovy @@ -63,12 +63,7 @@ abstract class AbstractOkHttp3Test extends HttpClientTest { } @Override - boolean testRedirects() { - false - } - - @Override - boolean testCausality() { + boolean testCircularRedirects() { false } diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy index 2e2f9f590c..174293c474 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy @@ -865,6 +865,7 @@ abstract class HttpClientTest extends InstrumentationSpecification { def "high concurrency test with callback"() { setup: assumeTrue(testCausality()) + assumeTrue(testCausalityWithCallback()) assumeTrue(testCallback()) assumeTrue(testCallbackWithParent()) @@ -1120,6 +1121,10 @@ abstract class HttpClientTest extends InstrumentationSpecification { true } + boolean testCausalityWithCallback() { + true + } + boolean testCallback() { return true }