Use NoopSpan instead of null for null parent.

Fix some more tests.
This commit is contained in:
Tyler Benson 2019-07-18 15:36:25 -07:00
parent e692605a3b
commit ea4fc4ab28
5 changed files with 44 additions and 34 deletions

View File

@ -25,8 +25,16 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest<AkkaHttpClientDec
def response def response
try { try {
response = Http.get(system).singleRequest(request, materializer).toCompletableFuture().get() response = Http.get(system)
.singleRequest(request, materializer)
//.whenComplete { result, error ->
// FIXME: Callback should be here instead.
// callback?.call()
//}
.toCompletableFuture()
.get()
} finally { } finally {
// FIXME: remove this when callback above works.
blockUntilChildSpansFinished(1) blockUntilChildSpansFinished(1)
} }
callback?.call() callback?.call()

View File

@ -1,6 +1,5 @@
import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
import io.opentracing.util.GlobalTracer
import org.apache.http.HttpResponse import org.apache.http.HttpResponse
import org.apache.http.concurrent.FutureCallback import org.apache.http.concurrent.FutureCallback
import org.apache.http.impl.nio.client.HttpAsyncClients import org.apache.http.impl.nio.client.HttpAsyncClients
@ -22,8 +21,6 @@ class ApacheHttpAsyncClientCallbackTest extends HttpClientTest<ApacheHttpAsyncCl
@Override @Override
int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) { int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
def hasParent = GlobalTracer.get().activeSpan() != null
def request = new HttpUriRequest(method, uri) def request = new HttpUriRequest(method, uri)
headers.entrySet().each { headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value)) request.addHeader(new BasicHeader(it.key, it.value))
@ -35,21 +32,13 @@ class ApacheHttpAsyncClientCallbackTest extends HttpClientTest<ApacheHttpAsyncCl
@Override @Override
void completed(HttpResponse result) { void completed(HttpResponse result) {
if (hasParent && GlobalTracer.get().activeSpan() == null) { responseFuture.complete(result.statusLine.statusCode)
responseFuture.completeExceptionally(new Exception("Missing span in scope"))
} else {
responseFuture.complete(result.statusLine.statusCode)
}
callback?.call() callback?.call()
} }
@Override @Override
void failed(Exception ex) { void failed(Exception ex) {
if (hasParent && GlobalTracer.get().activeSpan() == null) { responseFuture.completeExceptionally(ex)
responseFuture.completeExceptionally(new Exception("Missing span in scope"))
} else {
responseFuture.completeExceptionally(ex)
}
} }
@Override @Override

View File

@ -4,6 +4,7 @@ import javax.ws.rs.client.AsyncInvoker
import javax.ws.rs.client.Client import javax.ws.rs.client.Client
import javax.ws.rs.client.ClientBuilder import javax.ws.rs.client.ClientBuilder
import javax.ws.rs.client.Entity import javax.ws.rs.client.Entity
import javax.ws.rs.client.InvocationCallback
import javax.ws.rs.client.WebTarget import javax.ws.rs.client.WebTarget
import javax.ws.rs.core.MediaType import javax.ws.rs.core.MediaType
import javax.ws.rs.core.Response import javax.ws.rs.core.Response
@ -22,8 +23,16 @@ abstract class JaxRsClientAsyncTest extends HttpClientTest<JaxRsClientDecorator>
AsyncInvoker request = builder.async() AsyncInvoker request = builder.async()
def body = BODY_METHODS.contains(method) ? Entity.text("") : null def body = BODY_METHODS.contains(method) ? Entity.text("") : null
Response response = request.method(method, (Entity) body).get() Response response = request.method(method, (Entity) body, new InvocationCallback<Response>(){
callback?.call() @Override
void completed(Response s) {
callback?.call()
}
@Override
void failed(Throwable throwable) {
}
}).get()
return response.status return response.status
} }

View File

@ -7,35 +7,37 @@ import datadog.trace.instrumentation.netty40.AttributeKeys;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpResponse;
import io.netty.util.Attribute;
import io.opentracing.Scope; import io.opentracing.Scope;
import io.opentracing.Span; import io.opentracing.Span;
import io.opentracing.noop.NoopSpan;
import io.opentracing.util.GlobalTracer; import io.opentracing.util.GlobalTracer;
public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter {
@Override @Override
public void channelRead(final ChannelHandlerContext ctx, final Object msg) { public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
final Span parent = ctx.channel().attr(AttributeKeys.CLIENT_PARENT_ATTRIBUTE_KEY).get(); final Attribute<Span> 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 Span span = ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).get();
final boolean finishSpan = msg instanceof HttpResponse; final boolean finishSpan = msg instanceof HttpResponse;
if (span != null && finishSpan) { 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.onResponse(span, (HttpResponse) msg);
DECORATE.beforeFinish(span); DECORATE.beforeFinish(span);
span.finish();
} }
} }
// We want the callback in the scope of the parent, not the client span // 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)) {
try (final Scope scope = GlobalTracer.get().scopeManager().activate(parent, false)) { if (scope instanceof TraceScope) {
if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true);
((TraceScope) scope).setAsyncPropagation(true);
}
ctx.fireChannelRead(msg);
} }
} else {
ctx.fireChannelRead(msg); ctx.fireChannelRead(msg);
} }
} }

View File

@ -7,35 +7,37 @@ import datadog.trace.instrumentation.netty41.AttributeKeys;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpResponse;
import io.netty.util.Attribute;
import io.opentracing.Scope; import io.opentracing.Scope;
import io.opentracing.Span; import io.opentracing.Span;
import io.opentracing.noop.NoopSpan;
import io.opentracing.util.GlobalTracer; import io.opentracing.util.GlobalTracer;
public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter {
@Override @Override
public void channelRead(final ChannelHandlerContext ctx, final Object msg) { public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
final Span parent = ctx.channel().attr(AttributeKeys.CLIENT_PARENT_ATTRIBUTE_KEY).get(); final Attribute<Span> 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 Span span = ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).get();
final boolean finishSpan = msg instanceof HttpResponse; final boolean finishSpan = msg instanceof HttpResponse;
if (span != null && finishSpan) { 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.onResponse(span, (HttpResponse) msg);
DECORATE.beforeFinish(span); DECORATE.beforeFinish(span);
span.finish();
} }
} }
// We want the callback in the scope of the parent, not the client span // 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)) {
try (final Scope scope = GlobalTracer.get().scopeManager().activate(parent, false)) { if (scope instanceof TraceScope) {
if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true);
((TraceScope) scope).setAsyncPropagation(true);
}
ctx.fireChannelRead(msg);
} }
} else {
ctx.fireChannelRead(msg); ctx.fireChannelRead(msg);
} }
} }