Merge pull request #312 from DataDog/tyler/servlet-tests

Give more distinct names and migrate test style
This commit is contained in:
Tyler Benson 2018-05-08 11:54:44 +10:00 committed by GitHub
commit 5a4132713d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 270 additions and 198 deletions

View File

@ -17,18 +17,20 @@ import java.lang.reflect.Field
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
class JettyServletTest extends AgentTestRunner {
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class JettyServlet2Test extends AgentTestRunner {
static final int PORT = TestUtils.randomOpenPort()
// Jetty needs this to ensure consistent ordering for async.
static CountDownLatch latch
CountDownLatch latch = new CountDownLatch(1)
OkHttpClient client = new OkHttpClient.Builder()
.addNetworkInterceptor(new Interceptor() {
@Override
Response intercept(Interceptor.Chain chain) throws IOException {
def response = chain.proceed(chain.request())
JettyServletTest.latch.await(10, TimeUnit.SECONDS) // don't block forever or test never fails.
JettyServlet2Test.this.latch.await(10, TimeUnit.SECONDS) // don't block forever or test never fails.
return response
}
})
@ -45,7 +47,7 @@ class JettyServletTest extends AgentTestRunner {
@Override
void write(final List<DDSpan> trace) {
add(trace)
JettyServletTest.latch.countDown()
JettyServlet2Test.this.latch.countDown()
}
}
DDTracer tracer = new DDTracer(writer)
@ -54,7 +56,7 @@ class JettyServletTest extends AgentTestRunner {
jettyServer = new Server(PORT)
servletContext = new ServletContextHandler()
servletContext.addServlet(TestServlet.Sync, "/sync")
servletContext.addServlet(TestServlet2.Sync, "/sync")
jettyServer.setHandler(servletContext)
jettyServer.start()
@ -82,7 +84,6 @@ class JettyServletTest extends AgentTestRunner {
@Unroll
def "test #path servlet call"() {
setup:
latch = new CountDownLatch(1)
def request = new Request.Builder()
.url("http://localhost:$PORT/$path")
.get()
@ -91,26 +92,27 @@ class JettyServletTest extends AgentTestRunner {
expect:
response.body().string().trim() == expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
!span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == null // sadly servlet 2.x doesn't expose it generically.
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags.size() == 7
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
defaultTags()
}
}
}
}
where:
path | expectedResponse
@ -128,29 +130,67 @@ class JettyServletTest extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == null // sadly servlet 2.x doesn't expose it generically.
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags["error"] == true
span.context().tags["error.msg"] == "some $path error"
span.context().tags["error.type"] == RuntimeException.getName()
span.context().tags["error.stack"] != null
span.context().tags.size() == 11
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
errorTags(RuntimeException, "some $path error")
defaultTags()
}
}
}
}
where:
path | expectedResponse
"sync" | "Hello Sync"
}
@Unroll
def "test #path non-throwing-error servlet call"() {
// This doesn't actually detect the error because we can't get the status code via the old servlet API.
setup:
def request = new Request.Builder()
.url("http://localhost:$PORT/$path?non-throwing-error=true")
.get()
.build()
def response = client.newCall(request).execute()
expect:
response.body().string().trim() != expectedResponse
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
defaultTags()
}
}
}
}
where:
path | expectedResponse

View File

