From 0867cba06bed31b4700f66e3484f81f405777a05 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 21 Oct 2025 11:55:13 +0200 Subject: [PATCH 1/2] make httpurlconnection indy-ready --- .../HttpUrlConnectionInstrumentation.java | 173 ++++++++++-------- ...ttpUrlConnectionInstrumentationModule.java | 9 +- .../HttpUrlConnectionSingletons.java | 4 + 3 files changed, 104 insertions(+), 82 deletions(-) diff --git a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java index 6b1f1d16b7b2..46cf0aea6b53 100644 --- a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java +++ b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; +import static io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionSingletons.HTTP_URL_STATE; import static io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -17,11 +18,11 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.javaagent.bootstrap.CallDepth; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.net.HttpURLConnection; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -57,98 +58,110 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class HttpUrlConnectionAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void methodEnter( - @Advice.This HttpURLConnection connection, - @Advice.FieldValue("connected") boolean connected, - @Advice.Local("otelHttpUrlState") HttpUrlState httpUrlState, - @Advice.Local("otelScope") Scope scope, - @Advice.Local("otelCallDepth") CallDepth callDepth) { - - callDepth = CallDepth.forClass(HttpURLConnection.class); - if (callDepth.getAndIncrement() > 0) { - // only want the rest of the instrumentation rules (which are complex enough) to apply to - // top-level HttpURLConnection calls - return; + public static class AdviceScope { + private final CallDepth callDepth; + private final HttpUrlState httpUrlState; + private final Scope scope; + + private AdviceScope(CallDepth callDepth, HttpUrlState httpUrlState, Scope scope) { + this.callDepth = callDepth; + this.httpUrlState = httpUrlState; + this.scope = scope; } - Context parentContext = currentContext(); - if (!instrumenter().shouldStart(parentContext, connection)) { - return; + public static AdviceScope start(CallDepth callDepth, HttpURLConnection connection) { + if (callDepth.getAndIncrement() > 0) { + // only want the rest of the instrumentation rules (which are complex enough) to apply to + // top-level HttpURLConnection calls + return new AdviceScope(callDepth, null, null); + } + + Context parentContext = currentContext(); + if (!instrumenter().shouldStart(parentContext, connection)) { + return new AdviceScope(callDepth, null, null); + } + + // using virtual field for a couple of reasons: + // - to start an operation in connect() and end it in getInputStream() + // - to avoid creating a new operation on multiple subsequent calls to getInputStream() + HttpUrlState httpUrlState = HTTP_URL_STATE.get(connection); + + if (httpUrlState != null) { + if (!httpUrlState.finished) { + return new AdviceScope(callDepth, httpUrlState, httpUrlState.context.makeCurrent()); + } + return new AdviceScope(callDepth, httpUrlState, null); + } + + Context context = instrumenter().start(parentContext, connection); + httpUrlState = new HttpUrlState(context); + HTTP_URL_STATE.set(connection, httpUrlState); + return new AdviceScope(callDepth, httpUrlState, context.makeCurrent()); } - // using storage for a couple of reasons: - // - to start an operation in connect() and end it in getInputStream() - // - to avoid creating a new operation on multiple subsequent calls to getInputStream() - VirtualField storage = - VirtualField.find(HttpURLConnection.class, HttpUrlState.class); - httpUrlState = storage.get(connection); + public void end( + HttpURLConnection connection, + int responseCode, + @Nullable Throwable throwable, + String methodName) { + if (callDepth.decrementAndGet() > 0 || scope == null) { + return; + } - if (httpUrlState != null) { - if (!httpUrlState.finished) { - scope = httpUrlState.context.makeCurrent(); + // prevent infinite recursion in case end() captures response headers due to + // HttpUrlConnection.getHeaderField() calling HttpUrlConnection.getInputStream() which then + // enters this advice again + callDepth.getAndIncrement(); + try { + scope.close(); + Class connectionClass = connection.getClass(); + + String requestMethod = connection.getRequestMethod(); + GetOutputStreamContext.set( + httpUrlState.context, connectionClass, methodName, requestMethod); + + if (throwable != null) { + if (responseCode >= 400) { + // HttpURLConnection unnecessarily throws exception on error response. + // None of the other http clients do this, so not recording the exception on the span + // to be consistent with the telemetry for other http clients. + instrumenter().end(httpUrlState.context, connection, responseCode, null); + } else { + instrumenter() + .end( + httpUrlState.context, + connection, + responseCode > 0 ? responseCode : httpUrlState.statusCode, + throwable); + } + httpUrlState.finished = true; + } else if (methodName.equals("getInputStream") && responseCode > 0) { + // responseCode field is sometimes not populated. + // We can't call getResponseCode() due to some unwanted side-effects + // (e.g. breaks getOutputStream). + instrumenter().end(httpUrlState.context, connection, responseCode, null); + httpUrlState.finished = true; + } + } finally { + callDepth.decrementAndGet(); } - return; } + } - Context context = instrumenter().start(parentContext, connection); - httpUrlState = new HttpUrlState(context); - storage.set(connection, httpUrlState); - scope = context.makeCurrent(); + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AdviceScope methodEnter(@Advice.This HttpURLConnection connection) { + CallDepth callDepth = CallDepth.forClass(HttpURLConnection.class); + return AdviceScope.start(callDepth, connection); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void methodExit( @Advice.This HttpURLConnection connection, @Advice.FieldValue("responseCode") int responseCode, - @Advice.Thrown Throwable throwable, + @Advice.Thrown @Nullable Throwable throwable, @Advice.Origin("#m") String methodName, - @Advice.Local("otelHttpUrlState") HttpUrlState httpUrlState, - @Advice.Local("otelScope") Scope scope, - @Advice.Local("otelCallDepth") CallDepth callDepth) { - if (callDepth.decrementAndGet() > 0) { - return; - } - if (scope == null) { - return; - } - // prevent infinite recursion in case end() captures response headers due to - // HttpUrlConnection.getHeaderField() calling HttpUrlConnection.getInputStream() which then - // enters this advice again - callDepth.getAndIncrement(); - try { - scope.close(); - Class connectionClass = connection.getClass(); - - String requestMethod = connection.getRequestMethod(); - GetOutputStreamContext.set( - httpUrlState.context, connectionClass, methodName, requestMethod); - - if (throwable != null) { - if (responseCode >= 400) { - // HttpURLConnection unnecessarily throws exception on error response. - // None of the other http clients do this, so not recording the exception on the span - // to be consistent with the telemetry for other http clients. - instrumenter().end(httpUrlState.context, connection, responseCode, null); - } else { - instrumenter() - .end( - httpUrlState.context, - connection, - responseCode > 0 ? responseCode : httpUrlState.statusCode, - throwable); - } - httpUrlState.finished = true; - } else if (methodName.equals("getInputStream") && responseCode > 0) { - // responseCode field is sometimes not populated. - // We can't call getResponseCode() due to some unwanted side-effects - // (e.g. breaks getOutputStream). - instrumenter().end(httpUrlState.context, connection, responseCode, null); - httpUrlState.finished = true; - } - } finally { - callDepth.decrementAndGet(); - } + @Advice.Enter AdviceScope adviceScope) { + adviceScope.end(connection, responseCode, throwable, methodName); } } @@ -159,9 +172,7 @@ public static class GetResponseCodeAdvice { public static void methodExit( @Advice.This HttpURLConnection connection, @Advice.Return int returnValue) { - VirtualField storage = - VirtualField.find(HttpURLConnection.class, HttpUrlState.class); - HttpUrlState httpUrlState = storage.get(connection); + HttpUrlState httpUrlState = HTTP_URL_STATE.get(connection); if (httpUrlState != null) { httpUrlState.statusCode = returnValue; } diff --git a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentationModule.java b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentationModule.java index 33d574ae6d90..681b80c4d5a9 100644 --- a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentationModule.java +++ b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentationModule.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 HttpUrlConnectionInstrumentationModule extends InstrumentationModule { +public class HttpUrlConnectionInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public HttpUrlConnectionInstrumentationModule() { super("http-url-connection"); @@ -23,4 +25,9 @@ public HttpUrlConnectionInstrumentationModule() { public List typeInstrumentations() { return singletonList(new HttpUrlConnectionInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionSingletons.java b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionSingletons.java index 9f8b3006dc35..3c502e3f838f 100644 --- a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionSingletons.java +++ b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionSingletons.java @@ -6,12 +6,16 @@ package io.opentelemetry.javaagent.instrumentation.httpurlconnection; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig; import io.opentelemetry.javaagent.bootstrap.internal.JavaagentHttpClientInstrumenters; import java.net.HttpURLConnection; public final class HttpUrlConnectionSingletons { + public static final VirtualField HTTP_URL_STATE = + VirtualField.find(HttpURLConnection.class, HttpUrlState.class); + private static final Instrumenter INSTRUMENTER; static { From ff8957396587e3f9217037213543c0450a379d37 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 21 Oct 2025 12:57:52 +0200 Subject: [PATCH 2/2] post-review changes --- .../httpurlconnection/HttpUrlConnectionInstrumentation.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java index 46cf0aea6b53..4883aa43e2c8 100644 --- a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java +++ b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java @@ -5,7 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.httpurlconnection; -import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; import static io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionSingletons.HTTP_URL_STATE; import static io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionSingletons.instrumenter; @@ -76,7 +75,7 @@ public static AdviceScope start(CallDepth callDepth, HttpURLConnection connectio return new AdviceScope(callDepth, null, null); } - Context parentContext = currentContext(); + Context parentContext = Context.current(); if (!instrumenter().shouldStart(parentContext, connection)) { return new AdviceScope(callDepth, null, null); }