Skip to content

Commit 6fa3030

Browse files
author
Andrew Kent
committed
ApacheHttpClient bytebuddy instrumentation and tests.
1 parent f48b793 commit 6fa3030

File tree

13 files changed

+402
-88
lines changed

13 files changed

+402
-88
lines changed

‎dd-java-agent-ittests/dd-java-agent-ittests.gradle‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ dependencies {
2121
testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.4.1.v20170120'
2222
testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '8.0.41'
2323
testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '8.0.41'
24-
testCompile(group: 'com.amazonaws', name: 'aws-java-sdk', version: '1.11.119') {
25-
exclude(module: 'httpclient')
26-
}
24+
// note: aws transitively pulls in apache-httpclient, which we also test against:
25+
// testCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.3'
26+
testCompile(group: 'com.amazonaws', name: 'aws-java-sdk', version: '1.11.119')
2727
testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
28-
testCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.3'
2928
testCompile(group: 'com.datastax.cassandra', name: 'cassandra-driver-core', version: '3.2.0')
3029
testCompile(group: 'org.cassandraunit', name: 'cassandra-unit', version: '3.1.3.2') {
3130
exclude(module: 'netty-handler')

‎dd-java-agent-ittests/src/test/java/com/datadoghq/agent/integration/ApacheHTTPClientTest.java‎

Lines changed: 122 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,135 @@
22

33
import static org.assertj.core.api.Assertions.assertThat;
44

5+
import com.datadoghq.trace.DDBaseSpan;
6+
import com.datadoghq.trace.DDTracer;
7+
import com.datadoghq.trace.writer.ListWriter;
8+
import io.opentracing.tag.Tags;
9+
import java.net.URI;
10+
import java.util.List;
11+
import org.apache.http.HttpResponse;
12+
import org.apache.http.client.HttpClient;
13+
import org.apache.http.client.methods.HttpGet;
514
import org.apache.http.impl.client.HttpClientBuilder;
15+
import org.junit.AfterClass;
16+
import org.junit.Assert;
17+
import org.junit.Before;
18+
import org.junit.BeforeClass;
619
import org.junit.Test;
720

821
public class ApacheHTTPClientTest {
9-
static {
10-
try {
11-
// Since the HttpClientBuilder initializer doesn't work, invoke manually.
12-
Class.forName("com.datadoghq.agent.InstrumentationRulesManager")
13-
.getMethod("registerClassLoad", Object.class)
14-
.invoke(null, Thread.currentThread().getContextClassLoader());
15-
} catch (Exception e) {
16-
System.err.println("clinit error: " + e.getMessage());
17-
}
22+
private static final ListWriter writer = new ListWriter();
23+
private static final DDTracer tracer = new DDTracer(writer);
24+
25+
@BeforeClass
26+
public static void setup() throws Exception {
27+
TestUtils.registerOrReplaceGlobalTracer(tracer);
28+
TestHttpServer.startServer();
29+
}
30+
31+
@AfterClass
32+
public static void stopServer() throws Exception {
33+
TestHttpServer.stopServer();
34+
}
35+
36+
@Before
37+
public void beforeEachTest() {
38+
writer.clear();
1839
}
1940

2041
@Test
21-
public void test() throws Exception {
42+
public void propagatedTrace() throws Exception {
2243
final HttpClientBuilder builder = HttpClientBuilder.create();
23-
assertThat(builder.getClass().getSimpleName()).isEqualTo("TracingHttpClientBuilder");
44+
45+
final HttpClient client = builder.build();
46+
TestUtils.runUnderTrace(
47+
"someTrace",
48+
new Runnable() {
49+
@Override
50+
public void run() {
51+
try {
52+
HttpResponse response =
53+
client.execute(new HttpGet(new URI("http://localhost:" + TestHttpServer.PORT)));
54+
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
55+
} catch (Exception e) {
56+
e.printStackTrace();
57+
Assert.fail("Error running client");
58+
}
59+
}
60+
});
61+
// one trace on the server, one trace on the client
62+
assertThat(writer.size()).isEqualTo(2);
63+
final List<DDBaseSpan<?>> serverTrace = writer.get(0);
64+
assertThat(serverTrace.size()).isEqualTo(1);
65+
66+
final List<DDBaseSpan<?>> clientTrace = writer.get(1);
67+
assertThat(clientTrace.size()).isEqualTo(3);
68+
assertThat(clientTrace.get(0).getOperationName()).isEqualTo("someTrace");
69+
// our instrumentation makes 2 spans for apache-httpclient
70+
final DDBaseSpan<?> localSpan = clientTrace.get(1);
71+
assertThat(localSpan.getTags().get(Tags.COMPONENT.getKey())).isEqualTo("apache-httpclient");
72+
assertThat(localSpan.getOperationName()).isEqualTo("GET");
73+
74+
final DDBaseSpan<?> clientSpan = clientTrace.get(2);
75+
assertThat(clientSpan.getOperationName()).isEqualTo("GET");
76+
assertThat(clientSpan.getTags().get(Tags.HTTP_METHOD.getKey())).isEqualTo("GET");
77+
assertThat(clientSpan.getTags().get(Tags.HTTP_STATUS.getKey())).isEqualTo(200);
78+
assertThat(clientSpan.getTags().get(Tags.HTTP_URL.getKey())).isEqualTo("http://localhost:8089");
79+
assertThat(clientSpan.getTags().get(Tags.PEER_HOSTNAME.getKey())).isEqualTo("localhost");
80+
assertThat(clientSpan.getTags().get(Tags.PEER_PORT.getKey())).isEqualTo(8089);
81+
assertThat(clientSpan.getTags().get(Tags.SPAN_KIND.getKey())).isEqualTo(Tags.SPAN_KIND_CLIENT);
82+
83+
// client trace propagates to server
84+
assertThat(clientSpan.getTraceId()).isEqualTo(serverTrace.get(0).getTraceId());
85+
// server span is parented under http client
86+
assertThat(clientSpan.getSpanId()).isEqualTo(serverTrace.get(0).getParentId());
87+
}
88+
89+
@Test
90+
public void unPropagatedTrace() throws Exception {
91+
final HttpClientBuilder builder = HttpClientBuilder.create();
92+
93+
final HttpClient client = builder.build();
94+
TestUtils.runUnderTrace(
95+
"someTrace",
96+
new Runnable() {
97+
@Override
98+
public void run() {
99+
try {
100+
HttpResponse response =
101+
client.execute(
102+
new HttpGet(
103+
new URI(
104+
"http://localhost:"
105+
+ TestHttpServer.PORT
106+
+ "?"
107+
+ TestHttpServer.IS_DD_SERVER
108+
+ "=false")));
109+
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
110+
} catch (Exception e) {
111+
e.printStackTrace();
112+
Assert.fail("Error running client");
113+
}
114+
}
115+
});
116+
// only one trace (client).
117+
assertThat(writer.size()).isEqualTo(1);
118+
final List<DDBaseSpan<?>> clientTrace = writer.get(0);
119+
assertThat(clientTrace.size()).isEqualTo(3);
120+
assertThat(clientTrace.get(0).getOperationName()).isEqualTo("someTrace");
121+
// our instrumentation makes 2 spans for apache-httpclient
122+
final DDBaseSpan<?> localSpan = clientTrace.get(1);
123+
assertThat(localSpan.getTags().get(Tags.COMPONENT.getKey())).isEqualTo("apache-httpclient");
124+
assertThat(localSpan.getOperationName()).isEqualTo("GET");
125+
126+
final DDBaseSpan<?> clientSpan = clientTrace.get(2);
127+
assertThat(clientSpan.getOperationName()).isEqualTo("GET");
128+
assertThat(clientSpan.getTags().get(Tags.HTTP_METHOD.getKey())).isEqualTo("GET");
129+
assertThat(clientSpan.getTags().get(Tags.HTTP_STATUS.getKey())).isEqualTo(200);
130+
assertThat(clientSpan.getTags().get(Tags.HTTP_URL.getKey()))
131+
.isEqualTo("http://localhost:8089?is-dd-server=false");
132+
assertThat(clientSpan.getTags().get(Tags.PEER_HOSTNAME.getKey())).isEqualTo("localhost");
133+
assertThat(clientSpan.getTags().get(Tags.PEER_PORT.getKey())).isEqualTo(8089);
134+
assertThat(clientSpan.getTags().get(Tags.SPAN_KIND.getKey())).isEqualTo(Tags.SPAN_KIND_CLIENT);
24135
}
25136
}

‎dd-java-agent-ittests/src/test/java/com/datadoghq/agent/integration/TestUtils.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.datadoghq.agent.integration;
22

3+
import io.opentracing.ActiveSpan;
34
import io.opentracing.Tracer;
45
import io.opentracing.util.GlobalTracer;
56
import java.lang.reflect.Field;
@@ -15,4 +16,13 @@ public static void registerOrReplaceGlobalTracer(Tracer tracer) throws Exception
1516
field.set(null, tracer);
1617
}
1718
}
19+
20+
public static void runUnderTrace(final String rootOperationName, Runnable r) {
21+
ActiveSpan rootSpan = GlobalTracer.get().buildSpan(rootOperationName).startActive();
22+
try {
23+
r.run();
24+
} finally {
25+
rootSpan.deactivate();
26+
}
27+
}
1828
}

‎dd-java-agent/dd-java-agent.gradle‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ dependencies {
2121
compile project(':dd-java-agent:tooling')
2222
compile project(':dd-trace-annotations')
2323

24+
compile(project(':dd-java-agent:integrations:apache-httpclient-4.3')) {
25+
transitive = false
26+
}
2427
compile(project(':dd-java-agent:integrations:aws-sdk')) {
2528
transitive = false
2629
}

‎dd-java-agent/integrations/apache-httpclient-4.3/apache-httpclient-4.3.gradle‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
apply plugin: 'version-scan'
2+
3+
versionScan {
4+
group = "org.apache.httpcomponents"
5+
module = "httpclient"
6+
versions = "[4.3,)"
7+
legacyGroup = "commons-httpclient"
8+
legacyModule = "commons-httpclient"
9+
verifyPresent = [
10+
// The commented out classes are required for the instrumentation (this is checked by our bytebuddy rules).
11+
// Once the verifier can report on the failed versions these classes can be commented out and the pass range can be widened.
12+
13+
// "org.apache.http.HttpException" : null,
14+
// "org.apache.http.HttpRequest" : null,
15+
// "org.apache.http.client.RedirectStrategy" : null,
16+
"org.apache.http.client.methods.CloseableHttpResponse" : null,
17+
"org.apache.http.client.methods.HttpExecutionAware" : null,
18+
"org.apache.http.client.methods.HttpRequestWrapper" : null,
19+
"org.apache.http.client.protocol.HttpClientContext" : null,
20+
// "org.apache.http.conn.routing.HttpRoute" : null,
21+
"org.apache.http.impl.execchain.ClientExecChain": null
22+
]
23+
}
24+
25+
apply from: "${rootDir}/gradle/java.gradle"
26+
27+
dependencies {
28+
compile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3'
29+
30+
compile project(':dd-trace')
31+
compile project(':dd-java-agent:integrations:helpers')
32+
compile project(':dd-java-agent:tooling')
33+
34+
compile deps.bytebuddy
35+
compile deps.opentracing
36+
}

‎dd-java-agent/integrations/apache-httpclient-4.3/src/main/java/dd/inst/apachehttpclient/ApacheHttpClientInstrumentation.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package dd.inst.apachehttpclient;
2+
3+
import static dd.trace.ClassLoaderMatcher.classLoaderHasClasses;
4+
import static dd.trace.ExceptionHandlers.defaultExceptionHandler;
5+
import static net.bytebuddy.matcher.ElementMatchers.*;
6+
7+
import com.datadoghq.agent.integration.DDTracingClientExec;
8+
import com.google.auto.service.AutoService;
9+
import dd.trace.Instrumenter;
10+
import io.opentracing.util.GlobalTracer;
11+
import net.bytebuddy.agent.builder.AgentBuilder;
12+
import net.bytebuddy.asm.Advice;
13+
import org.apache.http.impl.client.DefaultRedirectStrategy;
14+
import org.apache.http.impl.execchain.ClientExecChain;
15+
16+
@AutoService(Instrumenter.class)
17+
public class ApacheHttpClientInstrumentation implements Instrumenter {
18+
19+
@Override
20+
public AgentBuilder instrument(AgentBuilder agentBuilder) {
21+
return agentBuilder
22+
.type(
23+
named("org.apache.http.impl.client.HttpClientBuilder"),
24+
classLoaderHasClasses(
25+
"org.apache.http.HttpException",
26+
"org.apache.http.HttpRequest",
27+
"org.apache.http.client.RedirectStrategy",
28+
"org.apache.http.client.methods.CloseableHttpResponse",
29+
"org.apache.http.client.methods.HttpExecutionAware",
30+
"org.apache.http.client.methods.HttpRequestWrapper",
31+
"org.apache.http.client.protocol.HttpClientContext",
32+
"org.apache.http.conn.routing.HttpRoute",
33+
"org.apache.http.impl.execchain.ClientExecChain"))
34+
.transform(
35+
new AgentBuilder.Transformer.ForAdvice()
36+
.advice(
37+
isMethod().and(named("decorateProtocolExec")),
38+
ApacheHttpClientAdvice.class.getName())
39+
.withExceptionHandler(defaultExceptionHandler()))
40+
.asDecorator();
41+
}
42+
43+
public static class ApacheHttpClientAdvice {
44+
/** Strategy: add our tracing exec to the apache exec chain. */
45+
@Advice.OnMethodExit(suppress = Throwable.class)
46+
public static void addTracingExec(@Advice.Return(readOnly = false) ClientExecChain execChain) {
47+
execChain =
48+
new DDTracingClientExec(
49+
execChain, DefaultRedirectStrategy.INSTANCE, false, GlobalTracer.get());
50+
}
51+
}
52+
}

‎dd-java-agent/integrations/apache-httpclient/apache-httpclient.gradle‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎dd-java-agent/integrations/helpers/helpers.gradle‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,13 @@ dependencies {
1717
compile group: 'io.opentracing.contrib', name: 'opentracing-okhttp3', version: '0.0.5'
1818
compile group: 'io.opentracing.contrib', name: 'opentracing-aws-sdk', version: '0.0.2'
1919
compile group: 'io.opentracing.contrib', name: 'opentracing-cassandra-driver', version: '0.0.2'
20-
compile group: 'io.opentracing.contrib', name: 'opentracing-apache-httpclient', version: '0.0.2'
2120

2221
compileOnly group: 'org.mongodb', name: 'mongo-java-driver', version: '3.4.2'
2322
compileOnly group: 'org.mongodb', name: 'mongodb-driver-async', version: '3.4.2'
2423
compileOnly group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
2524
compileOnly group: 'javax.jms', name: 'javax.jms-api', version: '2.0.1'
2625
compileOnly group: 'com.datastax.cassandra', name: 'cassandra-driver-core', version: '3.2.0'
27-
compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.3'
26+
compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3'
2827
}
2928

3029
jar {

‎dd-java-agent/integrations/helpers/src/main/java/com/datadoghq/agent/integration/ApacheHTTPClientHelper.java‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

0 commit comments

Comments
 (0)