@ -3,7 +3,7 @@ import groovy.servlet.AbstractHttpServlet
import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse
class TestServlet {
class TestServlet2 {
static class Sync extends AbstractHttpServlet {
@Override
@ -11,6 +11,10 @@ class TestServlet {
if (req.getParameter("error") != null) {
throw new RuntimeException("some sync error")
}
if (req.getParameter("non-throwing-error") != null) {
resp.sendError(500, "some sync error")
return
}
resp.writer.print("Hello Sync")
}
}

View File

@ -17,18 +17,21 @@ import java.lang.reflect.Field
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
class JettyServletTest extends AgentTestRunner {
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class JettyServlet3Test extends AgentTestRunner {
static final int PORT = TestUtils.randomOpenPort()
// Jetty needs this to ensure consistent ordering for async.
static CountDownLatch latch
CountDownLatch latch = new CountDownLatch(1)
OkHttpClient client = new OkHttpClient.Builder()
.addNetworkInterceptor(new Interceptor() {
@Override
Response intercept(Interceptor.Chain chain) throws IOException {
def response = chain.proceed(chain.request())
JettyServletTest.latch.await(10, TimeUnit.SECONDS) // don't block forever or test never fails.
JettyServlet3Test.this.latch.await(10, TimeUnit.SECONDS) // don't block forever or test never fails.
return response
}
})
@ -45,7 +48,7 @@ class JettyServletTest extends AgentTestRunner {
@Override
void write(final List<DDSpan> trace) {
add(trace)
JettyServletTest.latch.countDown()
JettyServlet3Test.this.latch.countDown()
}
}
DDTracer tracer = new DDTracer(writer)
@ -54,8 +57,8 @@ class JettyServletTest extends AgentTestRunner {
jettyServer = new Server(PORT)
servletContext = new ServletContextHandler()
servletContext.addServlet(TestServlet.Sync, "/sync")
servletContext.addServlet(TestServlet.Async, "/async")
servletContext.addServlet(TestServlet3.Sync, "/sync")
servletContext.addServlet(TestServlet3.Async, "/async")
jettyServer.setHandler(servletContext)
jettyServer.start()
@ -83,7 +86,6 @@ class JettyServletTest extends AgentTestRunner {
@Unroll
def "test #path servlet call"() {
setup:
latch = new CountDownLatch(1)
def request = new Request.Builder()
.url("http://localhost:$PORT/$path")
.get()
@ -92,26 +94,28 @@ class JettyServletTest extends AgentTestRunner {
expect:
response.body().string().trim() == expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
!span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == 200
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags.size() == 8
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 200
defaultTags()
}
}
}
}
where:
path | expectedResponse
@ -130,30 +134,29 @@ class JettyServletTest extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == 500
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags["error"] == true
span.context().tags["error.msg"] == "some $path error"
span.context().tags["error.type"] == RuntimeException.getName()
span.context().tags["error.stack"] != null
span.context().tags.size() == 12
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
defaultTags()
}
}
}
}
where:
path | expectedResponse
@ -172,30 +175,29 @@ class JettyServletTest extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == 500
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags["error"] == true
span.context().tags["error.msg"] == null
span.context().tags["error.type"] == null
span.context().tags["error.stack"] == null
span.context().tags.size() == 9
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 500
"error" true
defaultTags()
}
}
}
}
where:
path | expectedResponse

View File

@ -4,7 +4,7 @@ import javax.servlet.annotation.WebServlet
import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse
class TestServlet {
class TestServlet3 {
@WebServlet
static class Sync extends AbstractHttpServlet {

View File

@ -15,7 +15,9 @@ import spock.lang.Unroll
import java.lang.reflect.Field
class TomcatServletTest extends AgentTestRunner {
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class TomcatServlet3Test extends AgentTestRunner {
static final int PORT = TestUtils.randomOpenPort()
OkHttpClient client = new OkHttpClient.Builder()
@ -52,10 +54,10 @@ class TomcatServletTest extends AgentTestRunner {
}
})
Tomcat.addServlet(appContext, "syncServlet", new TestServlet.Sync())
Tomcat.addServlet(appContext, "syncServlet", new TestServlet3.Sync())
appContext.addServletMappingDecoded("/sync", "syncServlet")
Tomcat.addServlet(appContext, "asyncServlet", new TestServlet.Async())
Tomcat.addServlet(appContext, "asyncServlet", new TestServlet3.Async())
appContext.addServletMappingDecoded("/async", "asyncServlet")
tomcatServer.start()
@ -91,26 +93,28 @@ class TomcatServletTest extends AgentTestRunner {
expect:
response.body().string().trim() == expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
!span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == 200
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags.size() == 8
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 200
defaultTags()
}
}
}
}
where:
path | expectedResponse
@ -129,30 +133,29 @@ class TomcatServletTest extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == 500
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags["error"] == true
span.context().tags["error.msg"] == "some $path error"
span.context().tags["error.type"] == RuntimeException.getName()
span.context().tags["error.stack"] != null
span.context().tags.size() == 12
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
defaultTags()
}
}
}
}
where:
path | expectedResponse
@ -171,30 +174,29 @@ class TomcatServletTest extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
writer.waitForTraces(1)
writer.size() == 1
def trace = writer.firstTrace()
trace.size() == 1
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
span.context().operationName == "servlet.request"
span.context().resourceName == "GET /$path"
span.context().spanType == DDSpanTypes.WEB_SERVLET
span.context().getErrorFlag()
span.context().parentId == 0
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
span.context().tags["http.method"] == "GET"
span.context().tags["span.kind"] == "server"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["http.status_code"] == 500
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags["error"] == true
span.context().tags["error.msg"] == null
span.context().tags["error.type"] == null
span.context().tags["error.stack"] == null
span.context().tags.size() == 9
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
operationName "servlet.request"
resourceName "GET /$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 500
"error" true
defaultTags()
}
}
}
}
where:
path | expectedResponse

