Use startScope() on all HttpClientTracers (#971)

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java

@@ -38,7 +38,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class HttpClientTracer<REQUEST, RESPONSE> extends BaseTracer {
+public abstract class HttpClientTracer<REQUEST, CARRIER, RESPONSE> extends BaseTracer {
 
   private static final Logger log = LoggerFactory.getLogger(HttpClientTracer.class);
 
@@ -56,7 +56,7 @@ public abstract class HttpClientTracer<REQUEST, RESPONSE> extends BaseTracer {
 
   protected abstract String responseHeader(RESPONSE response, String name);
 
-  protected abstract HttpTextFormat.Setter<REQUEST> getSetter();
+  protected abstract HttpTextFormat.Setter<CARRIER> getSetter();
 
   protected HttpClientTracer() {
     super();
@@ -74,15 +74,15 @@ public Span startSpan(REQUEST request, long startTimeNanos) {
     return startSpan(request, spanNameForRequest(request), startTimeNanos);
   }
 
-  public Scope startScope(Span span, REQUEST request) {
+  public Scope startScope(Span span, CARRIER carrier) {
     Context context = withSpan(span, Context.current());
 
-    Setter<REQUEST> setter = getSetter();
+    Setter<CARRIER> setter = getSetter();
     if (setter == null) {
       throw new IllegalStateException(
           "getSetter() not defined but calling startScope(), either getSetter must be implemented or the scope should be setup manually");
     }
-    OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, setter);
+    OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, carrier, setter);
     context = context.withValue(CONTEXT_CLIENT_SPAN_KEY, span);
     return withScopedContext(context);
   }

instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy

@@ -170,7 +170,7 @@ class HttpClientTracerTest extends BaseTracerTest {
 
   @Override
   def newTracer() {
-    return new HttpClientTracer<Map, Map>() {
+    return new HttpClientTracer<Map, Map, Map>() {
 
       @Override
       protected String method(Map m) {

instrumentation-core/spring/spring-web-3.1/src/main/java/io/opentelemetry/instrumentation/spring/httpclients/HttpHeadersInjectAdapter.java

@@ -17,14 +17,14 @@
 package io.opentelemetry.instrumentation.spring.httpclients;
 
 import io.opentelemetry.context.propagation.HttpTextFormat;
-import org.springframework.http.HttpRequest;
+import org.springframework.http.HttpHeaders;
 
-class HttpHeadersInjectAdapter implements HttpTextFormat.Setter<HttpRequest> {
+class HttpHeadersInjectAdapter implements HttpTextFormat.Setter<HttpHeaders> {
 
   public static final HttpHeadersInjectAdapter SETTER = new HttpHeadersInjectAdapter();
 
   @Override
-  public void set(HttpRequest carrier, String key, String value) {
-    carrier.getHeaders().set(key, value);
+  public void set(HttpHeaders carrier, String key, String value) {
+    carrier.set(key, value);
   }
 }

instrumentation-core/spring/spring-web-3.1/src/main/java/io/opentelemetry/instrumentation/spring/httpclients/RestTemplateInterceptor.java

@@ -41,7 +41,7 @@ public ClientHttpResponse intercept(
       HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
 
     Span span = TRACER.startSpan(request);
-    try (Scope scope = TRACER.startScope(span, request)) {
+    try (Scope scope = TRACER.startScope(span, request.getHeaders())) {
       ClientHttpResponse response = execution.execute(request, body);
       TRACER.end(span, response);
       return response;

instrumentation-core/spring/spring-web-3.1/src/main/java/io/opentelemetry/instrumentation/spring/httpclients/RestTemplateTracer.java

@@ -16,34 +16,28 @@
 
 package io.opentelemetry.instrumentation.spring.httpclients;
 
-import static io.opentelemetry.OpenTelemetry.getPropagators;
 import static io.opentelemetry.instrumentation.spring.httpclients.HttpHeadersInjectAdapter.SETTER;
 
-import io.grpc.Context;
 import io.opentelemetry.context.propagation.HttpTextFormat.Setter;
 import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer;
 import java.io.IOException;
 import java.net.URI;
-import java.net.URISyntaxException;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpRequest;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.client.ClientHttpResponse;
 
-class RestTemplateTracer extends HttpClientTracer<HttpRequest, ClientHttpResponse> {
+class RestTemplateTracer extends HttpClientTracer<HttpRequest, HttpHeaders, ClientHttpResponse> {
 
   public static final RestTemplateTracer TRACER = new RestTemplateTracer();
 
-  public void inject(Context context, HttpRequest request) {
-    getPropagators().getHttpTextFormat().inject(context, request, getSetter());
-  }
-
   @Override
   protected String method(HttpRequest httpRequest) {
     return httpRequest.getMethod().name();
   }
 
   @Override
-  protected URI url(HttpRequest request) throws URISyntaxException {
+  protected URI url(HttpRequest request) {
     return request.getURI();
   }
 
@@ -67,7 +61,7 @@ protected String responseHeader(ClientHttpResponse response, String name) {
   }
 
   @Override
-  protected Setter<HttpRequest> getSetter() {
+  protected Setter<HttpHeaders> getSetter() {
     return SETTER;
   }
 

instrumentation-core/spring/spring-webflux-5.0/src/main/java/io/opentelemetry/instrumentation/spring/webflux/client/HttpHeadersInjectAdapter.java

@@ -17,14 +17,14 @@
 package io.opentelemetry.instrumentation.spring.webflux.client;
 
 import io.opentelemetry.context.propagation.HttpTextFormat;
-import org.springframework.http.HttpHeaders;
+import org.springframework.web.reactive.function.client.ClientRequest;
 
-class HttpHeadersInjectAdapter implements HttpTextFormat.Setter<HttpHeaders> {
+class HttpHeadersInjectAdapter implements HttpTextFormat.Setter<ClientRequest.Builder> {
 
   public static final HttpHeadersInjectAdapter SETTER = new HttpHeadersInjectAdapter();
 
   @Override
-  public void set(HttpHeaders carrier, String key, String value) {
-    carrier.set(key, value);
+  public void set(ClientRequest.Builder carrier, String key, String value) {
+    carrier.header(key, value);
   }
 }

instrumentation-core/spring/spring-webflux-5.0/src/main/java/io/opentelemetry/instrumentation/spring/webflux/client/SpringWebfluxHttpClientTracer.java

@@ -16,21 +16,19 @@
 
 package io.opentelemetry.instrumentation.spring.webflux.client;
 
-import static io.opentelemetry.OpenTelemetry.getPropagators;
 import static io.opentelemetry.instrumentation.spring.webflux.client.HttpHeadersInjectAdapter.SETTER;
 
-import io.grpc.Context;
 import io.opentelemetry.context.propagation.HttpTextFormat.Setter;
 import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer;
 import io.opentelemetry.trace.Span;
 import io.opentelemetry.trace.Tracer;
 import java.net.URI;
 import java.util.List;
-import org.springframework.http.HttpHeaders;
 import org.springframework.web.reactive.function.client.ClientRequest;
 import org.springframework.web.reactive.function.client.ClientResponse;
 
-class SpringWebfluxHttpClientTracer extends HttpClientTracer<ClientRequest, ClientResponse> {
+public class SpringWebfluxHttpClientTracer
+    extends HttpClientTracer<ClientRequest, ClientRequest.Builder, ClientResponse> {
 
   public static final SpringWebfluxHttpClientTracer TRACER = new SpringWebfluxHttpClientTracer();
 
@@ -39,10 +37,6 @@ public void onCancel(Span span) {
     span.setAttribute("message", "The subscription was cancelled");
   }
 
-  public void inject(Context context, HttpHeaders httpHeaders) {
-    getPropagators().getHttpTextFormat().inject(context, httpHeaders, SETTER);
-  }
-
   @Override
   protected String method(ClientRequest httpRequest) {
     return httpRequest.method().name();
@@ -70,8 +64,8 @@ protected String responseHeader(ClientResponse clientResponse, String name) {
   }
 
   @Override
-  protected Setter<ClientRequest> getSetter() {
-    return null;
+  protected Setter<ClientRequest.Builder> getSetter() {
+    return SETTER;
   }
 
   @Override

instrumentation-core/spring/spring-webflux-5.0/src/main/java/io/opentelemetry/instrumentation/spring/webflux/client/WebClientTracingFilter.java

@@ -18,7 +18,6 @@
 
 import static io.opentelemetry.instrumentation.spring.webflux.client.SpringWebfluxHttpClientTracer.TRACER;
 
-import io.grpc.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.trace.Span;
 import io.opentelemetry.trace.Tracer;
@@ -50,12 +49,9 @@ public static void addFilter(
   public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next) {
     Span span = TRACER.startSpan(request);
 
-    try (Scope scope = tracer.withSpan(span)) {
-      ClientRequest mutatedRequest =
-          ClientRequest.from(request)
-              .headers(httpHeaders -> TRACER.inject(Context.current(), httpHeaders))
-              .build();
-      return next.exchange(mutatedRequest)
+    ClientRequest.Builder requestBuilder = ClientRequest.from(request);
+    try (Scope ignored = TRACER.startScope(span, requestBuilder)) {
+      return next.exchange(requestBuilder.build())
           .doOnSuccessOrError(
               (clientResponse, throwable) -> {
                 if (throwable != null) {
@@ -66,6 +62,7 @@ public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next)
               })
           .doOnCancel(
               () -> {
+                TRACER.onCancel(span);
                 TRACER.end(span);
               });
     }

instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/instrumentation/auto/akkahttp/AkkaHttpClientInstrumentation.java

@@ -16,9 +16,7 @@
 
 package io.opentelemetry.instrumentation.auto.akkahttp;
 
-import static io.opentelemetry.context.ContextUtils.withScopedContext;
 import static io.opentelemetry.instrumentation.auto.akkahttp.AkkaHttpClientTracer.TRACER;
-import static io.opentelemetry.trace.TracingContextUtils.withSpan;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
@@ -27,8 +25,6 @@
 import akka.http.scaladsl.model.HttpRequest;
 import akka.http.scaladsl.model.HttpResponse;
 import com.google.auto.service.AutoService;
-import io.grpc.Context;
-import io.opentelemetry.OpenTelemetry;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.context.propagation.HttpTextFormat;
 import io.opentelemetry.instrumentation.auto.api.CallDepthThreadLocalMap.Depth;
@@ -60,6 +56,7 @@ public String[] helperClassNames() {
     return new String[] {
       AkkaHttpClientInstrumentation.class.getName() + "$OnCompleteHandler",
       AkkaHttpClientInstrumentation.class.getName() + "$AkkaHttpHeaders",
+      AkkaHttpClientInstrumentation.class.getName() + "$InjectAdapter",
       packageName + ".AkkaHttpClientTracer",
     };
   }
@@ -95,15 +92,10 @@ public static void methodEnter(
       callDepth = TRACER.getCallDepth();
       if (callDepth.getAndIncrement() == 0) {
         span = TRACER.startSpan(request);
-
-        Context context = withSpan(span, Context.current());
-        if (request != null) {
-          AkkaHttpHeaders headers = new AkkaHttpHeaders(request);
-          OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, headers);
-          // Request is immutable, so we have to assign new value once we update headers
-          request = headers.getRequest();
-        }
-        scope = withScopedContext(context);
+        // Request is immutable, so we have to assign new value once we update headers
+        AkkaHttpHeaders headers = new AkkaHttpHeaders(request);
+        scope = TRACER.startScope(span, headers);
+        request = headers.getRequest();
       }
     }
 
@@ -145,21 +137,33 @@ public Void apply(Try<HttpResponse> result) {
     }
   }
 
-  public static class AkkaHttpHeaders implements HttpTextFormat.Setter<HttpRequest> {
+  public static class AkkaHttpHeaders {
     private HttpRequest request;
 
     public AkkaHttpHeaders(HttpRequest request) {
       this.request = request;
     }
 
-    @Override
-    public void set(HttpRequest carrier, String key, String value) {
-      // It looks like this cast is only needed in Java, Scala would have figured it out
-      request = (HttpRequest) request.addHeader(RawHeader.create(key, value));
-    }
-
     public HttpRequest getRequest() {
       return request;
     }
+
+    public void setRequest(HttpRequest request) {
+      this.request = request;
+    }
+  }
+
+  public static class InjectAdapter implements HttpTextFormat.Setter<AkkaHttpHeaders> {
+
+    public static final InjectAdapter SETTER = new InjectAdapter();
+
+    @Override
+    public void set(AkkaHttpHeaders carrier, String key, String value) {
+      HttpRequest request = carrier.getRequest();
+      if (request != null) {
+        // It looks like this cast is only needed in Java, Scala would have figured it out
+        carrier.setRequest((HttpRequest) request.addHeader(RawHeader.create(key, value)));
+      }
+    }
   }
 }

instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/instrumentation/auto/akkahttp/AkkaHttpClientTracer.java

@@ -16,18 +16,22 @@
 
 package io.opentelemetry.instrumentation.auto.akkahttp;
 
+import static io.opentelemetry.instrumentation.auto.akkahttp.AkkaHttpClientInstrumentation.InjectAdapter.SETTER;
+
 import akka.http.javadsl.model.HttpHeader;
 import akka.http.scaladsl.HttpExt;
 import akka.http.scaladsl.model.HttpRequest;
 import akka.http.scaladsl.model.HttpResponse;
 import io.opentelemetry.context.propagation.HttpTextFormat.Setter;
 import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer;
+import io.opentelemetry.instrumentation.auto.akkahttp.AkkaHttpClientInstrumentation.AkkaHttpHeaders;
 import io.opentelemetry.instrumentation.auto.api.CallDepthThreadLocalMap;
 import io.opentelemetry.instrumentation.auto.api.CallDepthThreadLocalMap.Depth;
 import java.net.URI;
 import java.net.URISyntaxException;
 
-public class AkkaHttpClientTracer extends HttpClientTracer<HttpRequest, HttpResponse> {
+public class AkkaHttpClientTracer
+    extends HttpClientTracer<HttpRequest, AkkaHttpHeaders, HttpResponse> {
   public static final AkkaHttpClientTracer TRACER = new AkkaHttpClientTracer();
 
   public Depth getCallDepth() {
@@ -60,8 +64,8 @@ protected String responseHeader(HttpResponse httpResponse, String name) {
   }
 
   @Override
-  protected Setter<HttpRequest> getSetter() {
-    return null;
+  protected Setter<AkkaHttpHeaders> getSetter() {
+    return SETTER;
   }
 
   @Override

instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java

@@ -18,20 +18,16 @@
 
 import static io.opentelemetry.instrumentation.api.tracer.HttpClientTracer.DEFAULT_SPAN_NAME;
 import static io.opentelemetry.instrumentation.auto.apachehttpasyncclient.ApacheHttpAsyncClientTracer.TRACER;
-import static io.opentelemetry.instrumentation.auto.apachehttpasyncclient.HttpHeadersInjectAdapter.SETTER;
 import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.trace.TracingContextUtils.currentContextWith;
-import static io.opentelemetry.trace.TracingContextUtils.withSpan;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
 
 import com.google.auto.service.AutoService;
-import io.grpc.Context;
-import io.opentelemetry.OpenTelemetry;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.javaagent.tooling.Instrumenter;
 import io.opentelemetry.trace.Span;
@@ -140,8 +136,9 @@ public HttpRequest generateRequest() throws IOException, HttpException {
       span.updateName(TRACER.spanNameForRequest(request));
       TRACER.onRequest(span, request);
 
-      Context context = withSpan(span, Context.current());
-      OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, SETTER);
+      // TODO (trask) expose inject separate from startScope, e.g. for async cases
+      Scope scope = TRACER.startScope(span, request);
+      scope.close();
 
       return request;
     }

instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpasyncclient/ApacheHttpAsyncClientTracer.java

@@ -32,7 +32,8 @@
 import org.apache.http.StatusLine;
 import org.apache.http.client.methods.HttpUriRequest;
 
-public class ApacheHttpAsyncClientTracer extends HttpClientTracer<HttpRequest, HttpResponse> {
+public class ApacheHttpAsyncClientTracer
+    extends HttpClientTracer<HttpRequest, HttpRequest, HttpResponse> {
 
   public static final ApacheHttpAsyncClientTracer TRACER = new ApacheHttpAsyncClientTracer();
 

instrumentation/apache-httpclient/apache-httpclient-2.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpclient/v2_0/CommonsHttpClientTracer.java

@@ -28,7 +28,7 @@
 import org.apache.commons.httpclient.StatusLine;
 import org.apache.commons.httpclient.URIException;
 
-public class CommonsHttpClientTracer extends HttpClientTracer<HttpMethod, HttpMethod> {
+public class CommonsHttpClientTracer extends HttpClientTracer<HttpMethod, HttpMethod, HttpMethod> {
   public static final CommonsHttpClientTracer TRACER = new CommonsHttpClientTracer();
 
   @Override