Changes in apache and jax-rs2 instrumentation to not have interceptor…

…s depend on thread local, and instead on the implementation specific http context propagation mechanisms

Signed-off-by: Debosmit Ray <debo@uber.com>

jaeger-apachehttpclient/src/main/java/com/uber/jaeger/httpclient/SpanCreationRequestInterceptor.java

@@ -26,7 +26,6 @@
 import com.uber.jaeger.context.TracingUtils;
 import io.opentracing.Span;
 import io.opentracing.Tracer;
-import io.opentracing.propagation.Format;
 import io.opentracing.tag.Tags;
 import java.io.IOException;
 import lombok.extern.slf4j.Slf4j;
@@ -66,9 +65,6 @@ public void process(HttpRequest httpRequest, HttpContext httpContext)
       }
 
       Span clientSpan = clientSpanBuilder.startManual();
-      // putting newly created span on the trace context so that
-      // other interceptors in the chain
-      parentContext.push(clientSpan);
 
       RequestLine requestLine = httpRequest.getRequestLine();
 

jaeger-apachehttpclient/src/main/java/com/uber/jaeger/httpclient/SpanInjectionRequestInterceptor.java

@@ -22,20 +22,14 @@
 
 package com.uber.jaeger.httpclient;
 
-import com.uber.jaeger.context.TraceContext;
-import com.uber.jaeger.context.TracingUtils;
 import io.opentracing.Span;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
 import java.io.IOException;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.http.HttpException;
-import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
 import org.apache.http.HttpRequestInterceptor;
