Propagate context to apache httpclient async callback
MODIFY
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentationModule.java
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentationModule.java
@@ -193,7 +193,7 @@ public class ApacheHttpClientInstrumentationModule extends InstrumentationModule
// Wrap the handler so we capture the status code
if (handler != null) {
- handler = new WrappingStatusSettingResponseHandler<>(context, handler);
+ handler = new WrappingStatusSettingResponseHandler<>(context, parentContext, handler);
}
}
@@ -262,7 +262,7 @@ public class ApacheHttpClientInstrumentationModule extends InstrumentationModule
// Wrap the handler so we capture the status code
if (handler != null) {
- handler = new WrappingStatusSettingResponseHandler<>(context, handler);
+ handler = new WrappingStatusSettingResponseHandler<>(context, parentContext, handler);
}
}

MODIFY
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/WrappingStatusSettingResponseHandler.java
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/WrappingStatusSettingResponseHandler.java
@@ -8,22 +8,33 @@ package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v4_0;
import static io.opentelemetry.javaagent.instrumentation.apachehttpclient.v4_0.ApacheHttpClientTracer.tracer;
import io.opentelemetry.context.Context;
+import io.opentelemetry.context.Scope;
import java.io.IOException;
import org.apache.http.HttpResponse;
import org.apache.http.client.ResponseHandler;
public class WrappingStatusSettingResponseHandler<T> implements ResponseHandler<T> {
final Context context;
+ final Context parentContext;
final ResponseHandler<T> handler;
- public WrappingStatusSettingResponseHandler(Context context, ResponseHandler<T> handler) {
+ public WrappingStatusSettingResponseHandler(
+ Context context, Context parentContext, ResponseHandler<T> handler) {
this.context = context;
+ this.parentContext = parentContext;
this.handler = handler;
}
@Override
public T handleResponse(HttpResponse response) throws IOException {
tracer().end(context, response);
+ // ending the span before executing the callback handler (and scoping the callback handler to
+ // the parent context), even though we are inside of a synchronous http client callback
+ // underneath HttpClient.execute(..), in order to not attribute other CLIENT span timings that
+ // may be performed in the callback handler to the http client span (and so we don't end up with
+ // nested CLIENT spans, which we currently suppress)
+ try (Scope ignored = parentContext.makeCurrent()) {
return handler.handleResponse(response);
}
}
+}

MODIFY
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
@@ -37,8 +37,7 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
request.addHeader(new BasicHeader(it.key, it.value))
}
- def response = executeRequest(request, uri)
- callback?.call()
+ def response = executeRequest(request, uri, callback)
response.entity?.content?.close() // Make sure the connection is closed.
return response.statusLine.statusCode
@@ -46,7 +45,7 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
abstract T createRequest(String method, URI uri)
- abstract HttpResponse executeRequest(T request, URI uri)
+ abstract HttpResponse executeRequest(T request, URI uri, Closure callback)
static String fullPathFromURI(URI uri) {
StringBuilder builder = new StringBuilder()
@@ -75,8 +74,10 @@ class ApacheClientHostRequest extends ApacheHttpClientTest<BasicHttpRequest> {
}
@Override
- HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request)
+ HttpResponse executeRequest(BasicHttpRequest request, URI uri, Closure callback) {
+ def response = client.execute(new HttpHost(uri.getHost(), uri.getPort()), request)
+ callback?.call()
+ return response
}
@Override
@@ -93,8 +94,10 @@ class ApacheClientHostRequestContext extends ApacheHttpClientTest<BasicHttpReque
}
@Override
- HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, new BasicHttpContext())
+ HttpResponse executeRequest(BasicHttpRequest request, URI uri, Closure callback) {
+ def response = client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, new BasicHttpContext())
+ callback?.call()
+ return response
}
@Override
@@ -111,8 +114,11 @@ class ApacheClientHostRequestResponseHandler extends ApacheHttpClientTest<BasicH
}
@Override
- HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, { response -> response })
+ HttpResponse executeRequest(BasicHttpRequest request, URI uri, Closure callback) {
+ return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, {
+ callback?.call()
+ return it
+ })
}
@Override
@@ -129,8 +135,11 @@ class ApacheClientHostRequestResponseHandlerContext extends ApacheHttpClientTest
}
@Override
- HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
- return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, { response -> response }, new BasicHttpContext())
+ HttpResponse executeRequest(BasicHttpRequest request, URI uri, Closure callback) {
+ return client.execute(new HttpHost(uri.getHost(), uri.getPort()), request, {
+ callback?.call()
+ return it
+ }, new BasicHttpContext())
}
@Override
@@ -147,8 +156,10 @@ class ApacheClientUriRequest extends ApacheHttpClientTest<HttpUriRequest> {
}
@Override
- HttpResponse executeRequest(HttpUriRequest request, URI uri) {
- return client.execute(request)
+ HttpResponse executeRequest(HttpUriRequest request, URI uri, Closure callback) {
+ def response = client.execute(request)
+ callback?.call()
+ return response
}
}
@@ -160,8 +171,10 @@ class ApacheClientUriRequestContext extends ApacheHttpClientTest<HttpUriRequest>
}
@Override
- HttpResponse executeRequest(HttpUriRequest request, URI uri) {
- return client.execute(request, new BasicHttpContext())
+ HttpResponse executeRequest(HttpUriRequest request, URI uri, Closure callback) {
+ def response = client.execute(request, new BasicHttpContext())
+ callback?.call()
+ return response
}
}
@@ -173,8 +186,11 @@ class ApacheClientUriRequestResponseHandler extends ApacheHttpClientTest<HttpUri
}
@Override
- HttpResponse executeRequest(HttpUriRequest request, URI uri) {
- return client.execute(request, { response -> response })
+ HttpResponse executeRequest(HttpUriRequest request, URI uri, Closure callback) {
+ return client.execute(request, {
+ callback?.call()
+ it
+ })
}
}
@@ -186,7 +202,10 @@ class ApacheClientUriRequestResponseHandlerContext extends ApacheHttpClientTest<
}
@Override
- HttpResponse executeRequest(HttpUriRequest request, URI uri) {
- return client.execute(request, { response -> response })
+ HttpResponse executeRequest(HttpUriRequest request, URI uri, Closure callback) {
+ return client.execute(request, {
+ callback?.call()
+ it
+ }, new BasicHttpContext())
}
}

Propagate context to apache httpclient async callback (#2276)