Skip to content

Commit 36b49c7

Browse files
authored
Merge pull request DataDog#451 from DataDog/tyler/netty-client-fixes
Allow trace to persist across netty connect.
2 parents 4d91cf1 + 898647e commit 36b49c7

25 files changed

Lines changed: 498 additions & 90 deletions

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ abstract class Default implements Instrumenter {
5151
private final String instrumentationPrimaryName;
5252
protected final boolean enabled;
5353

54+
protected final String packageName =
55+
getClass().getPackage() == null ? "" : getClass().getPackage().getName();
56+
5457
public Default(final String instrumentationName, final String... additionalNames) {
5558
instrumentationNames = new HashSet<>(Arrays.asList(additionalNames));
5659
instrumentationNames.add(instrumentationName);

dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,18 @@ public final class ExecutorInstrumentation extends Instrumenter.Default {
7979
"akka.dispatch.PinnedDispatcher",
8080
"akka.dispatch.ExecutionContexts$sameThreadExecutionContext$",
8181
"akka.dispatch.ExecutionContexts$sameThreadExecutionContext$",
82-
"play.api.libs.streams.Execution$trampoline$"
82+
"play.api.libs.streams.Execution$trampoline$",
83+
"io.netty.channel.MultithreadEventLoopGroup",
84+
"io.netty.util.concurrent.MultithreadEventExecutorGroup",
85+
"io.netty.util.concurrent.AbstractEventExecutorGroup",
86+
"io.netty.channel.epoll.EpollEventLoopGroup",
87+
"io.netty.channel.nio.NioEventLoopGroup",
88+
"io.netty.util.concurrent.GlobalEventExecutor",
89+
"io.netty.util.concurrent.AbstractScheduledEventExecutor",
90+
"io.netty.util.concurrent.AbstractEventExecutor",
91+
"io.netty.util.concurrent.SingleThreadEventExecutor",
92+
"io.netty.channel.nio.NioEventLoop",
93+
"io.netty.channel.SingleThreadEventLoop",
8394
};
8495
WHITELISTED_EXECUTORS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(whitelist)));
8596

dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ dependencies {
3131
implementation deps.autoservice
3232

3333
testCompile project(':dd-java-agent:testing')
34+
testCompile project(':dd-java-agent:instrumentation:java-concurrent')
3435

35-
// testCompile group: 'io.netty', name: 'netty-all', version: '4.0.0.Final'
36+
testCompile group: 'io.netty', name: 'netty-codec-http', version: '4.0.0.Final'
3637
testCompile group: 'org.asynchttpclient', name: 'async-http-client', version: '2.0.0'
3738
}
3839