View File

@ -24,7 +24,7 @@ import net.bytebuddy.matcher.ElementMatcher;
@Slf4j
@AutoService(Instrumenter.class)
public final class TraceAnnotationInstrumentation extends Instrumenter.Configurable {
public final class TraceAnnotationsInstrumentation extends Instrumenter.Configurable {
private static final String CONFIG_NAME = "dd.trace.annotations";
static final String CONFIG_FORMAT =
@ -45,7 +45,7 @@ public final class TraceAnnotationInstrumentation extends Instrumenter.Configura
private final Set<String> additionalTraceAnnotations;
public TraceAnnotationInstrumentation() {
public TraceAnnotationsInstrumentation() {
super("trace", "trace-annotation");
final String configString = getPropOrEnv(CONFIG_NAME);

View File

@ -51,13 +51,16 @@ public class TraceConfigInstrumentation extends Instrumenter.Configurable {
} else {
final Map<String, Set<String>> toTrace = Maps.newHashMap();
final String[] classMethods = configString.split(";");
final String[] classMethods = configString.split(";", -1);
for (final String classMethod : classMethods) {
final String[] splitClassMethod = classMethod.split("\\[");
if (classMethod.trim().isEmpty()) {
continue;
}
final String[] splitClassMethod = classMethod.split("\\[", -1);
final String className = splitClassMethod[0];
final String method = splitClassMethod[1].trim();
final String methodNames = method.substring(0, method.length() - 1);
final String[] splitMethodNames = methodNames.split(",");
final String[] splitMethodNames = methodNames.split(",", -1);
final Set<String> trimmedMethodNames =
Sets.newHashSetWithExpectedSize(splitMethodNames.length);
for (final String methodName : splitMethodNames) {

View File

@ -1,13 +1,13 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.instrumentation.trace_annotation.TraceAnnotationInstrumentation
import datadog.trace.instrumentation.trace_annotation.TraceAnnotationsInstrumentation
import dd.test.trace.annotation.SayTracedHello
import spock.lang.Unroll
import java.util.concurrent.Callable
import static TraceAnnotationsInstrumentation.DEFAULT_ANNOTATIONS
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
import static datadog.trace.agent.test.TestUtils.withSystemProperty
import static datadog.trace.instrumentation.trace_annotation.TraceAnnotationInstrumentation.DEFAULT_ANNOTATIONS
class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
@ -22,7 +22,7 @@ class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
def "test disabled nr annotation"() {
setup:
SayTracedHello.fromCallable()
SayTracedHello.fromCallableWhenDisabled()
expect:
TEST_WRITER == []
@ -51,7 +51,7 @@ class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
setup:
def config = null
withSystemProperty("dd.trace.annotations", value) {
def instrumentation = new TraceAnnotationInstrumentation()
def instrumentation = new TraceAnnotationsInstrumentation()
config = instrumentation.additionalTraceAnnotations
}
expect:

View File

@ -60,6 +60,7 @@ class TraceConfigTest extends AgentTestRunner {
" " | [:]
"some.package.ClassName" | [:]
"some.package.ClassName[ , ]" | [:]
"some.package.ClassName[ , method]" | [:]
"some.package.Class\$Name[ method , ]" | ["some.package.Class\$Name": ["method"].toSet()]
"ClassName[ method1,]" | ["ClassName": ["method1"].toSet()]
"ClassName[method1 , method2]" | ["ClassName": ["method1", "method2"].toSet()]

View File

@ -44,4 +44,14 @@ public class SayTracedHello {
}
}.call();
}
public static String fromCallableWhenDisabled() throws Exception {
return new Callable<String>() {
@com.newrelic.api.agent.Trace
@Override
public String call() throws Exception {
return "Howdy!";
}
}.call();
}
}

View File

@ -35,6 +35,7 @@ class SpanAssert {
def spanType(String type) {
assert span.spanType == type
assert span.tags["span.type"] == type
}
def parent() {

View File

@ -30,10 +30,19 @@ class TagsAssert {
}
def errorTags(Class<Throwable> errorType) {
errorTags(errorType, null)
}
def errorTags(Class<Throwable> errorType, String message) {
assertedTags.add("error")
assertedTags.add("error.type")
assertedTags.add("error.stack")
if (message != null) {
assertedTags.add("error.msg")
tags["error.msg"] == message
}
tags["error"] == true
tags["error.type"] == errorType
tags["error.stack"] instanceof String