Skip to content

Commit

Permalink
Fixes where vert.x span ended prior to response headers
Browse files Browse the repository at this point in the history
Before, we accidentally ended a span when the request ended, vs when
response headers were sent. This prevents that and also removes
unnecessary duplication of handler code which was blindly copied from
opentracing as opposed to tested to see if actually necessary.
  • Loading branch information
Adrian Cole authored and adriancole committed Apr 2, 2018
1 parent b744165 commit 821a158
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
2 changes: 1 addition & 1 deletion instrumentation/vertx-web/pom.xml
Expand Up @@ -12,7 +12,7 @@

<properties>
<main.basedir>${project.basedir}/../..</main.basedir>
<vertx.version>3.4.2</vertx.version>
<vertx.version>3.5.1</vertx.version>
<main.java.version>1.8</main.java.version>
<main.signature.artifact>java18</main.signature.artifact>
</properties>
Expand Down
Expand Up @@ -12,6 +12,7 @@
import io.vertx.core.http.HttpServerResponse;
import io.vertx.core.net.SocketAddress;
import io.vertx.ext.web.RoutingContext;
import java.util.concurrent.atomic.AtomicBoolean;
import zipkin2.Endpoint;

/**
Expand Down Expand Up @@ -71,10 +72,6 @@ final class TracingRoutingContextHandler implements Handler<RoutingContext> {
Span span = serverHandler.handleReceive(extractor, context.request());
TracingHandler handler = new TracingHandler(context, span);
context.put(TracingHandler.class.getName(), handler);

// When a route ends a request directly, this will finish the span
context.request().endHandler(handler);
// When a route overwrites the above endHandler, this will finish the span
context.addHeadersEndHandler(handler);

try (Tracer.SpanInScope ws = tracer.withSpanInScope(span)) {
Expand All @@ -85,14 +82,15 @@ final class TracingRoutingContextHandler implements Handler<RoutingContext> {
class TracingHandler implements Handler<Void> {
final RoutingContext context;
final Span span;
final AtomicBoolean finished = new AtomicBoolean();

TracingHandler(RoutingContext context, Span span) {
this.context = context;
this.span = span;
}

@Override public void handle(Void aVoid) {
if (!context.request().isEnded()) return;
if (!finished.compareAndSet(false, true)) return;
Route route = new Route(context.request().rawMethod(), context.currentRoute().getPath());
try {
currentRoute.set(route);
Expand Down
Expand Up @@ -15,10 +15,11 @@ public static VertxWebTracing create(HttpTracing httpTracing) {
return new VertxWebTracing(httpTracing);
}

final Handler<RoutingContext> routingContextHandler;
final HttpTracing httpTracing;

VertxWebTracing(HttpTracing httpTracing) {
routingContextHandler = new TracingRoutingContextHandler(httpTracing);
if (httpTracing == null) throw new NullPointerException("httpTracing == null");
this.httpTracing = httpTracing;
}

/**
Expand All @@ -34,6 +35,6 @@ public static VertxWebTracing create(HttpTracing httpTracing) {
* }</pre>
*/
public Handler<RoutingContext> routingContextHandler() {
return routingContextHandler;
return new TracingRoutingContextHandler(httpTracing);
}
}

0 comments on commit 821a158

Please sign in to comment.