Skip to content

Commit 1e2cf79

Browse files
SylvainJugeSylvainJuge
authored
make httpurlconnection indy-ready (#15080)
1 parent 07605d7 commit 1e2cf79

File tree

3 files changed

+104
-83
lines changed

3 files changed

+104
-83
lines changed

‎instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java‎

Lines changed: 92 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
package io.opentelemetry.javaagent.instrumentation.httpurlconnection;
77

8-
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
98
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass;
9+
import static io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionSingletons.HTTP_URL_STATE;
1010
import static io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionSingletons.instrumenter;
1111
import static net.bytebuddy.matcher.ElementMatchers.isProtected;
1212
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
@@ -17,11 +17,11 @@
1717

1818
import io.opentelemetry.context.Context;
1919
import io.opentelemetry.context.Scope;
20-
import io.opentelemetry.instrumentation.api.util.VirtualField;
2120
import io.opentelemetry.javaagent.bootstrap.CallDepth;
2221
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2322
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
2423
import java.net.HttpURLConnection;
24+
import javax.annotation.Nullable;
2525
import net.bytebuddy.asm.Advice;
2626
import net.bytebuddy.description.type.TypeDescription;
2727
import net.bytebuddy.matcher.ElementMatcher;
@@ -57,98 +57,110 @@ public void transform(TypeTransformer transformer) {
5757
@SuppressWarnings("unused")
5858
public static class HttpUrlConnectionAdvice {
5959

60-
@Advice.OnMethodEnter(suppress = Throwable.class)
61-
public static void methodEnter(
62-
@Advice.This HttpURLConnection connection,
63-
@Advice.FieldValue("connected") boolean connected,
64-
@Advice.Local("otelHttpUrlState") HttpUrlState httpUrlState,
65-
@Advice.Local("otelScope") Scope scope,
66-
@Advice.Local("otelCallDepth") CallDepth callDepth) {
67-
68-
callDepth = CallDepth.forClass(HttpURLConnection.class);
69-
if (callDepth.getAndIncrement() > 0) {
70-
// only want the rest of the instrumentation rules (which are complex enough) to apply to
71-
// top-level HttpURLConnection calls
72-
return;
60+
public static class AdviceScope {
61+
private final CallDepth callDepth;
62+
private final HttpUrlState httpUrlState;
63+
private final Scope scope;
64+
65+
private AdviceScope(CallDepth callDepth, HttpUrlState httpUrlState, Scope scope) {
66+
this.callDepth = callDepth;
67+
this.httpUrlState = httpUrlState;
68+
this.scope = scope;
7369
}
7470

75-
Context parentContext = currentContext();
76-
if (!instrumenter().shouldStart(parentContext, connection)) {
77-
return;
71+
public static AdviceScope start(CallDepth callDepth, HttpURLConnection connection) {
72+
if (callDepth.getAndIncrement() > 0) {
73+
// only want the rest of the instrumentation rules (which are complex enough) to apply to
74+
// top-level HttpURLConnection calls
75+
return new AdviceScope(callDepth, null, null);
76+
}
77+
78+
Context parentContext = Context.current();
79+
if (!instrumenter().shouldStart(parentContext, connection)) {
80+
return new AdviceScope(callDepth, null, null);
81+
}
82+
83+
// using virtual field for a couple of reasons:
84+
// - to start an operation in connect() and end it in getInputStream()
85+
// - to avoid creating a new operation on multiple subsequent calls to getInputStream()
86+
HttpUrlState httpUrlState = HTTP_URL_STATE.get(connection);
87+
88+
if (httpUrlState != null) {
89+
if (!httpUrlState.finished) {
90+
return new AdviceScope(callDepth, httpUrlState, httpUrlState.context.makeCurrent());
91+
}
92+
return new AdviceScope(callDepth, httpUrlState, null);
93+
}
94+
95+
Context context = instrumenter().start(parentContext, connection);
96+
httpUrlState = new HttpUrlState(context);
97+
HTTP_URL_STATE.set(connection, httpUrlState);
98+
return new AdviceScope(callDepth, httpUrlState, context.makeCurrent());
7899
}
79100

80-
// using storage for a couple of reasons:
81-
// - to start an operation in connect() and end it in getInputStream()
82-
// - to avoid creating a new operation on multiple subsequent calls to getInputStream()
83-
VirtualField<HttpURLConnection, HttpUrlState> storage =
84-
VirtualField.find(HttpURLConnection.class, HttpUrlState.class);
85-
httpUrlState = storage.get(connection);
101+
public void end(
102+
HttpURLConnection connection,
103+
int responseCode,
104+
@Nullable Throwable throwable,
105+
String methodName) {
106+
if (callDepth.decrementAndGet() > 0 || scope == null) {
107+
return;
108+
}
86109

87-
if (httpUrlState != null) {
88-
if (!httpUrlState.finished) {
89-
scope = httpUrlState.context.makeCurrent();
110+
// prevent infinite recursion in case end() captures response headers due to
111+
// HttpUrlConnection.getHeaderField() calling HttpUrlConnection.getInputStream() which then
112+
// enters this advice again
113+
callDepth.getAndIncrement();
114+
try {
115+
scope.close();
116+
Class<? extends HttpURLConnection> connectionClass = connection.getClass();
117+
118+
String requestMethod = connection.getRequestMethod();
119+
GetOutputStreamContext.set(
120+
httpUrlState.context, connectionClass, methodName, requestMethod);
121+
122+
if (throwable != null) {
123+
if (responseCode >= 400) {
124+
// HttpURLConnection unnecessarily throws exception on error response.
125+
// None of the other http clients do this, so not recording the exception on the span
126+
// to be consistent with the telemetry for other http clients.
127+
instrumenter().end(httpUrlState.context, connection, responseCode, null);
128+
} else {
129+
instrumenter()
130+
.end(
131+
httpUrlState.context,
132+
connection,
133+
responseCode > 0 ? responseCode : httpUrlState.statusCode,
134+
throwable);
135+
}
136+
httpUrlState.finished = true;
137+
} else if (methodName.equals("getInputStream") && responseCode > 0) {
138+
// responseCode field is sometimes not populated.
139+
// We can't call getResponseCode() due to some unwanted side-effects
140+
// (e.g. breaks getOutputStream).
141+
instrumenter().end(httpUrlState.context, connection, responseCode, null);
142+
httpUrlState.finished = true;
143+
}
144+
} finally {
145+
callDepth.decrementAndGet();
90146
}
91-
return;
92147
}
148+
}
93149

94-
Context context = instrumenter().start(parentContext, connection);
95-
httpUrlState = new HttpUrlState(context);
96-
storage.set(connection, httpUrlState);
97-
scope = context.makeCurrent();
150+
@Advice.OnMethodEnter(suppress = Throwable.class)
151+
public static AdviceScope methodEnter(@Advice.This HttpURLConnection connection) {
152+
CallDepth callDepth = CallDepth.forClass(HttpURLConnection.class);
153+
return AdviceScope.start(callDepth, connection);
98154
}
99155

100156
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
101157
public static void methodExit(
102158
@Advice.This HttpURLConnection connection,
103159
@Advice.FieldValue("responseCode") int responseCode,
104-
@Advice.Thrown Throwable throwable,
160+
@Advice.Thrown @Nullable Throwable throwable,
105161
@Advice.Origin("#m") String methodName,
106-
@Advice.Local("otelHttpUrlState") HttpUrlState httpUrlState,
107-
@Advice.Local("otelScope") Scope scope,
108-
@Advice.Local("otelCallDepth") CallDepth callDepth) {
109-
if (callDepth.decrementAndGet() > 0) {
110-
return;
111-
}
112-
if (scope == null) {
113-
return;
114-
}
115-
// prevent infinite recursion in case end() captures response headers due to
116-
// HttpUrlConnection.getHeaderField() calling HttpUrlConnection.getInputStream() which then
117-
// enters this advice again
118-
callDepth.getAndIncrement();
119-
try {
120-
scope.close();
121-
Class<? extends HttpURLConnection> connectionClass = connection.getClass();
122-
123-
String requestMethod = connection.getRequestMethod();
124-
GetOutputStreamContext.set(
125-
httpUrlState.context, connectionClass, methodName, requestMethod);
126-
127-
if (throwable != null) {
128-
if (responseCode >= 400) {
129-
// HttpURLConnection unnecessarily throws exception on error response.
130-
// None of the other http clients do this, so not recording the exception on the span
131-
// to be consistent with the telemetry for other http clients.
132-
instrumenter().end(httpUrlState.context, connection, responseCode, null);
133-
} else {
134-
instrumenter()
135-
.end(
136-
httpUrlState.context,
137-
connection,
138-
responseCode > 0 ? responseCode : httpUrlState.statusCode,
139-
throwable);
140-
}
141-
httpUrlState.finished = true;
142-
} else if (methodName.equals("getInputStream") && responseCode > 0) {
143-
// responseCode field is sometimes not populated.
144-
// We can't call getResponseCode() due to some unwanted side-effects
145-
// (e.g. breaks getOutputStream).
146-
instrumenter().end(httpUrlState.context, connection, responseCode, null);
147-
httpUrlState.finished = true;
148-
}
149-
} finally {
150-
callDepth.decrementAndGet();
151-
}
162+
@Advice.Enter AdviceScope adviceScope) {
163+
adviceScope.end(connection, responseCode, throwable, methodName);
152164
}
153165
}
154166

@@ -159,9 +171,7 @@ public static class GetResponseCodeAdvice {
159171
public static void methodExit(
160172
@Advice.This HttpURLConnection connection, @Advice.Return int returnValue) {
161173

162-
VirtualField<HttpURLConnection, HttpUrlState> storage =
163-
VirtualField.find(HttpURLConnection.class, HttpUrlState.class);
164-
HttpUrlState httpUrlState = storage.get(connection);
174+
HttpUrlState httpUrlState = HTTP_URL_STATE.get(connection);
165175
if (httpUrlState != null) {
166176
httpUrlState.statusCode = returnValue;
167177
}

‎instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentationModule.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
import com.google.auto.service.AutoService;
1111
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1212
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
13+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1314
import java.util.List;
1415

1516
@AutoService(InstrumentationModule.class)
16-
public class HttpUrlConnectionInstrumentationModule extends InstrumentationModule {
17+
public class HttpUrlConnectionInstrumentationModule extends InstrumentationModule
18+
implements ExperimentalInstrumentationModule {
1719

1820
public HttpUrlConnectionInstrumentationModule() {
1921
super("http-url-connection");
@@ -23,4 +25,9 @@ public HttpUrlConnectionInstrumentationModule() {
2325
public List<TypeInstrumentation> typeInstrumentations() {
2426
return singletonList(new HttpUrlConnectionInstrumentation());
2527
}
28+
29+
@Override
30+
public boolean isIndyReady() {
31+
return true;
32+
}
2633
}

‎instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionSingletons.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@
66
package io.opentelemetry.javaagent.instrumentation.httpurlconnection;
77

88
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
9+
import io.opentelemetry.instrumentation.api.util.VirtualField;
910
import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
1011
import io.opentelemetry.javaagent.bootstrap.internal.JavaagentHttpClientInstrumenters;
1112
import java.net.HttpURLConnection;
1213

1314
public final class HttpUrlConnectionSingletons {
1415

16+
public static final VirtualField<HttpURLConnection, HttpUrlState> HTTP_URL_STATE =
17+
VirtualField.find(HttpURLConnection.class, HttpUrlState.class);
18+
1519
private static final Instrumenter<HttpURLConnection, Integer> INSTRUMENTER;
1620

1721
static {

0 commit comments

Comments
 (0)