@@ -50,7 +51,7 @@ configurations.testCompile {
5051

5152
configurations.latestDepTestCompile {
5253
resolutionStrategy {
53-
force group: 'io.netty', name: 'netty-all', version: '4.0.56.Final'
54+
force group: 'io.netty', name: 'netty-codec-http', version: '4.0.56.Final'
5455
force group: 'org.asynchttpclient', name: 'async-http-client', version: '2.0.+'
5556
}
5657
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package datadog.trace.instrumentation.netty40;
2+
3+
import datadog.trace.context.TraceScope;
4+
import datadog.trace.instrumentation.netty40.server.HttpServerTracingHandler;
5+
import io.netty.util.AttributeKey;
6+
import io.opentracing.Span;
7+
8+
public class AttributeKeys {
9+
public static final AttributeKey<TraceScope.Continuation>
10+
PARENT_CONNECT_CONTINUATION_ATTRIBUTE_KEY =
11+
new AttributeKey<>("datadog.trace.instrumentation.netty40.parent.connect.continuation");
12+
13+
public static final AttributeKey<Span> SERVER_ATTRIBUTE_KEY =
14+
new AttributeKey<>(HttpServerTracingHandler.class.getName() + ".span");
15+
16+
public static final AttributeKey<Span> CLIENT_ATTRIBUTE_KEY =
17+
new AttributeKey<>(HttpServerTracingHandler.class.getName() + ".span");
18+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package datadog.trace.instrumentation.netty40;
2+
3+
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
4+
import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
5+
import static io.opentracing.log.Fields.ERROR_OBJECT;
6+
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
7+
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
8+
import static net.bytebuddy.matcher.ElementMatchers.named;
9+
import static net.bytebuddy.matcher.ElementMatchers.not;
10+
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
11+
12+
import com.google.auto.service.AutoService;
13+
import datadog.trace.agent.tooling.Instrumenter;
14+
import datadog.trace.context.TraceScope;
15+
import io.netty.channel.ChannelFuture;
16+
import io.opentracing.Scope;
17+
import io.opentracing.Span;
18+
import io.opentracing.tag.Tags;
19+
import io.opentracing.util.GlobalTracer;
20+
import java.util.Collections;
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
import net.bytebuddy.asm.Advice;
24+
import net.bytebuddy.description.type.TypeDescription;
25+
import net.bytebuddy.matcher.ElementMatcher;
26+
27+
@AutoService(Instrumenter.class)
28+
public class ChannelFutureListenerInstrumentation extends Instrumenter.Default {
29+
30+
public ChannelFutureListenerInstrumentation() {
31+
super("netty", "netty-4.0");
32+
}
33+
34+
@Override
35+
protected boolean defaultEnabled() {
36+
return false;
37+
}
38+
39+
@Override
40+
public ElementMatcher<TypeDescription> typeMatcher() {
41+
return not(isInterface())
42+
.and(safeHasSuperType(named("io.netty.channel.ChannelFutureListener")));
43+
}
44+
45+
@Override
46+
public ElementMatcher<ClassLoader> classLoaderMatcher() {
47+
return classLoaderHasClasses("io.netty.handler.codec.spdy.SpdyOrHttpChooser");
48+
}
49+
50+
@Override
51+
public String[] helperClassNames() {
52+
return new String[] {packageName + ".AttributeKeys"};
53+
}
54+
55+
@Override
56+
public Map<ElementMatcher, String> transformers() {
57+
final Map<ElementMatcher, String> transformers = new HashMap<>();
58+
transformers.put(
59+
isMethod()
60+
.and(named("operationComplete"))
61+
.and(takesArgument(0, named("io.netty.channel.ChannelFuture"))),
62+
OperationCompleteAdvice.class.getName());
63+
return transformers;
64+
}
65+
66+
public static class OperationCompleteAdvice {
67+
@Advice.OnMethodEnter
68+
public static TraceScope activateScope(@Advice.Argument(0) final ChannelFuture future) {
69+
final TraceScope.Continuation continuation =
70+
future.channel().attr(AttributeKeys.PARENT_CONNECT_CONTINUATION_ATTRIBUTE_KEY).get();
71+
72+
if (continuation == null) {
73+
return null;
74+
}
75+
final TraceScope scope = continuation.activate();
76+
77+
final Throwable cause = future.cause();
78+
if (cause != null) {
79+
final Span errorSpan =
80+
GlobalTracer.get()
81+
.buildSpan("netty.connect")
82+
.withTag(Tags.COMPONENT.getKey(), "netty")
83+
.start();
84+
Tags.ERROR.set(errorSpan, true);
85+
errorSpan.log(Collections.singletonMap(ERROR_OBJECT, cause));
86+
errorSpan.finish();
87+
}
88+
89+
return scope;
90+
}
91+
92+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
93+
public static void deactivateScope(
94+
@Advice.Enter final TraceScope scope, @Advice.Thrown final Throwable throwable) {
95+
if (scope != null) {
96+
((Scope) scope).close();
97+
}
98+
}
99+
}
100+
}

dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
88
import static net.bytebuddy.matcher.ElementMatchers.named;
99
import static net.bytebuddy.matcher.ElementMatchers.not;
10+
import static net.bytebuddy.matcher.ElementMatchers.returns;
1011
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
1112

1213
import com.google.auto.service.AutoService;
1314
import datadog.trace.agent.tooling.Instrumenter;
1415
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
16+
import datadog.trace.context.TraceScope;
1517
import datadog.trace.instrumentation.netty40.client.HttpClientRequestTracingHandler;
1618
import datadog.trace.instrumentation.netty40.client.HttpClientResponseTracingHandler;
1719
import datadog.trace.instrumentation.netty40.client.HttpClientTracingHandler;
@@ -26,6 +28,8 @@
2628
import io.netty.handler.codec.http.HttpResponseDecoder;
2729
import io.netty.handler.codec.http.HttpResponseEncoder;
2830
import io.netty.handler.codec.http.HttpServerCodec;
31+
import io.opentracing.Scope;
32+
import io.opentracing.util.GlobalTracer;
2933
import java.util.HashMap;
3034
import java.util.Map;
3135
import net.bytebuddy.asm.Advice;
@@ -35,11 +39,8 @@
3539
@AutoService(Instrumenter.class)
3640
public class NettyChannelPipelineInstrumentation extends Instrumenter.Default {
3741

38-
private static final String PACKAGE =
39-
NettyChannelPipelineInstrumentation.class.getPackage().getName();
40-
4142
public NettyChannelPipelineInstrumentation() {
42-
super("netty", "netty-4.1");
43+
super("netty", "netty-4.0");
4344
}
4445

4546
@Override
@@ -60,16 +61,17 @@ public ElementMatcher<ClassLoader> classLoaderMatcher() {
6061
@Override
6162
public String[] helperClassNames() {
6263
return new String[] {
64+
packageName + ".AttributeKeys",
6365
// client helpers
64-
PACKAGE + ".client.NettyResponseInjectAdapter",
65-
PACKAGE + ".client.HttpClientRequestTracingHandler",
66-
PACKAGE + ".client.HttpClientResponseTracingHandler",
67-
PACKAGE + ".client.HttpClientTracingHandler",
66+
packageName + ".client.NettyResponseInjectAdapter",
67+
packageName + ".client.HttpClientRequestTracingHandler",
68+
packageName + ".client.HttpClientResponseTracingHandler",
69+
packageName + ".client.HttpClientTracingHandler",
6870
// server helpers
69-
PACKAGE + ".server.NettyRequestExtractAdapter",
70-
PACKAGE + ".server.HttpServerRequestTracingHandler",
71-
PACKAGE + ".server.HttpServerResponseTracingHandler",
72-
PACKAGE + ".server.HttpServerTracingHandler"
71+
packageName + ".server.NettyRequestExtractAdapter",
72+
packageName + ".server.HttpServerRequestTracingHandler",
73+
packageName + ".server.HttpServerResponseTracingHandler",
74+
packageName + ".server.HttpServerTracingHandler"
7375
};
7476
}
7577

@@ -81,6 +83,9 @@ public Map<ElementMatcher, String> transformers() {
8183
.and(nameStartsWith("add"))
8284
.and(takesArgument(2, named("io.netty.channel.ChannelHandler"))),
8385
ChannelPipelineAddAdvice.class.getName());
86+
transformers.put(
87+
isMethod().and(named("connect")).and(returns(named("io.netty.channel.ChannelFuture"))),
88+
ChannelPipelineConnectAdvice.class.getName());
8489
return transformers;
8590
}
8691

@@ -138,4 +143,18 @@ public static void addHandler(
138143
}
139144
}
140145
}
146+
147+
public static class ChannelPipelineConnectAdvice {
148+
@Advice.OnMethodEnter
149+
public static void addParentSpan(@Advice.This final ChannelPipeline pipeline) {
150+
final Scope scope = GlobalTracer.get().scopeManager().active();
151+
152+
if (scope instanceof TraceScope) {
153+
pipeline
154+
.channel()
155+
.attr(AttributeKeys.PARENT_CONNECT_CONTINUATION_ATTRIBUTE_KEY)
156+
.set(((TraceScope) scope).capture());
157+
}
158+
}
159+
}
141160
}

dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import datadog.trace.api.DDSpanTypes;
77
import datadog.trace.api.DDTags;
8+
import datadog.trace.instrumentation.netty40.AttributeKeys;
89
import io.netty.channel.ChannelHandlerContext;
910
import io.netty.channel.ChannelOutboundHandlerAdapter;
1011
import io.netty.channel.ChannelPromise;
@@ -47,7 +48,7 @@ public void write(final ChannelHandlerContext ctx, final Object msg, final Chann
4748
.inject(
4849
span.context(), Format.Builtin.HTTP_HEADERS, new NettyResponseInjectAdapter(request));
4950

50-
ctx.channel().attr(HttpClientTracingHandler.attributeKey).set(span);
51+
ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).set(span);
5152

5253
try {
5354
ctx.write(msg, prm);

dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,36 +2,53 @@
22

33
import static io.opentracing.log.Fields.ERROR_OBJECT;
44

5+
import datadog.trace.context.TraceScope;
6+
import datadog.trace.instrumentation.netty40.AttributeKeys;
57
import io.netty.channel.ChannelHandlerContext;
68
import io.netty.channel.ChannelInboundHandlerAdapter;
79
import io.netty.handler.codec.http.HttpResponse;
10+
import io.opentracing.Scope;
811
import io.opentracing.Span;
912
import io.opentracing.tag.Tags;
13+
import io.opentracing.util.GlobalTracer;
1014
import java.util.Collections;
1115

1216
public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter {
1317

1418
@Override
1519
public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
16-
final Span span = ctx.channel().attr(HttpClientTracingHandler.attributeKey).get();
17-
if (span == null || !(msg instanceof HttpResponse)) {
20+
final Span span = ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).get();
21+
if (span == null) {
1822
ctx.fireChannelRead(msg);
1923
return;
2024
}
2125

22-
final HttpResponse response = (HttpResponse) msg;
26+
try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) {
27+
final boolean finishSpan = msg instanceof HttpResponse;
2328

24-
try {
25-
ctx.fireChannelRead(msg);
26-
} catch (final Throwable throwable) {
27-
Tags.ERROR.set(span, Boolean.TRUE);
28-
span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
29-
Tags.HTTP_STATUS.set(span, 500);
30-
span.finish(); // Finish the span manually since finishSpanOnClose was false
31-
throw throwable;
32-
}
29+
if (scope instanceof TraceScope) {
30+
((TraceScope) scope).setAsyncPropagation(true);
31+
}
32+
try {
33+
ctx.fireChannelRead(msg);
34+
} catch (final Throwable throwable) {
35+
if (finishSpan) {
36+
Tags.ERROR.set(span, Boolean.TRUE);
37+
span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
38+
Tags.HTTP_STATUS.set(span, 500);
39+
span.finish(); // Finish the span manually since finishSpanOnClose was false
40+
throw throwable;
41+
}
42+
}
3343

34-
Tags.HTTP_STATUS.set(span, response.getStatus().code());
35-
span.finish(); // Finish the span manually since finishSpanOnClose was false
44+
if (scope instanceof TraceScope) {
45+
((TraceScope) scope).setAsyncPropagation(false);
46+
}
47+
48+
if (finishSpan) {
49+
Tags.HTTP_STATUS.set(span, ((HttpResponse) msg).getStatus().code());
50+
span.finish(); // Finish the span manually since finishSpanOnClose was false
51+
}
52+
}
3653
}
3754
}

dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientTracingHandler.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
package datadog.trace.instrumentation.netty40.client;
22

33
import io.netty.channel.CombinedChannelDuplexHandler;
4-
import io.netty.util.AttributeKey;
5-
import io.opentracing.Span;
64

75
public class HttpClientTracingHandler
86
extends CombinedChannelDuplexHandler<
97
HttpClientResponseTracingHandler, HttpClientRequestTracingHandler> {
108

11-
static final AttributeKey<Span> attributeKey =
12-
new AttributeKey<>(HttpClientTracingHandler.class.getName());
13-
149
public HttpClientTracingHandler() {
1510
super(new HttpClientResponseTracingHandler(), new HttpClientRequestTracingHandler());
1611
}

dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import datadog.trace.api.DDSpanTypes;
77
import datadog.trace.api.DDTags;
88
import datadog.trace.context.TraceScope;
9+
import datadog.trace.instrumentation.netty40.AttributeKeys;
910
import io.netty.channel.ChannelHandlerContext;
1011
import io.netty.channel.ChannelInboundHandlerAdapter;
1112
import io.netty.handler.codec.http.HttpRequest;
@@ -55,7 +56,7 @@ public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
5556
}
5657

5758
final Span span = scope.span();
58-
ctx.channel().attr(HttpServerTracingHandler.attributeKey).set(span);
59+
ctx.channel().attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(span);
5960

6061
try {
6162
ctx.fireChannelRead(msg);

0 commit comments

Comments
 (0)