From 75455b69f536db596207e604990a7cc93dbdf6c5 Mon Sep 17 00:00:00 2001 From: John Engelman Date: Tue, 5 Nov 2024 08:09:27 -0600 Subject: [PATCH 1/5] Add agent instrumentation for Ratpack 1.7 --- .../client/AbstractRatpackHttpClientTest.java | 28 ++++--- .../ratpack-1.7/javaagent/build.gradle.kts | 53 ++++++++++++ .../ratpack/v1_7/DownstreamWrapper.java | 55 ++++++++++++ .../v1_7/HttpClientInstrumentation.java | 46 ++++++++++ .../v1_7/RatpackInstrumentationModule.java | 44 ++++++++++ .../ratpack/v1_7/RatpackSingletons.java | 54 ++++++++++++ .../RequestActionSupportInstrumentation.java | 84 +++++++++++++++++++ .../v1_7/ServerRegistryInstrumentation.java | 42 ++++++++++ .../server/RatpackAsyncHttpServerTest.groovy | 22 +++++ .../server/RatpackForkedHttpServerTest.groovy | 27 ++++++ .../server/RatpackHttpServerTest.groovy | 22 +++++ .../groovy/server/RatpackRoutesTest.groovy | 21 +++++ .../v1_7/RatpackForkedHttpClientTest.java | 68 +++++++++++++++ .../ratpack/v1_7/RatpackHttpClientTest.java | 68 +++++++++++++++ .../v1_7/RatpackPooledHttpClientTest.java | 68 +++++++++++++++ .../v1_7/OpenTelemetryExecInitializer.java | 2 +- .../v1_7/OpenTelemetryExecInterceptor.java | 4 +- .../ratpack/v1_7/OpenTelemetryHttpClient.java | 26 ++++-- .../ratpack/v1_7/RatpackTelemetryBuilder.java | 8 ++ .../v1_7/AbstractRatpackHttpClientTest.java | 6 +- settings.gradle.kts | 1 + 21 files changed, 724 insertions(+), 25 deletions(-) create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DownstreamWrapper.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackSingletons.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackAsyncHttpServerTest.groovy create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackForkedHttpServerTest.groovy create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackHttpServerTest.groovy create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java diff --git a/instrumentation/ratpack/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java b/instrumentation/ratpack/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java index bb6a43e8aa84..f5626cefd2ec 100644 --- a/instrumentation/ratpack/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java +++ b/instrumentation/ratpack/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java @@ -29,13 +29,13 @@ public abstract class AbstractRatpackHttpClientTest extends AbstractHttpClientTest { - private final ExecHarness exec = ExecHarness.harness(); + protected final ExecHarness exec = ExecHarness.harness(); - private HttpClient client; - private HttpClient singleConnectionClient; + protected HttpClient client; + protected HttpClient singleConnectionClient; @BeforeAll - void setUpClient() throws Exception { + protected void setUpClient() throws Exception { exec.run( unused -> { client = buildHttpClient(); @@ -66,7 +66,7 @@ public Void buildRequest(String method, URI uri, Map headers) { @Override public int sendRequest(Void request, String method, URI uri, Map headers) throws Exception { - return exec.yield(unused -> internalSendRequest(client, method, uri, headers)) + return exec.yield(execution -> internalSendRequest(client, method, uri, headers)) .getValueOrThrow(); } @@ -78,13 +78,17 @@ public final void sendRequestWithCallback( Map headers, HttpClientResult httpClientResult) throws Exception { - exec.execute( - Operation.of( - () -> - internalSendRequest(client, method, uri, headers) - .result( - result -> - httpClientResult.complete(result::getValue, result.getThrowable())))); + exec.yield( + (e) -> + Operation.of( + () -> + internalSendRequest(client, method, uri, headers) + .result( + result -> + httpClientResult.complete( + result::getValue, result.getThrowable()))) + .promise()) + .getValueOrThrow(); } // overridden in RatpackForkedHttpClientTest diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts b/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts new file mode 100644 index 000000000000..dbc58cadd42d --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts @@ -0,0 +1,53 @@ +plugins { + id("otel.javaagent-instrumentation") +} + +muzzle { + pass { + group.set("io.ratpack") + module.set("ratpack-core") + versions.set("[1.7.0,)") + } +} + +dependencies { + library("io.ratpack:ratpack-core:1.7.0") + + implementation(project(":instrumentation:netty:netty-4.1:library")) + implementation(project(":instrumentation:ratpack:ratpack-1.4:javaagent")) + implementation(project(":instrumentation:ratpack:ratpack-1.7:library")) + + testImplementation(project(":instrumentation:ratpack:ratpack-1.4:testing")) + + testLibrary("io.ratpack:ratpack-test:1.7.0") + + if (JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_11)) { + testImplementation("com.sun.activation:jakarta.activation:1.2.2") + } +} + +// to allow all tests to pass we need to choose a specific netty version +if (!(findProperty("testLatestDeps") as Boolean)) { + configurations.configureEach { + if (!name.contains("muzzle")) { + resolutionStrategy { + eachDependency { + // specifying a fixed version for all libraries with io.netty group + if (requested.group == "io.netty" && requested.name != "netty-tcnative") { + useVersion("4.1.37.Final") + } + } + } + } + } +} + +tasks { + withType().configureEach { + systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean) + } +} + +tasks.withType().configureEach { + jvmArgs("-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true") +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DownstreamWrapper.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DownstreamWrapper.java new file mode 100644 index 000000000000..5bf5d2454fec --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DownstreamWrapper.java @@ -0,0 +1,55 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import ratpack.exec.Downstream; + +public class DownstreamWrapper implements Downstream { + + private final Downstream delegate; + private final Context parentContext; + + private DownstreamWrapper(Downstream delegate, Context parentContext) { + assert parentContext != null; + this.delegate = delegate; + this.parentContext = parentContext; + } + + @Override + public void success(T value) { + try (Scope ignored = parentContext.makeCurrent()) { + delegate.success(value); + } + } + + @Override + public void error(Throwable throwable) { + try (Scope ignored = parentContext.makeCurrent()) { + delegate.error(throwable); + } + } + + @Override + public void complete() { + try (Scope ignored = parentContext.makeCurrent()) { + delegate.complete(); + } + } + + public static Downstream wrapIfNeeded(Downstream delegate) { + if (delegate instanceof DownstreamWrapper) { + return delegate; + } + Context context = Context.current(); + if (context == Context.root()) { + // Skip wrapping, there is no need to propagate root context. + return delegate; + } + return new DownstreamWrapper<>(delegate, context); + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java new file mode 100644 index 000000000000..d14e25458201 --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java @@ -0,0 +1,46 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isStatic; +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 ratpack.http.client.HttpClient; + +public class HttpClientInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("ratpack.http.client.HttpClient"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod() + .and(isStatic()) + .and(named("of")) + .and(takesArgument(0, named("ratpack.func.Action"))), + HttpClientInstrumentation.class.getName() + "$OfAdvice"); + } + + @SuppressWarnings("unused") + public static class OfAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void injectTracing(@Advice.Return(readOnly = false) HttpClient httpClient) + throws Exception { + httpClient = RatpackSingletons.telemetry().instrumentHttpClient(httpClient); + } + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java new file mode 100644 index 000000000000..59232676bc1f --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; +import static java.util.Arrays.asList; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; +import java.util.List; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class RatpackInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { + public RatpackInstrumentationModule() { + super("ratpack", "ratpack-1.7"); + } + + @Override + public String getModuleGroup() { + // relies on netty + return "netty"; + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + // Only activate when running ratpack 1.7 or later + return hasClassesNamed("ratpack.exec.util.retry.Delay"); + } + + @Override + public List typeInstrumentations() { + return asList( + new ServerRegistryInstrumentation(), + new HttpClientInstrumentation(), + new RequestActionSupportInstrumentation()); + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackSingletons.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackSingletons.java new file mode 100644 index 000000000000..a0304c64a0b8 --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackSingletons.java @@ -0,0 +1,54 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import io.netty.channel.Channel; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys; +import io.opentelemetry.instrumentation.ratpack.v1_7.RatpackTelemetry; +import io.opentelemetry.instrumentation.ratpack.v1_7.internal.ContextHolder; +import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig; +import io.opentelemetry.javaagent.bootstrap.internal.ExperimentalConfig; +import ratpack.exec.Execution; + +public final class RatpackSingletons { + + static { + TELEMETRY = + RatpackTelemetry.builder(GlobalOpenTelemetry.get()) + .configure(AgentCommonConfig.get()) + .build(); + } + + private static final Instrumenter INSTRUMENTER = + Instrumenter.builder( + GlobalOpenTelemetry.get(), "io.opentelemetry.ratpack-1.7", s -> s) + .setEnabled(ExperimentalConfig.get().controllerTelemetryEnabled()) + .buildInstrumenter(); + + public static Instrumenter instrumenter() { + return INSTRUMENTER; + } + + private static final RatpackTelemetry TELEMETRY; + + public static RatpackTelemetry telemetry() { + return TELEMETRY; + } + + public static void propagateContextToChannel(Execution execution, Channel channel) { + Context parentContext = + execution + .maybeGet(ContextHolder.class) + .map(ContextHolder::context) + .orElse(Context.current()); + channel.attr(AttributeKeys.CLIENT_PARENT_CONTEXT).set(parentContext); + } + + private RatpackSingletons() {} +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java new file mode 100644 index 000000000000..a156ddb638ba --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java @@ -0,0 +1,84 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPrivate; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import io.netty.channel.Channel; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.ratpack.v1_7.internal.ContextHolder; +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 ratpack.exec.Downstream; +import ratpack.exec.Execution; + +public class RequestActionSupportInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return extendsClass(named("ratpack.http.client.internal.RequestActionSupport")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod() + .and(isPrivate()) + .and(named("send")) + .and(takesArgument(0, named("ratpack.exec.Downstream"))) + .and(takesArgument(1, named("io.netty.channel.Channel"))), + RequestActionSupportInstrumentation.class.getName() + "$SendAdvice"); + transformer.applyAdviceToMethod( + isMethod().and(named("connect")).and(takesArgument(0, named("ratpack.exec.Downstream"))), + RequestActionSupportInstrumentation.class.getName() + "$ConnectAdvice"); + } + + @SuppressWarnings("unused") + public static class SendAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void injectChannelAttribute( + @Advice.FieldValue("execution") Execution execution, + @Advice.Argument(value = 0, readOnly = false) Downstream downstream, + @Advice.Argument(value = 1, readOnly = false) Channel channel) { + RatpackSingletons.propagateContextToChannel(execution, channel); + } + } + + public static class ConnectAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope injectChannelAttribute( + @Advice.FieldValue("execution") Execution execution, + @Advice.Argument(value = 0, readOnly = false) Downstream downstream) { + // Propagate the current context to downstream + // since that the is the subsequent + downstream = DownstreamWrapper.wrapIfNeeded(downstream); + + // Capture the CLIENT span and make it current before cally Netty layer + return execution + .maybeGet(ContextHolder.class) + .map(ContextHolder::context) + .map(Context::makeCurrent) + .orElse(null); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void exit(@Advice.Enter Scope scope) { + if (scope != null) { + scope.close(); + } + } + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java new file mode 100644 index 000000000000..657a2e04cd07 --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java @@ -0,0 +1,42 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isStatic; +import static net.bytebuddy.matcher.ElementMatchers.named; + +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 ratpack.registry.Registry; + +public class ServerRegistryInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("ratpack.server.internal.ServerRegistry"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod().and(isStatic()).and(named("buildBaseRegistry")), + ServerRegistryInstrumentation.class.getName() + "$BuildAdvice"); + } + + @SuppressWarnings("unused") + public static class BuildAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) + throws Exception { + registry = registry.join(Registry.of(RatpackSingletons.telemetry()::configureServerRegistry)); + } + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackAsyncHttpServerTest.groovy b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackAsyncHttpServerTest.groovy new file mode 100644 index 000000000000..dc1cc42df27f --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackAsyncHttpServerTest.groovy @@ -0,0 +1,22 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package server + +import io.opentelemetry.instrumentation.ratpack.server.AbstractRatpackAsyncHttpServerTest +import io.opentelemetry.instrumentation.test.AgentTestTrait +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint +import ratpack.server.RatpackServerSpec + +class RatpackAsyncHttpServerTest extends AbstractRatpackAsyncHttpServerTest implements AgentTestTrait { + @Override + void configure(RatpackServerSpec serverSpec) { + } + + @Override + boolean hasResponseCustomizer(ServerEndpoint endpoint) { + true + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackForkedHttpServerTest.groovy b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackForkedHttpServerTest.groovy new file mode 100644 index 000000000000..87dc2848b1ff --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackForkedHttpServerTest.groovy @@ -0,0 +1,27 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package server + +import io.opentelemetry.instrumentation.ratpack.server.AbstractRatpackForkedHttpServerTest +import io.opentelemetry.instrumentation.test.AgentTestTrait +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint +import ratpack.server.RatpackServerSpec + +class RatpackForkedHttpServerTest extends AbstractRatpackForkedHttpServerTest implements AgentTestTrait { + @Override + void configure(RatpackServerSpec serverSpec) { + } + + @Override + boolean hasResponseCustomizer(ServerEndpoint endpoint) { + true + } + + @Override + boolean testHttpPipelining() { + false + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackHttpServerTest.groovy b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackHttpServerTest.groovy new file mode 100644 index 000000000000..3fd085dde585 --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -0,0 +1,22 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package server + +import io.opentelemetry.instrumentation.ratpack.server.AbstractRatpackHttpServerTest +import io.opentelemetry.instrumentation.test.AgentTestTrait +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint +import ratpack.server.RatpackServerSpec + +class RatpackHttpServerTest extends AbstractRatpackHttpServerTest implements AgentTestTrait { + @Override + void configure(RatpackServerSpec serverSpec) { + } + + @Override + boolean hasResponseCustomizer(ServerEndpoint endpoint) { + true + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy new file mode 100644 index 000000000000..e52bd4f93c1e --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy @@ -0,0 +1,21 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package server + +import io.opentelemetry.instrumentation.ratpack.server.AbstractRatpackRoutesTest +import io.opentelemetry.instrumentation.test.AgentTestTrait +import ratpack.server.RatpackServerSpec + +class RatpackRoutesTest extends AbstractRatpackRoutesTest implements AgentTestTrait { + @Override + void configure(RatpackServerSpec serverSpec) { + } + + @Override + boolean hasHandlerSpan() { + return true + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java new file mode 100644 index 000000000000..b9348fd8dcbc --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java @@ -0,0 +1,68 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import com.google.common.collect.ImmutableList; +import io.netty.channel.ConnectTimeoutException; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackForkedHttpClientTest; +import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.semconv.NetworkAttributes; +import java.net.URI; +import java.util.HashSet; +import java.util.Set; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.extension.RegisterExtension; +import ratpack.exec.internal.DefaultExecController; +import ratpack.http.client.HttpClientReadTimeoutException; + +class RatpackForkedHttpClientTest extends AbstractRatpackForkedHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + @BeforeAll + @Override + protected void setUpClient() throws Exception { + exec.run( + unused -> { + ((DefaultExecController) exec.getController()) + .setInitializers(ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE)); + client = buildHttpClient(); + singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); + }); + } + + @Override + protected Set> computeHttpAttributes(URI uri) { + Set> attributes = new HashSet<>(super.computeHttpAttributes(uri)); + attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION); + return attributes; + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setExpectedClientSpanNameMapper( + HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER); + optionsBuilder.setClientSpanErrorMapper( + (uri, exception) -> { + if (uri.toString().equals("https://192.0.2.1/")) { + return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri); + } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) { + return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri); + } else if (uri.getPath().equals("/read-timeout")) { + return new HttpClientReadTimeoutException( + "Read timeout (PT2S) waiting on HTTP server at " + uri); + } + return exception; + }); + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java new file mode 100644 index 000000000000..02f76cbc04e4 --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java @@ -0,0 +1,68 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import com.google.common.collect.ImmutableList; +import io.netty.channel.ConnectTimeoutException; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackHttpClientTest; +import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.semconv.NetworkAttributes; +import java.net.URI; +import java.util.HashSet; +import java.util.Set; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.extension.RegisterExtension; +import ratpack.exec.internal.DefaultExecController; +import ratpack.http.client.HttpClientReadTimeoutException; + +class RatpackHttpClientTest extends AbstractRatpackHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + @BeforeAll + @Override + protected void setUpClient() throws Exception { + exec.run( + unused -> { + ((DefaultExecController) exec.getController()) + .setInitializers(ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE)); + client = buildHttpClient(); + singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); + }); + } + + @Override + protected Set> computeHttpAttributes(URI uri) { + Set> attributes = new HashSet<>(super.computeHttpAttributes(uri)); + attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION); + return attributes; + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setExpectedClientSpanNameMapper( + HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER); + optionsBuilder.setClientSpanErrorMapper( + (uri, exception) -> { + if (uri.toString().equals("https://192.0.2.1/")) { + return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri); + } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) { + return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri); + } else if (uri.getPath().equals("/read-timeout")) { + return new HttpClientReadTimeoutException( + "Read timeout (PT2S) waiting on HTTP server at " + uri); + } + return exception; + }); + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java new file mode 100644 index 000000000000..80506e4d2fda --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java @@ -0,0 +1,68 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import com.google.common.collect.ImmutableList; +import io.netty.channel.ConnectTimeoutException; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackPooledHttpClientTest; +import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.semconv.NetworkAttributes; +import java.net.URI; +import java.util.HashSet; +import java.util.Set; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.extension.RegisterExtension; +import ratpack.exec.internal.DefaultExecController; +import ratpack.http.client.HttpClientReadTimeoutException; + +class RatpackPooledHttpClientTest extends AbstractRatpackPooledHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + @BeforeAll + @Override + protected void setUpClient() throws Exception { + exec.run( + unused -> { + ((DefaultExecController) exec.getController()) + .setInitializers(ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE)); + client = buildHttpClient(); + singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); + }); + } + + @Override + protected Set> computeHttpAttributes(URI uri) { + Set> attributes = new HashSet<>(super.computeHttpAttributes(uri)); + attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION); + return attributes; + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setExpectedClientSpanNameMapper( + HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER); + optionsBuilder.setClientSpanErrorMapper( + (uri, exception) -> { + if (uri.toString().equals("https://192.0.2.1/")) { + return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri); + } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) { + return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri); + } else if (uri.getPath().equals("/read-timeout")) { + return new HttpClientReadTimeoutException( + "Read timeout (PT2S) waiting on HTTP server at " + uri); + } + return exception; + }); + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInitializer.java b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInitializer.java index 5788db8c5655..bbedaee9caca 100644 --- a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInitializer.java +++ b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInitializer.java @@ -9,7 +9,7 @@ import ratpack.exec.ExecInitializer; import ratpack.exec.Execution; -final class OpenTelemetryExecInitializer implements ExecInitializer { +public final class OpenTelemetryExecInitializer implements ExecInitializer { public static final ExecInitializer INSTANCE = new OpenTelemetryExecInitializer(); @Override diff --git a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInterceptor.java b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInterceptor.java index f78c0671fe74..07c7a68abc5f 100644 --- a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInterceptor.java +++ b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryExecInterceptor.java @@ -11,9 +11,9 @@ import ratpack.exec.Execution; import ratpack.func.Block; -final class OpenTelemetryExecInterceptor implements ExecInterceptor { +public final class OpenTelemetryExecInterceptor implements ExecInterceptor { - static final ExecInterceptor INSTANCE = new OpenTelemetryExecInterceptor(); + public static final ExecInterceptor INSTANCE = new OpenTelemetryExecInterceptor(); @Override public void intercept(Execution execution, ExecType type, Block continuation) throws Exception { diff --git a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryHttpClient.java b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryHttpClient.java index aea5fc96faf6..3358f29b8fe9 100644 --- a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryHttpClient.java +++ b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/OpenTelemetryHttpClient.java @@ -44,18 +44,30 @@ public HttpClient instrument(HttpClient httpClient) throws Exception { httpClientSpec.responseIntercept( httpResponse -> { Execution execution = Execution.current(); - ContextHolder contextHolder = execution.get(ContextHolder.class); - execution.remove(ContextHolder.class); - instrumenter.end( - contextHolder.context(), contextHolder.requestSpec(), httpResponse, null); + execution + .maybeGet(ContextHolder.class) + .ifPresent( + contextHolder -> { + execution.remove(ContextHolder.class); + instrumenter.end( + contextHolder.context(), + contextHolder.requestSpec(), + httpResponse, + null); + }); }); httpClientSpec.errorIntercept( ex -> { Execution execution = Execution.current(); - ContextHolder contextHolder = execution.get(ContextHolder.class); - execution.remove(ContextHolder.class); - instrumenter.end(contextHolder.context(), contextHolder.requestSpec(), null, ex); + execution + .maybeGet(ContextHolder.class) + .ifPresent( + contextHolder -> { + execution.remove(ContextHolder.class); + instrumenter.end( + contextHolder.context(), contextHolder.requestSpec(), null, ex); + }); }); }); } diff --git a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/RatpackTelemetryBuilder.java b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/RatpackTelemetryBuilder.java index a7e56610913a..b285dbb4bde2 100644 --- a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/RatpackTelemetryBuilder.java +++ b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/v1_7/RatpackTelemetryBuilder.java @@ -9,6 +9,7 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.incubator.builder.internal.DefaultHttpClientInstrumenterBuilder; import io.opentelemetry.instrumentation.api.incubator.builder.internal.DefaultHttpServerInstrumenterBuilder; +import io.opentelemetry.instrumentation.api.incubator.config.internal.CommonConfig; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesExtractorBuilder; @@ -173,6 +174,13 @@ public RatpackTelemetryBuilder setServerSpanNameExtractor( return this; } + @CanIgnoreReturnValue + public RatpackTelemetryBuilder configure(CommonConfig config) { + clientBuilder.configure(config); + serverBuilder.configure(config); + return this; + } + /** Returns a new {@link RatpackTelemetry} with the configuration of this builder. */ public RatpackTelemetry build() { return new RatpackTelemetry(serverBuilder.build(), clientBuilder.build()); diff --git a/instrumentation/ratpack/ratpack-1.7/library/src/test/java/io/opentelemetry/instrumentation/ratpack/v1_7/AbstractRatpackHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/library/src/test/java/io/opentelemetry/instrumentation/ratpack/v1_7/AbstractRatpackHttpClientTest.java index d13cbcc65932..68057cae039e 100644 --- a/instrumentation/ratpack/ratpack-1.7/library/src/test/java/io/opentelemetry/instrumentation/ratpack/v1_7/AbstractRatpackHttpClientTest.java +++ b/instrumentation/ratpack/ratpack-1.7/library/src/test/java/io/opentelemetry/instrumentation/ratpack/v1_7/AbstractRatpackHttpClientTest.java @@ -148,16 +148,16 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) { } return exception; }); + optionsBuilder.setHttpAttributes(this::computeHttpAttributes); optionsBuilder.disableTestRedirects(); + // these tests will pass, but they don't really test anything since REQUEST is Void optionsBuilder.disableTestReusedRequest(); - optionsBuilder.setHttpAttributes(this::getHttpAttributes); - optionsBuilder.spanEndsAfterBody(); } - protected Set> getHttpAttributes(URI uri) { + protected Set> computeHttpAttributes(URI uri) { Set> attributes = new HashSet<>(HttpClientTestOptions.DEFAULT_HTTP_ATTRIBUTES); attributes.remove(NETWORK_PROTOCOL_VERSION); return attributes; diff --git a/settings.gradle.kts b/settings.gradle.kts index 8f897c9cade7..b2c3d2d0b1e5 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -507,6 +507,7 @@ include(":instrumentation:r2dbc-1.0:testing") include(":instrumentation:rabbitmq-2.7:javaagent") include(":instrumentation:ratpack:ratpack-1.4:javaagent") include(":instrumentation:ratpack:ratpack-1.4:testing") +include(":instrumentation:ratpack:ratpack-1.7:javaagent") include(":instrumentation:ratpack:ratpack-1.7:library") include(":instrumentation:reactor:reactor-3.1:javaagent") include(":instrumentation:reactor:reactor-3.1:library") From d8d4bf5695598f40a37d756ccab88fbeee30ae20 Mon Sep 17 00:00:00 2001 From: John Engelman Date: Wed, 6 Nov 2024 10:51:43 -0600 Subject: [PATCH 2/5] (chore) update from review --- .../ratpack-1.7/javaagent/build.gradle.kts | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts b/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts index dbc58cadd42d..b3645767cee1 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/build.gradle.kts @@ -14,34 +14,17 @@ dependencies { library("io.ratpack:ratpack-core:1.7.0") implementation(project(":instrumentation:netty:netty-4.1:library")) - implementation(project(":instrumentation:ratpack:ratpack-1.4:javaagent")) implementation(project(":instrumentation:ratpack:ratpack-1.7:library")) - testImplementation(project(":instrumentation:ratpack:ratpack-1.4:testing")) - testLibrary("io.ratpack:ratpack-test:1.7.0") + testImplementation(project(":instrumentation:ratpack:ratpack-1.4:testing")) + testInstrumentation(project(":instrumentation:ratpack:ratpack-1.4:javaagent")) if (JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_11)) { testImplementation("com.sun.activation:jakarta.activation:1.2.2") } } -// to allow all tests to pass we need to choose a specific netty version -if (!(findProperty("testLatestDeps") as Boolean)) { - configurations.configureEach { - if (!name.contains("muzzle")) { - resolutionStrategy { - eachDependency { - // specifying a fixed version for all libraries with io.netty group - if (requested.group == "io.netty" && requested.name != "netty-tcnative") { - useVersion("4.1.37.Final") - } - } - } - } - } -} - tasks { withType().configureEach { systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean) From 2ddcf3a67ba67e030d78035fd7c6d4107faee4b6 Mon Sep 17 00:00:00 2001 From: John Engelman Date: Wed, 6 Nov 2024 18:30:12 -0600 Subject: [PATCH 3/5] (bug) apply exec controller classes using instrumentation --- .../DefaultExecControllerInstrumentation.java | 86 +++++++++++++++++++ .../v1_7/RatpackInstrumentationModule.java | 1 + .../v1_7/ServerRegistryInstrumentation.java | 10 ++- .../v1_7/RatpackForkedHttpClientTest.java | 16 ---- .../ratpack/v1_7/RatpackHttpClientTest.java | 16 ---- .../v1_7/RatpackPooledHttpClientTest.java | 16 ---- 6 files changed, 94 insertions(+), 51 deletions(-) create mode 100644 instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java new file mode 100644 index 000000000000..7bdef2aaa609 --- /dev/null +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java @@ -0,0 +1,86 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; + +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.common.collect.ImmutableList; +import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; +import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInterceptor; +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 ratpack.exec.ExecInitializer; +import ratpack.exec.ExecInterceptor; + +public class DefaultExecControllerInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("ratpack.exec.internal.DefaultExecController"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("setInitializers") + .and(takesArgument(0, named("com.google.common.collect.ImmutableList"))), + DefaultExecControllerInstrumentation.class.getName() + "$SetInitializersAdvice"); + + transformer.applyAdviceToMethod( + named("setInterceptors") + .and(takesArgument(0, named("com.google.common.collect.ImmutableList"))), + DefaultExecControllerInstrumentation.class.getName() + "$SetInterceptorsAdvice"); + + transformer.applyAdviceToMethod( + isConstructor(), + DefaultExecControllerInstrumentation.class.getName() + "$ConstructorAdvice"); + } + + public static class SetInitializersAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void enter( + @Advice.Argument(value = 0, readOnly = false) + ImmutableList initializers) { + initializers = + ImmutableList.builder() + .addAll(initializers) + .add(OpenTelemetryExecInitializer.INSTANCE) + .build(); + } + } + + public static class SetInterceptorsAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void enter( + @Advice.Argument(value = 0, readOnly = false) + ImmutableList interceptors) { + interceptors = + ImmutableList.builder() + .addAll(interceptors) + .add(OpenTelemetryExecInterceptor.INSTANCE) + .build(); + } + } + + public static class ConstructorAdvice { + + @SuppressWarnings("UnusedVariable") + @Advice.OnMethodExit(suppress = Throwable.class) + public static void exit( + @Advice.FieldValue(value = "initializers", readOnly = false) + ImmutableList initializers, + @Advice.FieldValue(value = "interceptors", readOnly = false) + ImmutableList interceptors) { + initializers = ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE); + interceptors = ImmutableList.of(OpenTelemetryExecInterceptor.INSTANCE); + } + } +} diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java index 59232676bc1f..a98e61d4e0f2 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java @@ -37,6 +37,7 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public List typeInstrumentations() { return asList( + new DefaultExecControllerInstrumentation(), new ServerRegistryInstrumentation(), new HttpClientInstrumentation(), new RequestActionSupportInstrumentation()); diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java index 657a2e04cd07..df23c3a650e9 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java @@ -14,6 +14,7 @@ import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import ratpack.handling.HandlerDecorator; import ratpack.registry.Registry; public class ServerRegistryInstrumentation implements TypeInstrumentation { @@ -34,9 +35,12 @@ public void transform(TypeTransformer transformer) { public static class BuildAdvice { @Advice.OnMethodExit(suppress = Throwable.class) - public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) - throws Exception { - registry = registry.join(Registry.of(RatpackSingletons.telemetry()::configureServerRegistry)); + public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) { + registry = + registry.join( + Registry.single( + HandlerDecorator.prepend( + RatpackSingletons.telemetry().getOpenTelemetryServerHandler()))); } } } diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java index b9348fd8dcbc..36fd784b9242 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java @@ -5,11 +5,9 @@ package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; -import com.google.common.collect.ImmutableList; import io.netty.channel.ConnectTimeoutException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackForkedHttpClientTest; -import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; @@ -17,10 +15,8 @@ import java.net.URI; import java.util.HashSet; import java.util.Set; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.extension.RegisterExtension; -import ratpack.exec.internal.DefaultExecController; import ratpack.http.client.HttpClientReadTimeoutException; class RatpackForkedHttpClientTest extends AbstractRatpackForkedHttpClientTest { @@ -28,18 +24,6 @@ class RatpackForkedHttpClientTest extends AbstractRatpackForkedHttpClientTest { @RegisterExtension static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); - @BeforeAll - @Override - protected void setUpClient() throws Exception { - exec.run( - unused -> { - ((DefaultExecController) exec.getController()) - .setInitializers(ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE)); - client = buildHttpClient(); - singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); - }); - } - @Override protected Set> computeHttpAttributes(URI uri) { Set> attributes = new HashSet<>(super.computeHttpAttributes(uri)); diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java index 02f76cbc04e4..4955089b76e8 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java @@ -5,11 +5,9 @@ package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; -import com.google.common.collect.ImmutableList; import io.netty.channel.ConnectTimeoutException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackHttpClientTest; -import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; @@ -17,10 +15,8 @@ import java.net.URI; import java.util.HashSet; import java.util.Set; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.extension.RegisterExtension; -import ratpack.exec.internal.DefaultExecController; import ratpack.http.client.HttpClientReadTimeoutException; class RatpackHttpClientTest extends AbstractRatpackHttpClientTest { @@ -28,18 +24,6 @@ class RatpackHttpClientTest extends AbstractRatpackHttpClientTest { @RegisterExtension static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); - @BeforeAll - @Override - protected void setUpClient() throws Exception { - exec.run( - unused -> { - ((DefaultExecController) exec.getController()) - .setInitializers(ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE)); - client = buildHttpClient(); - singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); - }); - } - @Override protected Set> computeHttpAttributes(URI uri) { Set> attributes = new HashSet<>(super.computeHttpAttributes(uri)); diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java index 80506e4d2fda..6a98d8f0a8d9 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java @@ -5,11 +5,9 @@ package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7; -import com.google.common.collect.ImmutableList; import io.netty.channel.ConnectTimeoutException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackPooledHttpClientTest; -import io.opentelemetry.instrumentation.ratpack.v1_7.OpenTelemetryExecInitializer; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; @@ -17,10 +15,8 @@ import java.net.URI; import java.util.HashSet; import java.util.Set; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.extension.RegisterExtension; -import ratpack.exec.internal.DefaultExecController; import ratpack.http.client.HttpClientReadTimeoutException; class RatpackPooledHttpClientTest extends AbstractRatpackPooledHttpClientTest { @@ -28,18 +24,6 @@ class RatpackPooledHttpClientTest extends AbstractRatpackPooledHttpClientTest { @RegisterExtension static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); - @BeforeAll - @Override - protected void setUpClient() throws Exception { - exec.run( - unused -> { - ((DefaultExecController) exec.getController()) - .setInitializers(ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE)); - client = buildHttpClient(); - singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); - }); - } - @Override protected Set> computeHttpAttributes(URI uri) { Set> attributes = new HashSet<>(super.computeHttpAttributes(uri)); From 093aeaab3e134628372511129d90e2433c2ddb85 Mon Sep 17 00:00:00 2001 From: John Engelman Date: Wed, 6 Nov 2024 21:20:18 -0600 Subject: [PATCH 4/5] (chore) make indy compatible --- .../DefaultExecControllerInstrumentation.java | 59 +++++++++++-------- .../v1_7/HttpClientInstrumentation.java | 8 +-- .../v1_7/RatpackInstrumentationModule.java | 5 ++ .../RequestActionSupportInstrumentation.java | 35 +++++++---- .../v1_7/ServerRegistryInstrumentation.java | 14 ++--- 5 files changed, 72 insertions(+), 49 deletions(-) diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java index 7bdef2aaa609..3d16352fbe5a 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java @@ -15,6 +15,8 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; +import net.bytebuddy.asm.Advice.AssignReturned.ToFields.ToField; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import ratpack.exec.ExecInitializer; @@ -44,43 +46,48 @@ public void transform(TypeTransformer transformer) { DefaultExecControllerInstrumentation.class.getName() + "$ConstructorAdvice"); } + @SuppressWarnings("unused") public static class SetInitializersAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void enter( - @Advice.Argument(value = 0, readOnly = false) - ImmutableList initializers) { - initializers = - ImmutableList.builder() - .addAll(initializers) - .add(OpenTelemetryExecInitializer.INSTANCE) - .build(); + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToArguments(@ToArgument(0)) + public static ImmutableList enter( + @Advice.Argument(0) ImmutableList initializers) { + return ImmutableList.builder() + .addAll(initializers) + .add(OpenTelemetryExecInitializer.INSTANCE) + .build(); } } + @SuppressWarnings("unused") public static class SetInterceptorsAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void enter( - @Advice.Argument(value = 0, readOnly = false) - ImmutableList interceptors) { - interceptors = - ImmutableList.builder() - .addAll(interceptors) - .add(OpenTelemetryExecInterceptor.INSTANCE) - .build(); + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToArguments(@ToArgument(0)) + public static ImmutableList enter( + @Advice.Argument(0) ImmutableList interceptors) { + return ImmutableList.builder() + .addAll(interceptors) + .add(OpenTelemetryExecInterceptor.INSTANCE) + .build(); } } + @SuppressWarnings("unused") public static class ConstructorAdvice { @SuppressWarnings("UnusedVariable") - @Advice.OnMethodExit(suppress = Throwable.class) - public static void exit( - @Advice.FieldValue(value = "initializers", readOnly = false) - ImmutableList initializers, - @Advice.FieldValue(value = "interceptors", readOnly = false) - ImmutableList interceptors) { - initializers = ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE); - interceptors = ImmutableList.of(OpenTelemetryExecInterceptor.INSTANCE); + @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToFields({ + @ToField(value = "initializers", index = 0), + @ToField(value = "interceptors", index = 1) + }) + public static Object[] exit( + @Advice.FieldValue("initializers") ImmutableList initializers, + @Advice.FieldValue("interceptors") ImmutableList interceptors) { + return new Object[] { + ImmutableList.of(OpenTelemetryExecInitializer.INSTANCE), + ImmutableList.of(OpenTelemetryExecInterceptor.INSTANCE) + }; } } } diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java index d14e25458201..0fcdef428d80 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java @@ -37,10 +37,10 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class OfAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void injectTracing(@Advice.Return(readOnly = false) HttpClient httpClient) - throws Exception { - httpClient = RatpackSingletons.telemetry().instrumentHttpClient(httpClient); + @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToReturned + public static HttpClient injectTracing(@Advice.Return HttpClient httpClient) throws Exception { + return RatpackSingletons.telemetry().instrumentHttpClient(httpClient); } } } diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java index a98e61d4e0f2..3f00fa32bfc0 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java @@ -28,6 +28,11 @@ public String getModuleGroup() { return "netty"; } + @Override + public boolean isIndyModule() { + return true; + } + @Override public ElementMatcher.Junction classLoaderMatcher() { // Only activate when running ratpack 1.7 or later diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java index a156ddb638ba..e57b93735fd7 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java @@ -18,6 +18,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import ratpack.exec.Downstream; @@ -41,7 +42,10 @@ public void transform(TypeTransformer transformer) { RequestActionSupportInstrumentation.class.getName() + "$SendAdvice"); transformer.applyAdviceToMethod( isMethod().and(named("connect")).and(takesArgument(0, named("ratpack.exec.Downstream"))), - RequestActionSupportInstrumentation.class.getName() + "$ConnectAdvice"); + RequestActionSupportInstrumentation.class.getName() + "$ConnectDownstreamAdvice"); + transformer.applyAdviceToMethod( + isMethod().and(named("connect")).and(takesArgument(0, named("ratpack.exec.Downstream"))), + RequestActionSupportInstrumentation.class.getName() + "$ContextAdvice"); } @SuppressWarnings("unused") @@ -49,22 +53,29 @@ public static class SendAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void injectChannelAttribute( - @Advice.FieldValue("execution") Execution execution, - @Advice.Argument(value = 0, readOnly = false) Downstream downstream, - @Advice.Argument(value = 1, readOnly = false) Channel channel) { + @Advice.FieldValue("execution") Execution execution, @Advice.Argument(1) Channel channel) { RatpackSingletons.propagateContextToChannel(execution, channel); } } - public static class ConnectAdvice { + @SuppressWarnings("unused") + public static class ConnectDownstreamAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope injectChannelAttribute( - @Advice.FieldValue("execution") Execution execution, - @Advice.Argument(value = 0, readOnly = false) Downstream downstream) { + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToArguments(@ToArgument(0)) + public static Object wrapDownstream(@Advice.Argument(0) Downstream downstream) { // Propagate the current context to downstream - // since that the is the subsequent - downstream = DownstreamWrapper.wrapIfNeeded(downstream); + // since that is the subsequent code chained to the http client call + return DownstreamWrapper.wrapIfNeeded(downstream); + } + } + + @SuppressWarnings("unused") + public static class ContextAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static Scope injectChannelAttribute( + @Advice.FieldValue("execution") Execution execution) { // Capture the CLIENT span and make it current before cally Netty layer return execution @@ -74,7 +85,7 @@ public static Scope injectChannelAttribute( .orElse(null); } - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false) public static void exit(@Advice.Enter Scope scope) { if (scope != null) { scope.close(); diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java index df23c3a650e9..50d055bdb991 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java @@ -34,13 +34,13 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class BuildAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) { - registry = - registry.join( - Registry.single( - HandlerDecorator.prepend( - RatpackSingletons.telemetry().getOpenTelemetryServerHandler()))); + @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToReturned + public static Registry injectTracing(@Advice.Return Registry registry) { + return registry.join( + Registry.single( + HandlerDecorator.prepend( + RatpackSingletons.telemetry().getOpenTelemetryServerHandler()))); } } } From c7e5d8b8b946c86d7538ee9b3cf3cf7c0aaae988 Mon Sep 17 00:00:00 2001 From: John Engelman Date: Thu, 14 Nov 2024 10:26:10 -0600 Subject: [PATCH 5/5] (chore) remove indy specific inlining per review --- .../ratpack/v1_7/DefaultExecControllerInstrumentation.java | 6 +++--- .../ratpack/v1_7/HttpClientInstrumentation.java | 2 +- .../ratpack/v1_7/RatpackInstrumentationModule.java | 5 ----- .../ratpack/v1_7/RequestActionSupportInstrumentation.java | 6 +++--- .../ratpack/v1_7/ServerRegistryInstrumentation.java | 2 +- 5 files changed, 8 insertions(+), 13 deletions(-) diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java index 3d16352fbe5a..fb464efece3c 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/DefaultExecControllerInstrumentation.java @@ -48,7 +48,7 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class SetInitializersAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.OnMethodEnter(suppress = Throwable.class) @Advice.AssignReturned.ToArguments(@ToArgument(0)) public static ImmutableList enter( @Advice.Argument(0) ImmutableList initializers) { @@ -61,7 +61,7 @@ public static ImmutableList enter( @SuppressWarnings("unused") public static class SetInterceptorsAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.OnMethodEnter(suppress = Throwable.class) @Advice.AssignReturned.ToArguments(@ToArgument(0)) public static ImmutableList enter( @Advice.Argument(0) ImmutableList interceptors) { @@ -76,7 +76,7 @@ public static ImmutableList enter( public static class ConstructorAdvice { @SuppressWarnings("UnusedVariable") - @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.OnMethodExit(suppress = Throwable.class) @Advice.AssignReturned.ToFields({ @ToField(value = "initializers", index = 0), @ToField(value = "interceptors", index = 1) diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java index 0fcdef428d80..1c6b34b1ff34 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/HttpClientInstrumentation.java @@ -37,7 +37,7 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class OfAdvice { - @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.OnMethodExit(suppress = Throwable.class) @Advice.AssignReturned.ToReturned public static HttpClient injectTracing(@Advice.Return HttpClient httpClient) throws Exception { return RatpackSingletons.telemetry().instrumentHttpClient(httpClient); diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java index 3f00fa32bfc0..a98e61d4e0f2 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackInstrumentationModule.java @@ -28,11 +28,6 @@ public String getModuleGroup() { return "netty"; } - @Override - public boolean isIndyModule() { - return true; - } - @Override public ElementMatcher.Junction classLoaderMatcher() { // Only activate when running ratpack 1.7 or later diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java index e57b93735fd7..16952ab1c4d4 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java @@ -61,7 +61,7 @@ public static void injectChannelAttribute( @SuppressWarnings("unused") public static class ConnectDownstreamAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.OnMethodEnter(suppress = Throwable.class) @Advice.AssignReturned.ToArguments(@ToArgument(0)) public static Object wrapDownstream(@Advice.Argument(0) Downstream downstream) { // Propagate the current context to downstream @@ -73,7 +73,7 @@ public static Object wrapDownstream(@Advice.Argument(0) Downstream downstream @SuppressWarnings("unused") public static class ContextAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope injectChannelAttribute( @Advice.FieldValue("execution") Execution execution) { @@ -85,7 +85,7 @@ public static Scope injectChannelAttribute( .orElse(null); } - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false) + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void exit(@Advice.Enter Scope scope) { if (scope != null) { scope.close(); diff --git a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java index 50d055bdb991..7484d9637afc 100644 --- a/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java +++ b/instrumentation/ratpack/ratpack-1.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/ServerRegistryInstrumentation.java @@ -34,7 +34,7 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class BuildAdvice { - @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.OnMethodExit(suppress = Throwable.class) @Advice.AssignReturned.ToReturned public static Registry injectTracing(@Advice.Return Registry registry) { return registry.join(