Update to support 3.10 also and make muzzle check less brittle
MODIFY
dd-java-agent/instrumentation/netty-3.8/netty-3.8.gradle
dd-java-agent/instrumentation/netty-3.8/netty-3.8.gradle
@@ -10,7 +10,7 @@ muzzle {
pass {
group = "io.netty"
module = "netty"
- versions = "[3.8.0.Final,3.10)"
+ versions = "[3.8.0.Final,4)"
assertInverse = true
}
fail {
@@ -23,9 +23,7 @@ muzzle {
apply plugin: 'org.unbroken-dome.test-sets'
testSets {
- latestDepTest {
- dirName = 'test'
- }
+ latestDepTest
}
dependencies {
@@ -34,8 +32,8 @@ dependencies {
testCompile group: 'io.netty', name: 'netty', version: '3.8.0.Final'
testCompile group: 'com.ning', name: 'async-http-client', version: '1.8.0'
- latestDepTestCompile group: 'io.netty', name: 'netty', version: '3.9.+'
- latestDepTestCompile group: 'com.ning', name: 'async-http-client', version: '1.8.+'
+ latestDepTestCompile group: 'io.netty', name: 'netty', version: '3.10.+'
+ latestDepTestCompile group: 'com.ning', name: 'async-http-client', version: '1.9.+'
}
// We need to force the dependency to the earliest supported version because other libraries declare newer versions.

ADD
dd-java-agent/instrumentation/netty-3.8/src/latestDepTest/groovy/Netty38ClientTest.groovy
+import com.ning.http.client.AsyncCompletionHandler
+import com.ning.http.client.AsyncHttpClient
+import com.ning.http.client.AsyncHttpClientConfig
+import com.ning.http.client.Response
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.bootstrap.instrumentation.api.Tags
+import datadog.trace.instrumentation.netty38.client.NettyHttpClientDecorator
+import spock.lang.AutoCleanup
+import spock.lang.Shared
+
+import java.util.concurrent.ExecutionException
+import java.util.concurrent.TimeUnit
+
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
+import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
+import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+
+class Netty38ClientTest extends HttpClientTest {
+
+ @Shared
+ def clientConfig = new AsyncHttpClientConfig.Builder()
+ .setRequestTimeout(TimeUnit.SECONDS.toMillis(10).toInteger())
+ .build()
+
+ @Shared
+ @AutoCleanup
+ AsyncHttpClient asyncHttpClient = new AsyncHttpClient(clientConfig)
+
+ @Override
+ int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+ def methodName = "prepare" + method.toLowerCase().capitalize()
+ def requestBuilder = asyncHttpClient."$methodName"(uri.toString())
+ headers.each { requestBuilder.setHeader(it.key, it.value) }
+ def response = requestBuilder.execute(new AsyncCompletionHandler() {
+ @Override
+ Object onCompleted(Response response) throws Exception {
+ callback?.call()
+ return response
+ }
+ }).get()
+ blockUntilChildSpansFinished(1)
+ return response.statusCode
+ }
+
+ @Override
+ String component() {
+ return NettyHttpClientDecorator.DECORATE.component()
+ }
+
+ @Override
+ String expectedOperationName() {
+ return "netty.client.request"
+ }
+
+ @Override
+ boolean testRedirects() {
+ false
+ }
+
+ @Override
+ boolean testConnectionFailure() {
+ false
+ }
+
+ def "connection error (unopened port)"() {
+ given:
+ def uri = new URI("http://localhost:$UNUSABLE_PORT/")
+
+ when:
+ runUnderTrace("parent") {
+ doRequest(method, uri)
+ }
+
+ then:
+ def ex = thrown(Exception)
+ def thrownException = ex instanceof ExecutionException ? ex.cause : ex
+
+ and:
+ assertTraces(1) {
+ trace(0, 2) {
+ basicSpan(it, 0, "parent", null, thrownException)
+
+ span(1) {
+ operationName "netty.connect"
+ resourceName "netty.connect"
+ childOf span(0)
+ errored true
+ tags {
+ "$Tags.COMPONENT" "netty"
+ Class errorClass = ConnectException
+ try {
+ errorClass = Class.forName('io.netty.channel.AbstractChannel$AnnotatedConnectException')
+ } catch (ClassNotFoundException e) {
+ // Older versions use 'java.net.ConnectException' and do not have 'io.netty.channel.AbstractChannel$AnnotatedConnectException'
+ }
+ errorTags errorClass, "Connection refused: localhost/127.0.0.1:$UNUSABLE_PORT"
+ defaultTags()
+ }
+ }
+ }
+ }
+
+ where:
+ method = "GET"
+ }
+}

ADD
dd-java-agent/instrumentation/netty-3.8/src/latestDepTest/groovy/Netty38ServerTest.groovy
+import datadog.trace.agent.test.base.HttpServerTest
+import datadog.trace.instrumentation.netty38.server.NettyHttpServerDecorator
+import org.jboss.netty.bootstrap.ServerBootstrap
+import org.jboss.netty.buffer.ChannelBuffer
+import org.jboss.netty.buffer.ChannelBuffers
+import org.jboss.netty.channel.*
+import org.jboss.netty.channel.socket.nio.NioServerSocketChannelFactory
+import org.jboss.netty.handler.codec.http.*
+import org.jboss.netty.handler.logging.LoggingHandler
+import org.jboss.netty.logging.InternalLogLevel
+import org.jboss.netty.util.CharsetUtil
+
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.*
+import static org.jboss.netty.handler.codec.http.HttpHeaders.Names.*
+import static org.jboss.netty.handler.codec.http.HttpVersion.HTTP_1_1
+
+class Netty38ServerTest extends HttpServerTest<Channel> {
+
+ ChannelPipeline channelPipeline() {
+ ChannelPipeline channelPipeline = new DefaultChannelPipeline()
+
+ channelPipeline.addLast("http-codec", new HttpServerCodec())
+ channelPipeline.addLast("controller", new SimpleChannelHandler() {
+ @Override
+ void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) throws Exception {
+ if (msg.getMessage() instanceof HttpRequest) {
+ def uri = URI.create((msg.getMessage() as HttpRequest).getUri())
+ HttpServerTest.ServerEndpoint endpoint = forPath(uri.path)
+ ctx.sendDownstream controller(endpoint) {
+ HttpResponse response
+ ChannelBuffer responseContent = null
+ switch (endpoint) {
+ case SUCCESS:
+ case ERROR:
+ responseContent = ChannelBuffers.copiedBuffer(endpoint.body, CharsetUtil.UTF_8)
+ response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
+ response.setContent(responseContent)
+ break
+ case QUERY_PARAM:
+ responseContent = ChannelBuffers.copiedBuffer(uri.query, CharsetUtil.UTF_8)
+ response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
+ response.setContent(responseContent)
+ break
+ case REDIRECT:
+ response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
+ response.headers().set(LOCATION, endpoint.body)
+ break
+ case EXCEPTION:
+ throw new Exception(endpoint.body)
+ default:
+ responseContent = ChannelBuffers.copiedBuffer(NOT_FOUND.body, CharsetUtil.UTF_8)
+ response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
+ response.setContent(responseContent)
+ break
+ }
+ response.headers().set(CONTENT_TYPE, "text/plain")
+ if (responseContent) {
+ response.headers().set(CONTENT_LENGTH, responseContent.readableBytes())
+ }
+ return new DownstreamMessageEvent(
+ ctx.getChannel(),
+ new SucceededChannelFuture(ctx.getChannel()),
+ response,
+ ctx.getChannel().getRemoteAddress())
+ }
+ }
+ }
+
+ @Override
+ void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent ex) throws Exception {
+ ChannelBuffer buffer = ChannelBuffers.copiedBuffer(ex.getCause().getMessage(), CharsetUtil.UTF_8)
+ HttpResponse response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.INTERNAL_SERVER_ERROR)
+ response.setContent(buffer)
+ response.headers().set(CONTENT_TYPE, "text/plain")
+ response.headers().set(CONTENT_LENGTH, buffer.readableBytes())
+ ctx.sendDownstream(new DownstreamMessageEvent(
+ ctx.getChannel(),
+ new FailedChannelFuture(ctx.getChannel(), ex.getCause()),
+ response,
+ ctx.getChannel().getRemoteAddress()))
+ }
+ })
+
+ return channelPipeline
+ }
+
+ @Override
+ Channel startServer(int port) {
+ ServerBootstrap bootstrap = new ServerBootstrap(new NioServerSocketChannelFactory())
+ bootstrap.setParentHandler(new LoggingHandler(InternalLogLevel.INFO))
+ bootstrap.setPipeline(channelPipeline())
+
+ InetSocketAddress address = new InetSocketAddress(port)
+ return bootstrap.bind(address)
+ }
+
+ @Override
+ void stopServer(Channel server) {
+ server?.disconnect()
+ }
+
+ @Override
+ String component() {
+ NettyHttpServerDecorator.DECORATE.component()
+ }
+
+ @Override
+ String expectedOperationName() {
+ "netty.request"
+ }
+}

