Actually finish() the span :/

lucidsoftware · Mar 10, 2017 · d8478c3 · d8478c3
1 parent c65a2e8
commit d8478c3
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 25 deletions.
diff --git a/...context/src/main/java/com/github/threadcontext/httpasyncclient/ContextFutureCallback.java b/...context/src/main/java/com/github/threadcontext/httpasyncclient/ContextFutureCallback.java
@@ -22,7 +22,7 @@ public void failed(Exception ex) {
     }
 
     public void cancelled() {
-        context.run(() -> delegate.cancelled());
+        context.run(delegate::cancelled);
     }
 
 }
diff --git a/httpasyncclient/src/main/java/io/opentracing/contrib/httpcomponents/SpanFutureCallback.java b/httpasyncclient/src/main/java/io/opentracing/contrib/httpcomponents/SpanFutureCallback.java
@@ -0,0 +1,34 @@
+package io.opentracing.contrib.httpcomponents;
+
+import io.opentracing.Span;
+import io.opentracing.tag.Tags;
+import org.apache.http.concurrent.FutureCallback;
+
+public class SpanFutureCallback<T> implements FutureCallback<T> {
+
+    private final FutureCallback<T> delegate;
+    private final Span span;
+
+    public SpanFutureCallback(FutureCallback<T> delegate, Span span) {
+        this.delegate = delegate;
+        this.span = span;
+    }
+
+    public void completed(T result) {
+        span.finish();
+        delegate.completed(result);
+    }
+
+    public void failed(Exception ex) {
+        span.finish();
+        Tags.ERROR.set(span, true);
+        delegate.failed(ex);
+    }
+
+    public void cancelled() {
+        span.finish();
+        Tags.ERROR.set(span, true);
+        span.log("client cancel");
+    }
+
+}
diff --git a/httpasyncclient/src/main/java/io/opentracing/contrib/httpcomponents/SpanHttpAsyncClient.java b/httpasyncclient/src/main/java/io/opentracing/contrib/httpcomponents/SpanHttpAsyncClient.java
@@ -41,7 +41,12 @@ public <T> Future<T> execute(HttpAsyncRequestProducer producer, HttpAsyncRespons
         Span span = tracer.buildSpan("HTTP").addReference(References.CHILD_OF, contextSpan.get().context())
             .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
             .start();
-        return contextSpan.set(span).supply(() -> client.execute(producer, responseConsumer, context, callback));
+        return contextSpan.set(span).supply(() -> client.execute(
+            new SpanHttpAsyncRequestProducer(producer, span),
+            new SpanHttpAsyncResponseConsumer<>(responseConsumer, span),
+            context,
+            new SpanFutureCallback<>(callback, span)
+        ));
     }
 
 }
diff --git a/...ent/src/main/java/io/opentracing/contrib/httpcomponents/SpanHttpAsyncRequestProducer.java b/...ent/src/main/java/io/opentracing/contrib/httpcomponents/SpanHttpAsyncRequestProducer.java
@@ -38,7 +38,7 @@ public void produceContent(ContentEncoder encoder, IOControl ioctrl) throws IOEx
     }
 
     public void requestCompleted(HttpContext context) {
-        span.log("requestCompleted");
+        span.log("request complete");
         delegate.requestCompleted(context);
     }
 

diff --git a/...nt/src/main/java/io/opentracing/contrib/httpcomponents/SpanHttpAsyncResponseConsumer.java b/...nt/src/main/java/io/opentracing/contrib/httpcomponents/SpanHttpAsyncResponseConsumer.java
@@ -22,6 +22,7 @@ public SpanHttpAsyncResponseConsumer(HttpAsyncResponseConsumer<T> delegate, Span
 
     public void responseReceived(HttpResponse response) throws IOException, HttpException {
         StandardHttpTagger.tagResponse(span, response);
+        delegate.responseReceived(response);
     }
 
     public void consumeContent(ContentDecoder decoder, IOControl ioctrl) throws IOException {
@@ -55,7 +56,7 @@ public void close() throws IOException {
     }
 
     public boolean cancel() {
-        span.log("cancel");
+        span.log("response cancel");
         return delegate.cancel();
     }
 

diff --git a/httpclient/src/main/java/io/opentracing/contrib/httpcomponents/SpanExec.java b/httpclient/src/main/java/io/opentracing/contrib/httpcomponents/SpanExec.java
@@ -29,27 +29,28 @@ public SpanExec(ClientExecChain exec, Tracer tracer, ContextSpan contextSpan, Ht
     }
 
     public CloseableHttpResponse execute(HttpRoute route, HttpRequestWrapper request, HttpClientContext context, HttpExecutionAware execAware) throws IOException, HttpException {
-        Span span = tracer.buildSpan("HTTP " + request.getRequestLine().getMethod())
-                .asChildOf(contextSpan.get())
-                .start();
-        this.tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new HttpRequestTextMap(request));
-
-        return contextSpan.set(span).<CloseableHttpResponse, HttpException, IOException>supplyException2(() -> {
-            for (HttpTagger tagger : taggers) {
-                tagger.tag(span, route, request, context);
-            }
-            CloseableHttpResponse response;
-            try {
-                response = exec.execute(route, request, context, execAware);
-            } catch (HttpException | IOException e) {
-                Tags.ERROR.set(span, true);
-                throw e;
-            }
-            for (HttpTagger tagger : taggers) {
-                tagger.tag(span, route, response, context);
-            }
-            return response;
-        });
+        Tracer.SpanBuilder spanBuilder = tracer.buildSpan("HTTP " + request.getRequestLine().getMethod())
+                .asChildOf(contextSpan.get());
+        try(Span span = spanBuilder.start()) {
+            this.tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new HttpRequestTextMap(request));
+            return contextSpan.set(span).<CloseableHttpResponse, HttpException, IOException>supplyException2(() -> {
+                for (HttpTagger tagger : taggers) {
+                    tagger.tag(span, route, request, context);
+                }
+                CloseableHttpResponse response;
+                try {
+                    response = exec.execute(route, request, context, execAware);
+                } catch (HttpException | IOException e) {
+                    Tags.ERROR.set(span, true);
+                    throw e;
+                }
+                for (HttpTagger tagger : taggers) {
+                    tagger.tag(span, route, response, context);
+                }
+                span.finish();
+                return response;
+            });
+        }
     }
 
 }