Break apart functionality of TracingRequestInterceptor (#248)

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

@@ -0,0 +1,114 @@
+/*
+ * Copyright (c) 2017, Uber Technologies, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+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;
+
+/**
+ * Apache http client request interceptor This is designed to be used along with
+ * {@link TracingResponseInterceptor} to report tracing information.
+ *
+ * In most cases you shouldn't be using this directly. Use the appropriate client builder from
+ * {@link TracingInterceptors}
+ *
+ */
+@Slf4j
+public class SpanCreationRequestInterceptor implements HttpRequestInterceptor {
+  private final Tracer tracer;
+
+  public SpanCreationRequestInterceptor(Tracer tracer) {
+    this.tracer = tracer;
+  }
+
+  @Override
+  public void process(HttpRequest httpRequest, HttpContext httpContext)
+      throws HttpException, IOException {
+    try {
+      TraceContext parentContext = TracingUtils.getTraceContext();
+
+      Tracer.SpanBuilder clientSpanBuilder = tracer.buildSpan(getOperationName(httpRequest));
+      if (!parentContext.isEmpty()) {
+        clientSpanBuilder.asChildOf(parentContext.getCurrentSpan());
+      }
+
+      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();
+
+      Tags.SPAN_KIND.set(clientSpan, Tags.SPAN_KIND_CLIENT);
+      Tags.HTTP_URL.set(clientSpan, requestLine.getUri());
+
+      if (httpContext instanceof HttpClientContext) {
+        HttpHost host = ((HttpClientContext) httpContext).getTargetHost();
+        Tags.PEER_HOSTNAME.set(clientSpan, host.getHostName());
+        Tags.PEER_PORT.set(clientSpan, host.getPort());
+      }
+
+      onSpanStarted(clientSpan, httpRequest, httpContext);
+
+      httpContext.setAttribute(Constants.CURRENT_SPAN_CONTEXT_KEY, clientSpan);
+    } catch (Exception e) {
+      log.error("Could not start client tracing span.", e);
+    }
+  }
+
+  /**
+   * onSpanStarted will be called right after the span is created.
+   * The subclasses can override this method to add additional information
+   * to the span, including tags and logs.
+   *
+   * @param clientSpan - the span that's being created
+   * @param httpRequest - the http request for the operation
+   * @param httpContext - the context on which the operation is being executed
+   */
+  protected void onSpanStarted(Span clientSpan, HttpRequest httpRequest, HttpContext httpContext) {
+    // no-op, can be overriden by subclasses
+  }
+
+  /**
+   * Get the Span's operation name. Defaults to the HTTP method.
+   *
+   * @param httpRequest the request for the http operation being executed
+   * @return the operation name
+   */
+  protected String getOperationName(HttpRequest httpRequest) {
+    return httpRequest.getRequestLine().getMethod();
+  }
+}

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

@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2017, Uber Technologies, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+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;
+
+/**
+ * Apache http client request interceptor This is designed to be used along with
+ * {@link TracingResponseInterceptor} to report tracing information.
+ *
+ * In most cases you shouldn't be using this directly. Use the appropriate client builder from
+ * {@link TracingInterceptors}
+ *
+ */
+@Slf4j
+public class SpanInjectionRequestInterceptor implements HttpRequestInterceptor {
+  private final Tracer tracer;
+
+  public SpanInjectionRequestInterceptor(Tracer tracer) {
+    this.tracer = tracer;
+  }
+
+  @Override
+  public void process(HttpRequest httpRequest, HttpContext httpContext)
+      throws HttpException, IOException {
+    try {
+      TraceContext parentContext = TracingUtils.getTraceContext();
+
+      Span clientSpan = parentContext.pop();
+
+      tracer.inject(
+          clientSpan.context(), Format.Builtin.HTTP_HEADERS, new ClientRequestCarrier(httpRequest));
+
+    } catch (Exception e) {
+      log.error("Could not start client tracing span.", e);
+    }
+  }
+
+  /**
+   * Get the Span's operation name. Defaults to the HTTP method.
+   *
+   * @param httpRequest the request for the http operation being executed
+   * @return the operation name
+   */
+  protected String getOperationName(HttpRequest httpRequest) {
+    return httpRequest.getRequestLine().getMethod();
+  }
+}

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

@@ -41,39 +41,22 @@
 @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);
   }
 
   @Override
   public void process(HttpRequest httpRequest, HttpContext httpContext)
       throws HttpException, IOException {
     try {
-      TraceContext parentContext = TracingUtils.getTraceContext();
-
-      RequestLine requestLine = httpRequest.getRequestLine();
-      Tracer.SpanBuilder clientSpanBuilder = tracer.buildSpan(getOperationName(httpRequest));
-      if (!parentContext.isEmpty()) {
-        clientSpanBuilder.asChildOf(parentContext.getCurrentSpan());
-      }
-
-      Span clientSpan = clientSpanBuilder.startManual();
-      Tags.SPAN_KIND.set(clientSpan, Tags.SPAN_KIND_CLIENT);
-      Tags.HTTP_URL.set(clientSpan, requestLine.getUri());
-
-      if (httpContext instanceof HttpClientContext) {
-        HttpHost host = ((HttpClientContext) httpContext).getTargetHost();
-        Tags.PEER_HOSTNAME.set(clientSpan, host.getHostName());
-        Tags.PEER_PORT.set(clientSpan, host.getPort());
-      }
-
-      onSpanStarted(clientSpan, httpRequest, httpContext);
-
-      tracer.inject(
-          clientSpan.context(), Format.Builtin.HTTP_HEADERS, new ClientRequestCarrier(httpRequest));
-
-      httpContext.setAttribute(Constants.CURRENT_SPAN_CONTEXT_KEY, clientSpan);
+      this.spanCreationInterceptor.process(httpRequest, httpContext);
+      onSpanStarted(TracingUtils.getTraceContext().getCurrentSpan(), httpRequest, httpContext);
+      this.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

@@ -113,7 +113,7 @@ public void testHttpClientTracing() throws Exception {
     HttpClientBuilder clientBuilder = HttpClients.custom();
     CloseableHttpClient client = TracingInterceptors.addTo(clientBuilder, tracer).build();
 
-    //Make a request to the async client and wait for response
+    //Make a request to the sync client and wait for response
     client.execute(
         new HttpHost("localhost", mockServerRule.getPort()),
         new BasicHttpRequest("GET", "/testing"));
@@ -124,7 +124,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(2, spans.size());
+    assertEquals(3, spans.size());
     Span span = spans.get(1);
     assertEquals("GET", span.getOperationName());
     assertEquals(parentSpan.context().getSpanId(), span.context().getParentId());
@@ -139,4 +139,26 @@ private void verifyTracing(Span parentSpan) {
     String baggage = httpRequests[0].getFirstHeader("uberctx-" + BAGGAGE_KEY);
     assertEquals(BAGGAGE_VALUE, baggage);
   }
+
+  @Test
+  public void testHttpClientTracingWithSplitSpanCreationInterceptors() throws Exception {
+    HttpClientBuilder clientBuilder = HttpClients.custom();
+    CloseableHttpClient client = CustomTracingInterceptor.addTo(clientBuilder, tracer).build();
+
+    // Make a request to the sync client and wait for response
+    client.execute(
+        new HttpHost("localhost", mockServerRule.getPort()),
+        new BasicHttpRequest("GET", "/testing"));
+
+    verifyTracing(parentSpan);
+  }
+
+  static class CustomTracingInterceptor {
+    static HttpClientBuilder addTo(HttpClientBuilder clientBuilder, Tracer tracer) {
+      return clientBuilder
+          .addInterceptorFirst(new SpanCreationRequestInterceptor(tracer))
+          .addInterceptorLast(new SpanInjectionRequestInterceptor(tracer))
+          .addInterceptorFirst(new TracingResponseInterceptor());
+    }
+  }
 }