ADD
dd-java-agent/instrumentation/netty-3.8/src/latestDepTest/groovy/NettyServerTestInstrumentation.java
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.test.base.HttpServerTestAdvice;
+import datadog.trace.agent.tooling.Instrumenter;
+import net.bytebuddy.agent.builder.AgentBuilder;
+
+@AutoService(Instrumenter.class)
+public class NettyServerTestInstrumentation implements Instrumenter {
+
+ @Override
+ public AgentBuilder instrument(final AgentBuilder agentBuilder) {
+ return agentBuilder
+ .type(named("org.jboss.netty.handler.codec.http.HttpRequestDecoder"))
+ .transform(
+ new AgentBuilder.Transformer.ForAdvice()
+ .advice(
+ named("createMessage"),
+ HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
+ }
+}

ADD
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/AbstractNettyAdvice.java
+package datadog.trace.instrumentation.netty38;
+
+import org.jboss.netty.handler.codec.http.HttpHeaders;
+import org.jboss.netty.handler.codec.http.HttpRequest;
+
+public class AbstractNettyAdvice {
+ public static void muzzleCheck(final HttpRequest httpRequest) {
+ final HttpHeaders headers = httpRequest.headers();
+ }
+}

MODIFY
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelFutureListenerInstrumentation.java
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/ChannelFutureListenerInstrumentation.java
@@ -39,11 +39,7 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default {
@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
// Optimization for expensive typeMatcher.
- return hasClassesNamed(
- "org.jboss.netty.channel.ChannelFutureListener",
- // 3.10: NoSuchMethodError: org.jboss.netty.handler.codec.http.HttpRequest.setHeader
- "org.jboss.netty.channel.StaticChannelPipeline" // Not in 3.10
- );
+ return hasClassesNamed("org.jboss.netty.channel.ChannelFutureListener");
}
@Override
@@ -54,6 +50,7 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
+ packageName + ".AbstractNettyAdvice",
packageName + ".ChannelTraceContext",
packageName + ".ChannelTraceContext$Factory",
packageName + ".server.NettyHttpServerDecorator",
@@ -76,7 +73,7 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default {
"org.jboss.netty.channel.Channel", ChannelTraceContext.class.getName());
}
- public static class OperationCompleteAdvice {
+ public static class OperationCompleteAdvice extends AbstractNettyAdvice {
@Advice.OnMethodEnter
public static TraceScope activateScope(@Advice.Argument(0) final ChannelFuture future) {
/*

MODIFY
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelInstrumentation.java
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelInstrumentation.java
@@ -32,13 +32,7 @@ public class NettyChannelInstrumentation extends Instrumenter.Default {
@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
// Optimization for expensive typeMatcher.
- return hasClassesNamed(
- "org.jboss.netty.channel.Channel",
- // 3.7: cannot find symbol method headers() for type HttpRequest
- "org.jboss.netty.handler.codec.http.DefaultHttpHeaders", // Not in 3.7
- // 3.10: NoSuchMethodError: org.jboss.netty.handler.codec.http.HttpRequest.setHeader
- "org.jboss.netty.channel.StaticChannelPipeline" // Not in 3.10
- );
+ return hasClassesNamed("org.jboss.netty.channel.Channel");
}
@Override
@@ -49,7 +43,9 @@ public class NettyChannelInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
- packageName + ".ChannelTraceContext", packageName + ".ChannelTraceContext$Factory"
+ packageName + ".AbstractNettyAdvice",
+ packageName + ".ChannelTraceContext",
+ packageName + ".ChannelTraceContext$Factory"
};
}
@@ -70,7 +66,7 @@ public class NettyChannelInstrumentation extends Instrumenter.Default {
"org.jboss.netty.channel.Channel", ChannelTraceContext.class.getName());
}
- public static class ChannelConnectAdvice {
+ public static class ChannelConnectAdvice extends AbstractNettyAdvice {
@Advice.OnMethodEnter
public static void addConnectContinuation(@Advice.This final Channel channel) {
final TraceScope scope = activeScope();

MODIFY
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelPipelineInstrumentation.java
dd-java-agent/instrumentation/netty-3.8/src/main/java/datadog/trace/instrumentation/netty38/NettyChannelPipelineInstrumentation.java
@@ -48,11 +48,7 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default {
@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
// Optimization for expensive typeMatcher.
- return hasClassesNamed(
- "org.jboss.netty.channel.ChannelPipeline",
- // 3.10: NoSuchMethodError: org.jboss.netty.handler.codec.http.HttpRequest.setHeader
- "org.jboss.netty.channel.StaticChannelPipeline" // Not in 3.10
- );
+ return hasClassesNamed("org.jboss.netty.channel.ChannelPipeline");
}
@Override
@@ -63,6 +59,7 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
+ packageName + ".AbstractNettyAdvice",
packageName + ".ChannelTraceContext",
packageName + ".ChannelTraceContext$Factory",
NettyChannelPipelineInstrumentation.class.getName() + "$ChannelPipelineAdviceUtil",
@@ -150,7 +147,7 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default {
}
}
- public static class ChannelPipelineAdd2ArgsAdvice {
+ public static class ChannelPipelineAdd2ArgsAdvice extends AbstractNettyAdvice {
@Advice.OnMethodEnter
public static int checkDepth(
@Advice.This final ChannelPipeline pipeline,
@@ -180,7 +177,7 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default {
}
}
- public static class ChannelPipelineAdd3ArgsAdvice {
+ public static class ChannelPipelineAdd3ArgsAdvice extends AbstractNettyAdvice {
@Advice.OnMethodEnter
public static int checkDepth(
@Advice.This final ChannelPipeline pipeline,

MODIFY
dd-java-agent/instrumentation/netty-3.8/src/test/groovy/Netty38ClientTest.groovy
dd-java-agent/instrumentation/netty-3.8/src/test/groovy/Netty38ClientTest.groovy
@@ -18,7 +18,10 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
class Netty38ClientTest extends HttpClientTest {
@Shared
- def clientConfig = new AsyncHttpClientConfig.Builder().setRequestTimeoutInMs(TimeUnit.SECONDS.toMillis(10).toInteger()).build()
+ def clientConfig = new AsyncHttpClientConfig.Builder()
+ .setRequestTimeoutInMs(TimeUnit.SECONDS.toMillis(10).toInteger())
+ .build()
+
@Shared
@AutoCleanup
AsyncHttpClient asyncHttpClient = new AsyncHttpClient(clientConfig)

Update to support 3.10 also and make muzzle check less brittle