-import org.apache.http.RequestLine;
-import org.apache.http.client.protocol.HttpClientContext;
 import org.apache.http.protocol.HttpContext;
 
 /**
@@ -58,9 +52,7 @@ public SpanInjectionRequestInterceptor(Tracer tracer) {
   public void process(HttpRequest httpRequest, HttpContext httpContext)
       throws HttpException, IOException {
     try {
-      TraceContext parentContext = TracingUtils.getTraceContext();
-
-      Span clientSpan = parentContext.pop();
+      Span clientSpan = (Span) httpContext.getAttribute(Constants.CURRENT_SPAN_CONTEXT_KEY);
 
       tracer.inject(
           clientSpan.context(), Format.Builtin.HTTP_HEADERS, new ClientRequestCarrier(httpRequest));

jaeger-apachehttpclient/src/main/java/com/uber/jaeger/httpclient/TracingRequestInterceptor.java

@@ -14,20 +14,14 @@
 
 package com.uber.jaeger.httpclient;
 
-import com.uber.jaeger.context.TraceContext;
 import com.uber.jaeger.context.TracingUtils;
 import io.opentracing.Span;
 import io.opentracing.Tracer;
-import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
 import java.io.IOException;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.http.HttpException;
-import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
 import org.apache.http.HttpRequestInterceptor;
-import org.apache.http.RequestLine;
-import org.apache.http.client.protocol.HttpClientContext;
 import org.apache.http.protocol.HttpContext;
 
 /**
@@ -40,12 +34,10 @@
  */
 @Slf4j
 public class TracingRequestInterceptor implements HttpRequestInterceptor {
-  private final Tracer tracer;
   private final SpanCreationRequestInterceptor spanCreationInterceptor;
   private final SpanInjectionRequestInterceptor spanInjectionInterceptor;
 
   public TracingRequestInterceptor(Tracer tracer) {
-    this.tracer = tracer;
     this.spanCreationInterceptor = new SpanCreationRequestInterceptor(tracer);
     this.spanInjectionInterceptor = new SpanInjectionRequestInterceptor(tracer);
   }
@@ -54,9 +46,9 @@ public TracingRequestInterceptor(Tracer tracer) {
   public void process(HttpRequest httpRequest, HttpContext httpContext)
       throws HttpException, IOException {
     try {
-      this.spanCreationInterceptor.process(httpRequest, httpContext);
+      spanCreationInterceptor.process(httpRequest, httpContext);
       onSpanStarted(TracingUtils.getTraceContext().getCurrentSpan(), httpRequest, httpContext);
-      this.spanInjectionInterceptor.process(httpRequest, httpContext);
+      spanInjectionInterceptor.process(httpRequest, httpContext);
     } catch (Exception e) {
       log.error("Could not start client tracing span.", e);
     }

jaeger-apachehttpclient/src/test/java/com/uber/jaeger/httpclient/JaegerRequestAndResponseInterceptorIntegrationTest.java

@@ -88,6 +88,9 @@ public void testAsyncHttpClientTracing() throws Exception {
 
     client.start();
 
+    // Verify that parent span is on top of the stack _before_ request is made
+    assertEquals(parentSpan, TracingUtils.getTraceContext().getCurrentSpan());
+
     //Make a request to the async client and wait for response
     client
         .execute(
@@ -105,6 +108,9 @@ public void cancelled() {}
             })
         .get();
 
+    // Verify that parent span is on top of the stack _after_ request is made
+    assertEquals(parentSpan, TracingUtils.getTraceContext().getCurrentSpan());
+
     verifyTracing(parentSpan);
   }
 
@@ -124,7 +130,7 @@ public void testHttpClientTracing() throws Exception {
   private void verifyTracing(Span parentSpan) {
     //Assert that traces are correctly emitted by the client
     List<Span> spans = reporter.getSpans();
-    assertEquals(3, spans.size());
+    assertEquals(2, spans.size());
     Span span = spans.get(1);
     assertEquals("GET", span.getOperationName());
     assertEquals(parentSpan.context().getSpanId(), span.context().getParentId());

jaeger-jaxrs2/src/main/java/com/uber/jaeger/filters/jaxrs2/ClientFilter.java

@@ -41,7 +41,7 @@ public ClientFilter(Tracer tracer, TraceContext traceContext) {
     this.traceContext = traceContext;
 
     this.spanCreationFilter = new ClientSpanCreationFilter(tracer, traceContext);
-    this.spanInjectionFilter = new ClientSpanInjectionFilter(tracer, traceContext);
+    this.spanInjectionFilter = new ClientSpanInjectionFilter(tracer);
   }
 
   @Override

jaeger-jaxrs2/src/main/java/com/uber/jaeger/filters/jaxrs2/ClientSpanCreationFilter.java

@@ -51,7 +51,5 @@ public void filter(ClientRequestContext clientRequestContext) throws IOException
     Tags.SPAN_KIND.set(clientSpan, Tags.SPAN_KIND_CLIENT);
     Tags.HTTP_URL.set(clientSpan, clientRequestContext.getUri().toString());
     Tags.PEER_HOSTNAME.set(clientSpan, clientRequestContext.getUri().getHost());
-
-    traceContext.push(clientSpan);
   }
 }

jaeger-jaxrs2/src/main/java/com/uber/jaeger/filters/jaxrs2/ClientSpanInjectionFilter.java

@@ -34,16 +34,20 @@
 @Slf4j
 public class ClientSpanInjectionFilter implements ClientRequestFilter, ClientResponseFilter {
   private final Tracer tracer;
-  private final TraceContext traceContext;
 
+  /** @deprecated use {@link ClientSpanInjectionFilter(Tracer)} */
+  @Deprecated
   public ClientSpanInjectionFilter(Tracer tracer, TraceContext traceContext) {
+    this(tracer);
+  }
+
+  public ClientSpanInjectionFilter(Tracer tracer) {
     this.tracer = tracer;
-    this.traceContext = traceContext;
   }
 
   @Override
   public void filter(ClientRequestContext clientRequestContext) throws IOException {
-    Span clientSpan = traceContext.getCurrentSpan();
+    Span clientSpan = (Span) clientRequestContext.getProperty(Constants.CURRENT_SPAN_CONTEXT_KEY);
 
     tracer.inject(
         clientSpan.context(),

jaeger-jaxrs2/src/test/java/com/uber/jaeger/filters/jaxrs2/ClientFilterTest.java

@@ -33,7 +33,6 @@
 import java.util.Map;
 import javax.ws.rs.client.ClientRequestContext;
 import javax.ws.rs.client.ClientResponseContext;
-import javax.ws.rs.core.MultivaluedHashMap;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -73,7 +72,6 @@ public void testFilter() throws Exception {
     String method = "GET";
     when(clientRequestContext.getMethod()).thenReturn(method);
     when(clientRequestContext.getUri()).thenReturn(new URI("http://localhost/path"));
-    when(clientRequestContext.getHeaders()).thenReturn(new MultivaluedHashMap<String, Object>());
 
     when(clientResponseContext.getStatus()).thenReturn(200);