diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy index 3ae9770f32..696d153a67 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy @@ -25,8 +25,16 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest + // FIXME: Callback should be here instead. + // callback?.call() + //} + .toCompletableFuture() + .get() } finally { + // FIXME: remove this when callback above works. blockUntilChildSpansFinished(1) } callback?.call() diff --git a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy b/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy index fde0e1bfad..44b31ab31c 100644 --- a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy +++ b/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy @@ -1,6 +1,5 @@ import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator -import io.opentracing.util.GlobalTracer import org.apache.http.HttpResponse import org.apache.http.concurrent.FutureCallback import org.apache.http.impl.nio.client.HttpAsyncClients @@ -22,8 +21,6 @@ class ApacheHttpAsyncClientCallbackTest extends HttpClientTest headers, Closure callback) { - def hasParent = GlobalTracer.get().activeSpan() != null - def request = new HttpUriRequest(method, uri) headers.entrySet().each { request.addHeader(new BasicHeader(it.key, it.value)) @@ -35,21 +32,13 @@ class ApacheHttpAsyncClientCallbackTest extends HttpClientTest AsyncInvoker request = builder.async() def body = BODY_METHODS.contains(method) ? Entity.text("") : null - Response response = request.method(method, (Entity) body).get() - callback?.call() + Response response = request.method(method, (Entity) body, new InvocationCallback(){ + @Override + void completed(Response s) { + callback?.call() + } + + @Override + void failed(Throwable throwable) { + } + }).get() return response.status } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java index 6dedbb0157..ac1865b6bf 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java @@ -7,35 +7,37 @@ import datadog.trace.instrumentation.netty40.AttributeKeys; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.http.HttpResponse; +import io.netty.util.Attribute; import io.opentracing.Scope; import io.opentracing.Span; +import io.opentracing.noop.NoopSpan; import io.opentracing.util.GlobalTracer; public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { @Override public void channelRead(final ChannelHandlerContext ctx, final Object msg) { - final Span parent = ctx.channel().attr(AttributeKeys.CLIENT_PARENT_ATTRIBUTE_KEY).get(); + final Attribute parentAttr = + ctx.channel().attr(AttributeKeys.CLIENT_PARENT_ATTRIBUTE_KEY); + parentAttr.setIfAbsent(NoopSpan.INSTANCE); + final Span parent = parentAttr.get(); final Span span = ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).get(); final boolean finishSpan = msg instanceof HttpResponse; if (span != null && finishSpan) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onResponse(span, (HttpResponse) msg); DECORATE.beforeFinish(span); + span.finish(); } } // We want the callback in the scope of the parent, not the client span - if (parent != null) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(parent, false)) { - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(true); - } - ctx.fireChannelRead(msg); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(parent, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); } - } else { ctx.fireChannelRead(msg); } } diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java index a08926ec0e..c13a766361 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java @@ -7,35 +7,37 @@ import datadog.trace.instrumentation.netty41.AttributeKeys; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.http.HttpResponse; +import io.netty.util.Attribute; import io.opentracing.Scope; import io.opentracing.Span; +import io.opentracing.noop.NoopSpan; import io.opentracing.util.GlobalTracer; public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { @Override public void channelRead(final ChannelHandlerContext ctx, final Object msg) { - final Span parent = ctx.channel().attr(AttributeKeys.CLIENT_PARENT_ATTRIBUTE_KEY).get(); + final Attribute parentAttr = + ctx.channel().attr(AttributeKeys.CLIENT_PARENT_ATTRIBUTE_KEY); + parentAttr.setIfAbsent(NoopSpan.INSTANCE); + final Span parent = parentAttr.get(); final Span span = ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).get(); final boolean finishSpan = msg instanceof HttpResponse; if (span != null && finishSpan) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onResponse(span, (HttpResponse) msg); DECORATE.beforeFinish(span); + span.finish(); } } // We want the callback in the scope of the parent, not the client span - if (parent != null) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(parent, false)) { - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(true); - } - ctx.fireChannelRead(msg); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(parent, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); } - } else { ctx.fireChannelRead(msg); } }