Skip to content

Commit f820789

Browse files
tylerbensontylerbenson
authored
Merge pull request #219 from DataDog/tyler/move-tests
Move servlet tests to individual modules. Fix servlet 2 instrumentation.
2 parents 1a855c4 + 3e57a7a commit f820789

File tree

13 files changed

+355
-39
lines changed

13 files changed

+355
-39
lines changed

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ dependencies {
2424
testCompile project(':dd-trace-ot')
2525

2626
testCompile deps.opentracingMock
27+
testCompile deps.testLogging
2728

2829
testCompile(project(':dd-java-agent:testing')) {
2930
// Otherwise this can bring in classes that aren't "shadowed"
@@ -36,16 +37,9 @@ dependencies {
3637
// run embeded mongodb for integration testing
3738
testCompile group: 'de.flapdoodle.embed', name: 'de.flapdoodle.embed.mongo', version: '1.50.5'
3839

39-
testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.2.0.v20160908'
40-
testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.2.0.v20160908'
41-
testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '8.0.41'
42-
testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '8.0.41'
4340
testCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3'
4441
testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
4542

46-
testCompile group: 'org.apache.logging.log4j', name: 'log4j-api', version: '2.8.2'
47-
testCompile group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.8.2'
48-
4943
// JDBC tests:
5044
testCompile group: 'com.h2database', name: 'h2', version: '1.4.196'
5145
testCompile group: 'org.hsqldb', name: 'hsqldb', version: '2.3.+'

‎dd-java-agent-ittests/src/test/resources/log4j2.properties‎

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

‎dd-java-agent/instrumentation/servlet-2/servlet-2.gradle‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,11 @@ dependencies {
2424
compile deps.bytebuddy
2525
compile deps.opentracing
2626
compile deps.autoservice
27+
28+
testCompile project(':dd-java-agent:testing')
29+
testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '7.0.0.v20091005'
30+
testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '7.0.0.v20091005'
31+
32+
testCompile project(':dd-java-agent:instrumentation:okhttp-3') // used in the tests
33+
testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
2734
}

‎dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import io.opentracing.Span;
2020
import io.opentracing.SpanContext;
2121
import io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter;
22-
import io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator;
2322
import io.opentracing.propagation.Format;
2423
import io.opentracing.tag.Tags;
2524
import io.opentracing.util.GlobalTracer;
@@ -52,10 +51,7 @@ public AgentBuilder apply(final AgentBuilder agentBuilder) {
5251
new HelperInjector(
5352
"io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter",
5453
"io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator",
55-
"io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator",
56-
"io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator$1",
57-
"io.opentracing.contrib.web.servlet.filter.TracingFilter",
58-
"io.opentracing.contrib.web.servlet.filter.TracingFilter$1"))
54+
"datadog.trace.instrumentation.servlet2.ServletFilterSpanDecorator"))
5955
.transform(
6056
DDAdvice.create()
6157
.advice(

‎dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import io.opentracing.Span;
2020
import io.opentracing.SpanContext;
2121
import io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter;
22-
import io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator;
2322
import io.opentracing.propagation.Format;
2423
import io.opentracing.tag.Tags;
2524
import io.opentracing.util.GlobalTracer;
@@ -52,12 +51,9 @@ public AgentBuilder apply(final AgentBuilder agentBuilder) {
5251
new HelperInjector(
5352
"io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter",
5453
"io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator",
55-
"io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator",
56-
"io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator$1",
57-
"io.opentracing.contrib.web.servlet.filter.TracingFilter",
58-
"io.opentracing.contrib.web.servlet.filter.TracingFilter$1"))
54+
"datadog.trace.instrumentation.servlet2.ServletFilterSpanDecorator"))
5955
.transform(
60-
DDAdvice.create()
56+
DDAdvice.create(false) // Can't use the error handler for pre 1.5 classes...
6157
.advice(
6258
named("service")
6359
.and(takesArgument(0, named("javax.servlet.http.HttpServletRequest")))

‎dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/ServletFilterSpanDecorator.java‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package datadog.trace.instrumentation.servlet2;
2+
3+
import io.opentracing.Span;
4+
import io.opentracing.tag.Tags;
5+
import java.io.PrintWriter;
6+
import java.io.StringWriter;
7+
import java.util.HashMap;
8+
import java.util.Map;
9+
import javax.servlet.FilterChain;
10+
import javax.servlet.ServletRequest;
11+
import javax.servlet.ServletResponse;
12+
import javax.servlet.http.HttpServletRequest;
13+
import javax.servlet.http.HttpServletResponse;
14+
15+
/**
16+
* SpanDecorator to decorate span at different stages in filter processing (before
17+
* filterChain.doFilter(), after and if exception is thrown).
18+
*
19+
* <p>Taken from
20+
* https://raw.githubusercontent.com/opentracing-contrib/java-web-servlet-filter/v0.1.0/opentracing-web-servlet-filter/src/main/java/io/opentracing/contrib/web/servlet/filter/ServletFilterSpanDecorator.java
21+
* and removed async and status code stuff to be Servlet 2.x compatible.
22+
*
23+
* @author Pavol Loffay
24+
*/
25+
public interface ServletFilterSpanDecorator {
26+
27+
/**
28+
* Decorate span before {@link javax.servlet.Filter#doFilter(ServletRequest, ServletResponse,
29+
* FilterChain)} is called. This is called right after span in created. Span is already present in
30+
* request attributes with name {@link TracingFilter#SERVER_SPAN_CONTEXT}.
31+
*
32+
* @param httpServletRequest request
33+
* @param span span to decorate
34+
*/
35+
void onRequest(HttpServletRequest httpServletRequest, Span span);
36+
37+
/**
38+
* Decorate span after {@link javax.servlet.Filter#doFilter(ServletRequest, ServletResponse,
39+
* FilterChain)}.
40+
*
41+
* @param httpServletRequest request
42+
* @param httpServletResponse response
43+
* @param span span to decorate
44+
*/
45+
void onResponse(
46+
HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Span span);
47+
48+
/**
49+
* Decorate span when an exception is thrown during processing in {@link
50+
* javax.servlet.Filter#doFilter(ServletRequest, ServletResponse, FilterChain)}.
51+
*
52+
* @param httpServletRequest request
53+
* @param exception exception
54+
* @param span span to decorate
55+
*/
56+
void onError(
57+
HttpServletRequest httpServletRequest,
58+
HttpServletResponse httpServletResponse,
59+
Throwable exception,
60+
Span span);
61+
62+
/**
63+
* Decorate span on asynchronous request timeout.
64+
*
65+
* @param httpServletRequest request
66+
* @param httpServletResponse response
67+
* @param timeout timeout
68+
* @param span span to decorate
69+
*/
70+
void onTimeout(
71+
HttpServletRequest httpServletRequest,
72+
HttpServletResponse httpServletResponse,
73+
long timeout,
74+
Span span);
75+
76+
/**
77+
* Adds standard tags to span. {@link Tags#HTTP_URL}, {@link Tags#HTTP_STATUS}, {@link
78+
* Tags#HTTP_METHOD} and {@link Tags#COMPONENT}. If an exception during {@link
79+
* javax.servlet.Filter#doFilter(ServletRequest, ServletResponse, FilterChain)} is thrown tag
80+
* {@link Tags#ERROR} is added and {@link Tags#HTTP_STATUS} not because at this point it is not
81+
* known.
82+
*/
83+
ServletFilterSpanDecorator STANDARD_TAGS =
84+
new ServletFilterSpanDecorator() {
85+
@Override
86+
public void onRequest(final HttpServletRequest httpServletRequest, final Span span) {
87+
Tags.COMPONENT.set(span, "java-web-servlet");
88+
89+
Tags.HTTP_METHOD.set(span, httpServletRequest.getMethod());
90+
//without query params
91+
Tags.HTTP_URL.set(span, httpServletRequest.getRequestURL().toString());
92+
}
93+
94+
@Override
95+
public void onResponse(
96+
final HttpServletRequest httpServletRequest,
97+
final HttpServletResponse httpServletResponse,
98+
final Span span) {}
99+
100+
@Override
101+
public void onError(
102+
final HttpServletRequest httpServletRequest,
103+
final HttpServletResponse httpServletResponse,
104+
final Throwable exception,
105+
final Span span) {
106+
Tags.ERROR.set(span, Boolean.TRUE);
107+
span.log(logsForException(exception));
108+
}
109+
110+
@Override
111+
public void onTimeout(
112+
final HttpServletRequest httpServletRequest,
113+
final HttpServletResponse httpServletResponse,
114+
final long timeout,
115+
final Span span) {
116+
Tags.ERROR.set(span, Boolean.TRUE);
117+
118+
final Map<String, Object> timeoutLogs = new HashMap<>();
119+
timeoutLogs.put("event", Tags.ERROR.getKey());
120+
timeoutLogs.put("message", "timeout");
121+
timeoutLogs.put("timeout", timeout);
122+
}
123+
124+
private Map<String, String> logsForException(final Throwable throwable) {
125+
final Map<String, String> errorLog = new HashMap<>(3);
126+
errorLog.put("event", Tags.ERROR.getKey());
127+
128+
final String message =
129+
throwable.getCause() != null
130+
? throwable.getCause().getMessage()
131+
: throwable.getMessage();
132+
if (message != null) {
133+
errorLog.put("message", message);
134+
}
135+
final StringWriter sw = new StringWriter();
136+
throwable.printStackTrace(new PrintWriter(sw));
137+
errorLog.put("stack", sw.toString());
138+
139+
return errorLog;
140+
}
141+
};
142+
}

‎dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServletTest.groovy‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
import datadog.opentracing.DDSpan
2+
import datadog.opentracing.DDTracer
3+
import datadog.trace.agent.test.AgentTestRunner
4+
import datadog.trace.api.DDSpanTypes
5+
import datadog.trace.common.writer.ListWriter
6+
import io.opentracing.util.GlobalTracer
7+
import okhttp3.Interceptor
8+
import okhttp3.OkHttpClient
9+
import okhttp3.Request
10+
import okhttp3.Response
11+
import org.eclipse.jetty.server.Server
12+
import org.eclipse.jetty.servlet.ServletContextHandler
13+
import spock.lang.Unroll
14+
15+
import java.lang.reflect.Field
16+
import java.util.concurrent.CountDownLatch
17+
import java.util.concurrent.TimeUnit
18+
19+
class JettyServletTest extends AgentTestRunner {
20+
21+
static final int PORT = randomOpenPort()
22+
23+
// Jetty needs this to ensure consistent ordering for async.
24+
static CountDownLatch latch
25+
OkHttpClient client = new OkHttpClient.Builder()
26+
.addNetworkInterceptor(new Interceptor() {
27+
@Override
28+
Response intercept(Interceptor.Chain chain) throws IOException {
29+
def response = chain.proceed(chain.request())
30+
JettyServletTest.latch.await(10, TimeUnit.SECONDS) // don't block forever or test never fails.
31+
return response
32+
}
33+
})
34+
// Uncomment when debugging:
35+
.connectTimeout(1, TimeUnit.HOURS)
36+
.writeTimeout(1, TimeUnit.HOURS)
37+
.readTimeout(1, TimeUnit.HOURS)
38+
.build()
39+
40+
private Server jettyServer
41+
private ServletContextHandler servletContext
42+
43+
ListWriter writer = new ListWriter() {
44+
@Override
45+
void write(final List<DDSpan> trace) {
46+
add(trace)
47+
JettyServletTest.latch.countDown()
48+
}
49+
}
50+
DDTracer tracer = new DDTracer(writer)
51+
52+
def setup() {
53+
jettyServer = new Server(PORT)
54+
servletContext = new ServletContextHandler()
55+
56+
servletContext.addServlet(TestServlet.Sync, "/sync")
57+
58+
jettyServer.setHandler(servletContext)
59+
jettyServer.start()
60+
61+
System.out.println(
62+
"Jetty server: http://localhost:" + PORT + "/")
63+
64+
try {
65+
GlobalTracer.register(tracer)
66+
} catch (final Exception e) {
67+
// Force it anyway using reflection
68+
final Field field = GlobalTracer.getDeclaredField("tracer")
69+
field.setAccessible(true)
70+
field.set(null, tracer)
71+
}
72+
writer.start()
73+
assert GlobalTracer.isRegistered()
74+
}
75+
76+
def cleanup() {
77+
jettyServer.stop()
78+
jettyServer.destroy()
79+
}
80+
81+
@Unroll
82+
def "test #path servlet call"() {
83+
setup:
84+
latch = new CountDownLatch(1)
85+
def request = new Request.Builder()
86+
.url("http://localhost:$PORT/$path")
87+
.get()
88+
.build()
89+
def response = client.newCall(request).execute()
90+
91+
expect:
92+
response.body().string().trim() == expectedResponse
93+
writer.size() == 2 // second (parent) trace is the okhttp call above...
94+
def trace = writer.firstTrace()
95+
trace.size() == 1
96+
def span = trace[0]
97+
98+
span.context().operationName == "servlet.request"
99+
span.context().spanType == DDSpanTypes.WEB_SERVLET
100+
!span.context().getErrorFlag()
101+
span.context().parentId != 0 // parent should be the okhttp call.
102+
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
103+
span.context().tags["http.method"] == "GET"
104+
span.context().tags["span.kind"] == "server"
105+
span.context().tags["component"] == "java-web-servlet"
106+
span.context().tags["http.status_code"] == null // sadly servlet 2.x doesn't expose it generically.
107+
span.context().tags["thread.name"] != null
108+
span.context().tags["thread.id"] != null
109+
span.context().tags.size() == 7
110+
111+
where:
112+
path | expectedResponse
113+
"sync" | "Hello Sync"
114+
}
115+
116+
@Unroll
117+
def "test #path error servlet call"() {
118+
setup:
119+
def request = new Request.Builder()
120+
.url("http://localhost:$PORT/$path?error=true")
121+
.get()
122+
.build()
123+
def response = client.newCall(request).execute()
124+
125+
expect:
126+
response.body().string().trim() != expectedResponse
127+
writer.size() == 2 // second (parent) trace is the okhttp call above...
128+
def trace = writer.firstTrace()
129+
trace.size() == 1
130+
def span = trace[0]
131+
132+
span.context().operationName == "servlet.request"
133+
span.context().spanType == DDSpanTypes.WEB_SERVLET
134+
span.context().getErrorFlag()
135+
span.context().parentId != 0 // parent should be the okhttp call.
136+
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
137+
span.context().tags["http.method"] == "GET"
138+
span.context().tags["span.kind"] == "server"
139+
span.context().tags["component"] == "java-web-servlet"
140+
span.context().tags["http.status_code"] == null // sadly servlet 2.x doesn't expose it generically.
141+
span.context().tags["thread.name"] != null
142+
span.context().tags["thread.id"] != null
143+
span.context().tags["error"] == true
144+
span.context().tags["error.msg"] == "some $path error"
145+
span.context().tags["error.type"] == RuntimeException.getName()
146+
span.context().tags["error.stack"] != null
147+
span.context().tags.size() == 11
148+
149+
where:
150+
path | expectedResponse
151+
"sync" | "Hello Sync"
152+
}
153+
154+
private static int randomOpenPort() {
155+
new ServerSocket(0).withCloseable {
156+
it.setReuseAddress(true)
157+
return it.getLocalPort()
158+
}
159+
}
160+
}

0 commit comments

Comments
 (0)