From b0ec59e2b47a4fd27a30de6a39856eed1a016c3b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 21 May 2025 17:09:27 +0200 Subject: [PATCH 1/8] apache httpclient indy-ready --- .../v2_0/ApacheHttpClientInstrumentation.java | 50 ++-- ...ApacheHttpClientInstrumentationModule.java | 9 +- .../v4_0/ApacheHttpClientInstrumentation.java | 186 ++++++-------- ...ApacheHttpClientInstrumentationModule.java | 9 +- .../ApacheHttpAsyncClientInstrumentation.java | 24 +- .../v5_0/ApacheHttpClientInstrumentation.java | 227 +++++++++--------- ...ApacheHttpClientInstrumentationModule.java | 9 +- 7 files changed, 262 insertions(+), 252 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java index 765ac61ec642..0613fe86be6a 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java @@ -18,6 +18,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -47,32 +48,45 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class ExecuteMethodAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(1) HttpMethod httpMethod, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); - if (!instrumenter().shouldStart(parentContext, httpMethod)) { - return; + public static class AdviceScope { + private final Context context; + private final Scope scope; + + private AdviceScope(Context context, Scope scope) { + this.context = context; + this.scope = scope; } - context = instrumenter().start(parentContext, httpMethod); - scope = context.makeCurrent(); + @Nullable + public static AdviceScope start(HttpMethod httpMethod) { + Context parentContext = currentContext(); + if (!instrumenter().shouldStart(parentContext, httpMethod)) { + return null; + } + Context context = instrumenter().start(parentContext, httpMethod); + return new AdviceScope(context, context.makeCurrent()); + } + + public void end(HttpMethod httpMethod, Throwable throwable) { + scope.close(); + instrumenter().end(context, httpMethod, httpMethod, throwable); + } + } + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AdviceScope methodEnter(@Advice.Argument(1) HttpMethod httpMethod) { + return AdviceScope.start(httpMethod); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Argument(1) HttpMethod httpMethod, - @Advice.Thrown Throwable throwable, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Thrown @Nullable Throwable throwable, + @Advice.Enter @Nullable AdviceScope adviceScope) { - scope.close(); - instrumenter().end(context, httpMethod, httpMethod, throwable); + if (adviceScope != null) { + adviceScope.end(httpMethod, throwable); + } } } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentationModule.java b/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentationModule.java index 6b6bd1897a12..ef649105013d 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentationModule.java +++ b/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentationModule.java @@ -10,10 +10,12 @@ 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; @AutoService(InstrumentationModule.class) -public class ApacheHttpClientInstrumentationModule extends InstrumentationModule { +public class ApacheHttpClientInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public ApacheHttpClientInstrumentationModule() { super("apache-httpclient", "apache-httpclient-2.0"); @@ -23,4 +25,9 @@ public ApacheHttpClientInstrumentationModule() { public List typeInstrumentations() { return singletonList(new ApacheHttpClientInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java index 09b38bd89f3b..a7f2678b6330 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java @@ -20,7 +20,10 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; +import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.apache.http.HttpHost; @@ -123,87 +126,88 @@ public void transform(TypeTransformer transformer) { this.getClass().getName() + "$RequestWithHandlerAdvice"); } - @SuppressWarnings("unused") - public static class UriRequestAdvice { + public static class AdviceScope { + private final ApacheHttpClientRequest otelRequest; + private final Context parentContext; + private final Context context; + private final Scope scope; + + private AdviceScope( + ApacheHttpClientRequest otelRequest, Context parentContext, Context context, Scope scope) { + this.otelRequest = otelRequest; + this.parentContext = parentContext; + this.context = context; + this.scope = scope; + } - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(0) HttpUriRequest request, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Nullable + public static AdviceScope start(ApacheHttpClientRequest otelRequest) { Context parentContext = currentContext(); - - otelRequest = new ApacheHttpClientRequest(request); - if (!instrumenter().shouldStart(parentContext, otelRequest)) { - return; + return null; } - - context = instrumenter().start(parentContext, otelRequest); - scope = context.makeCurrent(); + Context context = instrumenter().start(parentContext, otelRequest); + return new AdviceScope(otelRequest, parentContext, context, context.makeCurrent()); } - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void methodExit( - @Advice.Argument(0) HttpUriRequest request, - @Advice.Return Object result, - @Advice.Thrown Throwable throwable, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + public ResponseHandler wrapHandler(ResponseHandler handler) { + return new WrappingStatusSettingResponseHandler<>( + context, parentContext, otelRequest, handler); + } + public void end(@Nullable Object result, @Nullable Throwable throwable) { scope.close(); ApacheHttpClientHelper.doMethodExit(context, otelRequest, result, throwable); } } @SuppressWarnings("unused") - public static class UriRequestWithHandlerAdvice { + public static class UriRequestAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(0) HttpUriRequest request, - @Advice.Argument(value = 1, readOnly = false) ResponseHandler handler, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); + public static AdviceScope methodEnter(@Advice.Argument(0) HttpUriRequest request) { + return AdviceScope.start(new ApacheHttpClientRequest(request)); + } - otelRequest = new ApacheHttpClientRequest(request); + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void methodExit( + @Advice.Argument(0) HttpUriRequest request, + @Advice.Return @Nullable Object result, + @Advice.Thrown @Nullable Throwable throwable, + @Advice.Enter @Nullable AdviceScope adviceScope) { - if (!instrumenter().shouldStart(parentContext, otelRequest)) { - return; + if (adviceScope != null) { + adviceScope.end(result, throwable); } + } + } - context = instrumenter().start(parentContext, otelRequest); - scope = context.makeCurrent(); + @SuppressWarnings("unused") + public static class UriRequestWithHandlerAdvice { + + @AssignReturned.ToArguments(@ToArgument(value = 1, index = 1)) + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Object[] methodEnter( + @Advice.Argument(0) HttpUriRequest request, + @Advice.Argument(1) ResponseHandler handler) { + AdviceScope adviceScope = AdviceScope.start(new ApacheHttpClientRequest(request)); // Wrap the handler so we capture the status code - if (handler != null) { - handler = - new WrappingStatusSettingResponseHandler<>( - context, parentContext, otelRequest, handler); - } + return new Object[] { + adviceScope, adviceScope == null ? handler : adviceScope.wrapHandler(handler) + }; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Argument(0) HttpUriRequest request, @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter Object[] enterResult) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, otelRequest, result, throwable); + AdviceScope adviceScope = (AdviceScope) enterResult[0]; + if (adviceScope != null) { + adviceScope.end(result, throwable); + } } } @@ -211,83 +215,51 @@ public static void methodExit( public static class RequestAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(0) HttpHost host, - @Advice.Argument(1) HttpRequest request, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); - - otelRequest = new ApacheHttpClientRequest(host, request); - - if (!instrumenter().shouldStart(parentContext, otelRequest)) { - return; - } - - context = instrumenter().start(parentContext, otelRequest); - scope = context.makeCurrent(); + public static AdviceScope methodEnter( + @Advice.Argument(0) HttpHost host, @Advice.Argument(1) HttpRequest request) { + return AdviceScope.start(new ApacheHttpClientRequest(host, request)); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter AdviceScope adviceScope) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, otelRequest, result, throwable); + if (adviceScope != null) { + adviceScope.end(result, throwable); + } } } @SuppressWarnings("unused") public static class RequestWithHandlerAdvice { + @AssignReturned.ToArguments(@ToArgument(value = 2, index = 1)) @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( + public static Object[] methodEnter( @Advice.Argument(0) HttpHost host, @Advice.Argument(1) HttpRequest request, - @Advice.Argument(value = 2, readOnly = false) ResponseHandler handler, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); - - otelRequest = new ApacheHttpClientRequest(host, request); - - if (!instrumenter().shouldStart(parentContext, otelRequest)) { - return; - } - - context = instrumenter().start(parentContext, otelRequest); - scope = context.makeCurrent(); - - // Wrap the handler so we capture the status code - if (handler != null) { - handler = - new WrappingStatusSettingResponseHandler<>( - context, parentContext, otelRequest, handler); - } + @Advice.Argument(2) ResponseHandler handler) { + + AdviceScope adviceScope = AdviceScope.start(new ApacheHttpClientRequest(host, request)); + return new Object[] { + adviceScope, + // Wrap the handler so we capture the status code + adviceScope == null ? handler : adviceScope.wrapHandler(handler) + }; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelRequest") ApacheHttpClientRequest otelRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter Object[] enterResult) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, otelRequest, result, throwable); + AdviceScope adviceScope = (AdviceScope) enterResult[0]; + if (adviceScope != null) { + adviceScope.end(result, throwable); + } } } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentationModule.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentationModule.java index b0821b7dcfc3..c89fc473d45c 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentationModule.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentationModule.java @@ -10,10 +10,12 @@ 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; @AutoService(InstrumentationModule.class) -public class ApacheHttpClientInstrumentationModule extends InstrumentationModule { +public class ApacheHttpClientInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public ApacheHttpClientInstrumentationModule() { super("apache-httpclient", "apache-httpclient-4.0"); @@ -23,4 +25,9 @@ public ApacheHttpClientInstrumentationModule() { public List typeInstrumentations() { return singletonList(new ApacheHttpClientInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java index 5a6885710e0e..8a4dac1b5458 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java @@ -23,6 +23,8 @@ import java.util.logging.Logger; import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; +import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.apache.hc.core5.concurrent.FutureCallback; @@ -66,11 +68,17 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class ClientAdvice { + @AssignReturned.ToArguments({ + @ToArgument(value = 0, index = 0), + @ToArgument(value = 3, index = 1), + @ToArgument(value = 4, index = 2) + }) @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(value = 0, readOnly = false) AsyncRequestProducer requestProducer, - @Advice.Argument(value = 3, readOnly = false) HttpContext httpContext, - @Advice.Argument(value = 4, readOnly = false) FutureCallback futureCallback) { + public static Object[] methodEnter( + @Advice.Argument(0) AsyncRequestProducer requestProducer, + @Advice.Argument(3) HttpContext originalHttpContext, + @Advice.Argument(4) FutureCallback futureCallback) { + HttpContext httpContext = originalHttpContext; Context parentContext = currentContext(); if (httpContext == null) { @@ -79,9 +87,11 @@ public static void methodEnter( WrappedFutureCallback wrappedFutureCallback = new WrappedFutureCallback<>(parentContext, httpContext, futureCallback); - requestProducer = - new DelegatingRequestProducer(parentContext, requestProducer, wrappedFutureCallback); - futureCallback = wrappedFutureCallback; + return new Object[] { + new DelegatingRequestProducer(parentContext, requestProducer, wrappedFutureCallback), + httpContext, + wrappedFutureCallback + }; } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index b5897ec9ecc6..ee2c6477cea7 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -20,7 +20,10 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; +import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.apache.hc.core5.http.ClassicHttpRequest; @@ -121,61 +124,80 @@ public void transform(TypeTransformer transformer) { this.getClass().getName() + "$RequestWithHostAndContextAndHandlerAdvice"); } - @SuppressWarnings("unused") - public static class RequestAdvice { + public static class AdviceLocals { + public final ClassicHttpRequest request; + public final Context parentContext; + public final Context context; + public final Scope scope; + + private AdviceLocals( + ClassicHttpRequest request, Context parentContext, Context context, Scope scope) { + this.request = request; + this.context = context; + this.parentContext = parentContext; + this.scope = scope; + } - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(0) ClassicHttpRequest request, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Nullable + public static AdviceLocals start(ClassicHttpRequest request) { Context parentContext = currentContext(); + if (!instrumenter().shouldStart(parentContext, request)) { - return; + return null; } + Context context = instrumenter().start(parentContext, request); + return new AdviceLocals(request, parentContext, context, context.makeCurrent()); + } - context = instrumenter().start(parentContext, request); - scope = context.makeCurrent(); + public void end(Object result, Throwable throwable) { + scope.close(); + ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); + } + } + + @SuppressWarnings("unused") + public static class RequestAdvice { + + @Nullable + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AdviceLocals methodEnter(@Advice.Argument(0) ClassicHttpRequest request) { + return AdviceLocals.start(request); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Argument(0) ClassicHttpRequest request, @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter @Nullable AdviceLocals locals) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); + if (locals != null) { + locals.end(result, throwable); + } } } @SuppressWarnings("unused") public static class RequestWithHandlerAdvice { + @AssignReturned.ToArguments(@ToArgument(value = 1, index = 1)) @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( + public static Object[] methodEnter( @Advice.Argument(0) ClassicHttpRequest request, - @Advice.Argument(value = 1, readOnly = false) HttpClientResponseHandler handler, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); - if (!instrumenter().shouldStart(parentContext, request)) { - return; - } + @Advice.Argument(1) HttpClientResponseHandler originalHandler) { - context = instrumenter().start(parentContext, request); - scope = context.makeCurrent(); + HttpClientResponseHandler handler = originalHandler; + AdviceLocals locals = AdviceLocals.start(request); + if (locals == null) { + return new Object[] {null, handler}; + } // Wrap the handler so we capture the status code if (handler != null) { handler = - new WrappingStatusSettingResponseHandler<>(context, parentContext, request, handler); + new WrappingStatusSettingResponseHandler<>( + locals.context, locals.parentContext, request, handler); } + return new Object[] {locals, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -183,39 +205,37 @@ public static void methodExit( @Advice.Argument(0) ClassicHttpRequest request, @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter Object[] enterResult) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); + AdviceLocals locals = (AdviceLocals) enterResult[0]; + if (locals != null) { + locals.end(result, throwable); + } } } @SuppressWarnings("unused") public static class RequestWithContextAndHandlerAdvice { + @AssignReturned.ToArguments(@ToArgument(value = 2, index = 1)) @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( + public static Object[] methodEnter( @Advice.Argument(0) ClassicHttpRequest request, - @Advice.Argument(value = 2, readOnly = false) HttpClientResponseHandler handler, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); - if (!instrumenter().shouldStart(parentContext, request)) { - return; - } + @Advice.Argument(2) HttpClientResponseHandler originalHandler) { - context = instrumenter().start(parentContext, request); - scope = context.makeCurrent(); + HttpClientResponseHandler handler = originalHandler; + AdviceLocals locals = AdviceLocals.start(request); + if (locals == null) { + return new Object[] {null, handler}; + } // Wrap the handler so we capture the status code if (handler != null) { handler = - new WrappingStatusSettingResponseHandler<>(context, parentContext, request, handler); + new WrappingStatusSettingResponseHandler<>( + locals.context, locals.parentContext, request, handler); } + return new Object[] {locals, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -223,140 +243,113 @@ public static void methodExit( @Advice.Argument(0) ClassicHttpRequest request, @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter Object[] enterResult) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); + AdviceLocals locals = (AdviceLocals) enterResult[0]; + if (locals != null) { + locals.end(result, throwable); + } } } @SuppressWarnings("unused") public static class RequestWithHostAdvice { + @Nullable @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.Argument(0) HttpHost host, - @Advice.Argument(1) ClassicHttpRequest request, - @Advice.Local("otelFullRequest") ClassicHttpRequest fullRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - Context parentContext = currentContext(); - fullRequest = new RequestWithHost(host, request); - if (!instrumenter().shouldStart(parentContext, fullRequest)) { - return; - } + public static AdviceLocals methodEnter( + @Advice.Argument(0) HttpHost host, @Advice.Argument(1) ClassicHttpRequest request) { - context = instrumenter().start(parentContext, fullRequest); - scope = context.makeCurrent(); + return AdviceLocals.start(new RequestWithHost(host, request)); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelFullRequest") ClassicHttpRequest fullRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter @Nullable AdviceLocals locals) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, fullRequest, result, throwable); + if (locals != null) { + locals.end(result, throwable); + } } } @SuppressWarnings("unused") public static class RequestWithHostAndHandlerAdvice { + @AssignReturned.ToArguments(@ToArgument(value = 2, index = 1)) @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( + public static Object[] methodEnter( @Advice.Argument(0) HttpHost host, @Advice.Argument(1) ClassicHttpRequest request, - @Advice.Argument(value = 2, readOnly = false) HttpClientResponseHandler handler, - @Advice.Local("otelFullRequest") ClassicHttpRequest fullRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Advice.Argument(2) HttpClientResponseHandler originalHandler) { - Context parentContext = currentContext(); - fullRequest = new RequestWithHost(host, request); - if (!instrumenter().shouldStart(parentContext, fullRequest)) { - return; - } + HttpClientResponseHandler handler = originalHandler; + AdviceLocals locals = AdviceLocals.start(new RequestWithHost(host, request)); - context = instrumenter().start(parentContext, fullRequest); - scope = context.makeCurrent(); + if (locals == null) { + return new Object[] {null, handler}; + } // Wrap the handler so we capture the status code if (handler != null) { handler = new WrappingStatusSettingResponseHandler<>( - context, parentContext, fullRequest, handler); + locals.context, locals.parentContext, locals.request, handler); } + return new Object[] {locals, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelFullRequest") ClassicHttpRequest fullRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter Object[] enterResult) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, fullRequest, result, throwable); + AdviceLocals locals = (AdviceLocals) enterResult[0]; + if (locals != null) { + locals.end(result, throwable); + } } } @SuppressWarnings("unused") public static class RequestWithHostAndContextAndHandlerAdvice { + @AssignReturned.ToArguments(@ToArgument(value = 3, index = 1)) @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( + public static Object[] methodEnter( @Advice.Argument(0) HttpHost host, @Advice.Argument(1) ClassicHttpRequest request, - @Advice.Argument(value = 3, readOnly = false) HttpClientResponseHandler handler, - @Advice.Local("otelFullRequest") ClassicHttpRequest fullRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Advice.Argument(3) HttpClientResponseHandler originalHandler) { - Context parentContext = currentContext(); - fullRequest = new RequestWithHost(host, request); - if (!instrumenter().shouldStart(parentContext, fullRequest)) { - return; - } + HttpClientResponseHandler handler = originalHandler; + AdviceLocals locals = AdviceLocals.start(new RequestWithHost(host, request)); - context = instrumenter().start(parentContext, fullRequest); - scope = context.makeCurrent(); + if (locals == null) { + return new Object[] {null, handler}; + } // Wrap the handler so we capture the status code if (handler != null) { handler = new WrappingStatusSettingResponseHandler<>( - context, parentContext, fullRequest, handler); + locals.context, locals.parentContext, locals.request, handler); } + return new Object[] {locals, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Local("otelFullRequest") ClassicHttpRequest fullRequest, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } + @Advice.Enter Object[] enterResult) { - scope.close(); - ApacheHttpClientHelper.doMethodExit(context, fullRequest, result, throwable); + AdviceLocals locals = (AdviceLocals) enterResult[0]; + if (locals != null) { + locals.end(result, throwable); + } } } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java index 0e478a395f1c..43febca4b4eb 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java @@ -8,11 +8,13 @@ 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.Arrays; import java.util.List; @AutoService(InstrumentationModule.class) -public class ApacheHttpClientInstrumentationModule extends InstrumentationModule { +public class ApacheHttpClientInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public ApacheHttpClientInstrumentationModule() { super("apache-httpclient", "apache-httpclient-5.0"); @@ -23,4 +25,9 @@ public List typeInstrumentations() { return Arrays.asList( new ApacheHttpClientInstrumentation(), new ApacheHttpAsyncClientInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } From a12b3221f20f21993c146f7a1facd0de975b2c9b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 22 May 2025 15:11:56 +0200 Subject: [PATCH 2/8] post-review cleanup --- .../v2_0/ApacheHttpClientInstrumentation.java | 11 ++- .../v5_0/ApacheHttpClientInstrumentation.java | 97 +++++++++---------- 2 files changed, 53 insertions(+), 55 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java index 0613fe86be6a..68385f2f9491 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumentation.java @@ -51,10 +51,12 @@ public static class ExecuteMethodAdvice { public static class AdviceScope { private final Context context; private final Scope scope; + private final HttpMethod httpMethod; - private AdviceScope(Context context, Scope scope) { + private AdviceScope(Context context, Scope scope, HttpMethod httpMethod) { this.context = context; this.scope = scope; + this.httpMethod = httpMethod; } @Nullable @@ -64,10 +66,10 @@ public static AdviceScope start(HttpMethod httpMethod) { return null; } Context context = instrumenter().start(parentContext, httpMethod); - return new AdviceScope(context, context.makeCurrent()); + return new AdviceScope(context, context.makeCurrent(), httpMethod); } - public void end(HttpMethod httpMethod, Throwable throwable) { + public void end(Throwable throwable) { scope.close(); instrumenter().end(context, httpMethod, httpMethod, throwable); } @@ -80,12 +82,11 @@ public static AdviceScope methodEnter(@Advice.Argument(1) HttpMethod httpMethod) @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Argument(1) HttpMethod httpMethod, @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter @Nullable AdviceScope adviceScope) { if (adviceScope != null) { - adviceScope.end(httpMethod, throwable); + adviceScope.end(throwable); } } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index ee2c6477cea7..8d2ba315731a 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -124,13 +124,13 @@ public void transform(TypeTransformer transformer) { this.getClass().getName() + "$RequestWithHostAndContextAndHandlerAdvice"); } - public static class AdviceLocals { + public static class AdviceScope { public final ClassicHttpRequest request; public final Context parentContext; public final Context context; public final Scope scope; - private AdviceLocals( + private AdviceScope( ClassicHttpRequest request, Context parentContext, Context context, Scope scope) { this.request = request; this.context = context; @@ -139,20 +139,25 @@ private AdviceLocals( } @Nullable - public static AdviceLocals start(ClassicHttpRequest request) { + public static AdviceScope start(ClassicHttpRequest request) { Context parentContext = currentContext(); if (!instrumenter().shouldStart(parentContext, request)) { return null; } Context context = instrumenter().start(parentContext, request); - return new AdviceLocals(request, parentContext, context, context.makeCurrent()); + return new AdviceScope(request, parentContext, context, context.makeCurrent()); } public void end(Object result, Throwable throwable) { scope.close(); ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); } + + public WrappingStatusSettingResponseHandler wrapResponseHandler( + HttpClientResponseHandler handler) { + return new WrappingStatusSettingResponseHandler<>(context, parentContext, request, handler); + } } @SuppressWarnings("unused") @@ -160,18 +165,18 @@ public static class RequestAdvice { @Nullable @Advice.OnMethodEnter(suppress = Throwable.class) - public static AdviceLocals methodEnter(@Advice.Argument(0) ClassicHttpRequest request) { - return AdviceLocals.start(request); + public static AdviceScope methodEnter(@Advice.Argument(0) ClassicHttpRequest request) { + return AdviceScope.start(request); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Enter @Nullable AdviceLocals locals) { + @Advice.Enter @Nullable AdviceScope scope) { - if (locals != null) { - locals.end(result, throwable); + if (scope != null) { + scope.end(result, throwable); } } } @@ -186,18 +191,16 @@ public static Object[] methodEnter( @Advice.Argument(1) HttpClientResponseHandler originalHandler) { HttpClientResponseHandler handler = originalHandler; - AdviceLocals locals = AdviceLocals.start(request); - if (locals == null) { + AdviceScope scope = AdviceScope.start(request); + if (scope == null) { return new Object[] {null, handler}; } // Wrap the handler so we capture the status code if (handler != null) { - handler = - new WrappingStatusSettingResponseHandler<>( - locals.context, locals.parentContext, request, handler); + handler = scope.wrapResponseHandler(handler); } - return new Object[] {locals, handler}; + return new Object[] {scope, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -207,9 +210,9 @@ public static void methodExit( @Advice.Thrown Throwable throwable, @Advice.Enter Object[] enterResult) { - AdviceLocals locals = (AdviceLocals) enterResult[0]; - if (locals != null) { - locals.end(result, throwable); + AdviceScope scope = (AdviceScope) enterResult[0]; + if (scope != null) { + scope.end(result, throwable); } } } @@ -224,18 +227,16 @@ public static Object[] methodEnter( @Advice.Argument(2) HttpClientResponseHandler originalHandler) { HttpClientResponseHandler handler = originalHandler; - AdviceLocals locals = AdviceLocals.start(request); - if (locals == null) { + AdviceScope scope = AdviceScope.start(request); + if (scope == null) { return new Object[] {null, handler}; } // Wrap the handler so we capture the status code if (handler != null) { - handler = - new WrappingStatusSettingResponseHandler<>( - locals.context, locals.parentContext, request, handler); + handler = scope.wrapResponseHandler(handler); } - return new Object[] {locals, handler}; + return new Object[] {scope, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -245,9 +246,9 @@ public static void methodExit( @Advice.Thrown Throwable throwable, @Advice.Enter Object[] enterResult) { - AdviceLocals locals = (AdviceLocals) enterResult[0]; - if (locals != null) { - locals.end(result, throwable); + AdviceScope scope = (AdviceScope) enterResult[0]; + if (scope != null) { + scope.end(result, throwable); } } } @@ -257,20 +258,20 @@ public static class RequestWithHostAdvice { @Nullable @Advice.OnMethodEnter(suppress = Throwable.class) - public static AdviceLocals methodEnter( + public static AdviceScope methodEnter( @Advice.Argument(0) HttpHost host, @Advice.Argument(1) ClassicHttpRequest request) { - return AdviceLocals.start(new RequestWithHost(host, request)); + return AdviceScope.start(new RequestWithHost(host, request)); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, @Advice.Thrown Throwable throwable, - @Advice.Enter @Nullable AdviceLocals locals) { + @Advice.Enter @Nullable AdviceScope scope) { - if (locals != null) { - locals.end(result, throwable); + if (scope != null) { + scope.end(result, throwable); } } } @@ -286,19 +287,17 @@ public static Object[] methodEnter( @Advice.Argument(2) HttpClientResponseHandler originalHandler) { HttpClientResponseHandler handler = originalHandler; - AdviceLocals locals = AdviceLocals.start(new RequestWithHost(host, request)); + AdviceScope scope = AdviceScope.start(new RequestWithHost(host, request)); - if (locals == null) { + if (scope == null) { return new Object[] {null, handler}; } // Wrap the handler so we capture the status code if (handler != null) { - handler = - new WrappingStatusSettingResponseHandler<>( - locals.context, locals.parentContext, locals.request, handler); + scope.wrapResponseHandler(handler); } - return new Object[] {locals, handler}; + return new Object[] {scope, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -307,9 +306,9 @@ public static void methodExit( @Advice.Thrown Throwable throwable, @Advice.Enter Object[] enterResult) { - AdviceLocals locals = (AdviceLocals) enterResult[0]; - if (locals != null) { - locals.end(result, throwable); + AdviceScope scope = (AdviceScope) enterResult[0]; + if (scope != null) { + scope.end(result, throwable); } } } @@ -325,19 +324,17 @@ public static Object[] methodEnter( @Advice.Argument(3) HttpClientResponseHandler originalHandler) { HttpClientResponseHandler handler = originalHandler; - AdviceLocals locals = AdviceLocals.start(new RequestWithHost(host, request)); + AdviceScope scope = AdviceScope.start(new RequestWithHost(host, request)); - if (locals == null) { + if (scope == null) { return new Object[] {null, handler}; } // Wrap the handler so we capture the status code if (handler != null) { - handler = - new WrappingStatusSettingResponseHandler<>( - locals.context, locals.parentContext, locals.request, handler); + handler = scope.wrapResponseHandler(handler); } - return new Object[] {locals, handler}; + return new Object[] {scope, handler}; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -346,9 +343,9 @@ public static void methodExit( @Advice.Thrown Throwable throwable, @Advice.Enter Object[] enterResult) { - AdviceLocals locals = (AdviceLocals) enterResult[0]; - if (locals != null) { - locals.end(result, throwable); + AdviceScope scope = (AdviceScope) enterResult[0]; + if (scope != null) { + scope.end(result, throwable); } } } From 0627a7db840bc463fdf6e0cf1a926af5399d9d35 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 23 May 2025 10:41:25 +0200 Subject: [PATCH 3/8] add missing @Nullable annotations --- .../v5_0/ApacheHttpClientHelper.java | 3 ++- .../v5_0/ApacheHttpClientInstrumentation.java | 18 +++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java index 0a75742dd513..b230edf8cf20 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java @@ -10,11 +10,12 @@ import io.opentelemetry.context.Context; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.HttpResponse; +import javax.annotation.Nullable; public class ApacheHttpClientHelper { public static void doMethodExit( - Context context, ClassicHttpRequest request, Object result, Throwable throwable) { + Context context, ClassicHttpRequest request, @Nullable Object result, @Nullable Throwable throwable) { if (throwable != null) { instrumenter().end(context, request, null, throwable); } else if (result instanceof HttpResponse) { diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index 8d2ba315731a..81b51caf18b5 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -149,7 +149,7 @@ public static AdviceScope start(ClassicHttpRequest request) { return new AdviceScope(request, parentContext, context, context.makeCurrent()); } - public void end(Object result, Throwable throwable) { + public void end(@Nullable Object result, @Nullable Throwable throwable) { scope.close(); ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); } @@ -172,7 +172,7 @@ public static AdviceScope methodEnter(@Advice.Argument(0) ClassicHttpRequest req @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, - @Advice.Thrown Throwable throwable, + @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter @Nullable AdviceScope scope) { if (scope != null) { @@ -206,8 +206,8 @@ public static Object[] methodEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Argument(0) ClassicHttpRequest request, - @Advice.Return Object result, - @Advice.Thrown Throwable throwable, + @Advice.Return @Nullable Object result, + @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter Object[] enterResult) { AdviceScope scope = (AdviceScope) enterResult[0]; @@ -242,8 +242,8 @@ public static Object[] methodEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Argument(0) ClassicHttpRequest request, - @Advice.Return Object result, - @Advice.Thrown Throwable throwable, + @Advice.Return @Nullable Object result, + @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter Object[] enterResult) { AdviceScope scope = (AdviceScope) enterResult[0]; @@ -267,7 +267,7 @@ public static AdviceScope methodEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.Return Object result, - @Advice.Thrown Throwable throwable, + @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter @Nullable AdviceScope scope) { if (scope != null) { @@ -302,8 +302,8 @@ public static Object[] methodEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( - @Advice.Return Object result, - @Advice.Thrown Throwable throwable, + @Advice.Return @Nullable Object result, + @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter Object[] enterResult) { AdviceScope scope = (AdviceScope) enterResult[0]; From b678d5d6ed56a250bb2b5736efc8f1bb7b5db600 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Fri, 23 May 2025 12:38:36 +0000 Subject: [PATCH 4/8] ./gradlew spotlessApply --- .../apachehttpclient/v5_0/ApacheHttpClientHelper.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java index b230edf8cf20..376ae96484ef 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java @@ -8,14 +8,17 @@ import static io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0.ApacheHttpClientSingletons.instrumenter; import io.opentelemetry.context.Context; +import javax.annotation.Nullable; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.HttpResponse; -import javax.annotation.Nullable; public class ApacheHttpClientHelper { public static void doMethodExit( - Context context, ClassicHttpRequest request, @Nullable Object result, @Nullable Throwable throwable) { + Context context, + ClassicHttpRequest request, + @Nullable Object result, + @Nullable Throwable throwable) { if (throwable != null) { instrumenter().end(context, request, null, throwable); } else if (result instanceof HttpResponse) { From 0e14a5c015cba004ce59fd2f22ecd8496d95651a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 23 May 2025 15:13:33 +0200 Subject: [PATCH 5/8] fix hard to spot typo --- .../apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index 81b51caf18b5..3edc420b26e0 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -295,7 +295,7 @@ public static Object[] methodEnter( // Wrap the handler so we capture the status code if (handler != null) { - scope.wrapResponseHandler(handler); + handler = scope.wrapResponseHandler(handler); } return new Object[] {scope, handler}; } From 75016df24bb12dbd49bb1c7882bc5085abfc37be Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 23 May 2025 15:21:36 +0200 Subject: [PATCH 6/8] make fields final in AdviceScope --- .../v5_0/ApacheHttpClientInstrumentation.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index 3edc420b26e0..6824276c434c 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -125,10 +125,10 @@ public void transform(TypeTransformer transformer) { } public static class AdviceScope { - public final ClassicHttpRequest request; - public final Context parentContext; - public final Context context; - public final Scope scope; + private final ClassicHttpRequest request; + private final Context parentContext; + private final Context context; + private final Scope scope; private AdviceScope( ClassicHttpRequest request, Context parentContext, Context context, Scope scope) { From 7b3d023c213ecbef4e53fff327eb987371d30e37 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 27 May 2025 13:47:08 +0200 Subject: [PATCH 7/8] post-review cleanup --- .../v5_0/ApacheHttpClientHelper.java | 32 ------------------- .../v5_0/ApacheHttpClientInstrumentation.java | 9 +++++- 2 files changed, 8 insertions(+), 33 deletions(-) delete mode 100644 instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java deleted file mode 100644 index 376ae96484ef..000000000000 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHelper.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; - -import static io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0.ApacheHttpClientSingletons.instrumenter; - -import io.opentelemetry.context.Context; -import javax.annotation.Nullable; -import org.apache.hc.core5.http.ClassicHttpRequest; -import org.apache.hc.core5.http.HttpResponse; - -public class ApacheHttpClientHelper { - - public static void doMethodExit( - Context context, - ClassicHttpRequest request, - @Nullable Object result, - @Nullable Throwable throwable) { - if (throwable != null) { - instrumenter().end(context, request, null, throwable); - } else if (result instanceof HttpResponse) { - instrumenter().end(context, request, (HttpResponse) result, null); - } else { - // ended in WrappingStatusSettingResponseHandler - } - } - - private ApacheHttpClientHelper() {} -} diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index 6824276c434c..48e2bb7dcf61 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -28,6 +28,7 @@ import net.bytebuddy.matcher.ElementMatcher; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.io.HttpClientResponseHandler; class ApacheHttpClientInstrumentation implements TypeInstrumentation { @@ -151,7 +152,13 @@ public static AdviceScope start(ClassicHttpRequest request) { public void end(@Nullable Object result, @Nullable Throwable throwable) { scope.close(); - ApacheHttpClientHelper.doMethodExit(context, request, result, throwable); + if (throwable != null) { + instrumenter().end(context, request, null, throwable); + } else if (result instanceof HttpResponse) { + instrumenter().end(context, request, (HttpResponse) result, null); + } else { + // ended in WrappingStatusSettingResponseHandler + } } public WrappingStatusSettingResponseHandler wrapResponseHandler( From 458c6ac8216af99cd2a6aa37a8a174601160175f Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 27 May 2025 15:08:44 +0200 Subject: [PATCH 8/8] remove another copy of ApacheHttpClientHelper --- .../v4_0/ApacheHttpClientHelper.java | 27 ------------------- .../v4_0/ApacheHttpClientInstrumentation.java | 8 +++++- .../v5_0/ApacheHttpClientInstrumentation.java | 3 +-- 3 files changed, 8 insertions(+), 30 deletions(-) delete mode 100644 instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientHelper.java diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientHelper.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientHelper.java deleted file mode 100644 index bdfc241a81fd..000000000000 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientHelper.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v4_0; - -import static io.opentelemetry.javaagent.instrumentation.apachehttpclient.v4_0.ApacheHttpClientSingletons.instrumenter; - -import io.opentelemetry.context.Context; -import org.apache.http.HttpResponse; - -public final class ApacheHttpClientHelper { - - public static void doMethodExit( - Context context, ApacheHttpClientRequest request, Object result, Throwable throwable) { - if (throwable != null) { - instrumenter().end(context, request, null, throwable); - } else if (result instanceof HttpResponse) { - instrumenter().end(context, request, (HttpResponse) result, null); - } else { - // ended in WrappingStatusSettingResponseHandler - } - } - - private ApacheHttpClientHelper() {} -} diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java index a7f2678b6330..78e94e664678 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java @@ -28,6 +28,7 @@ import net.bytebuddy.matcher.ElementMatcher; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; import org.apache.http.client.ResponseHandler; import org.apache.http.client.methods.HttpUriRequest; @@ -157,7 +158,12 @@ public ResponseHandler wrapHandler(ResponseHandler handler) { public void end(@Nullable Object result, @Nullable Throwable throwable) { scope.close(); - ApacheHttpClientHelper.doMethodExit(context, otelRequest, result, throwable); + if (throwable != null) { + instrumenter().end(context, otelRequest, null, throwable); + } else if (result instanceof HttpResponse) { + instrumenter().end(context, otelRequest, (HttpResponse) result, null); + } + // ended in WrappingStatusSettingResponseHandler } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index 48e2bb7dcf61..79ba36350b8c 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -156,9 +156,8 @@ public void end(@Nullable Object result, @Nullable Throwable throwable) { instrumenter().end(context, request, null, throwable); } else if (result instanceof HttpResponse) { instrumenter().end(context, request, (HttpResponse) result, null); - } else { - // ended in WrappingStatusSettingResponseHandler } + // ended in WrappingStatusSettingResponseHandler } public WrappingStatusSettingResponseHandler wrapResponseHandler(