Fix test. Format.

This commit is contained in:
Tyler Benson 2019-07-26 16:44:57 -07:00
parent d6b903665e
commit 9681b91f3e
14 changed files with 181 additions and 135 deletions

View File

@ -319,7 +319,14 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 500
errorTags(JasperException, String)
"error" true
"error.type" { String tagExceptionType ->
return tagExceptionType == exceptionClass.getName() || tagExceptionType.contains(exceptionClass.getSimpleName())
}
"error.msg" { String tagErrorMsg ->
return errorMessageOptional || tagErrorMsg instanceof String
}
"error.stack" String
defaultTags()
}
}

View File

@ -15,7 +15,6 @@ public class NettyServerTestInstrumentation implements Instrumenter {
.transform(
new AgentBuilder.Transformer.ForAdvice()
.advice(
named("channelRead"),
HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
named("channelRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
}
}

View File

@ -15,7 +15,6 @@ public class NettyServerTestInstrumentation implements Instrumenter {
.transform(
new AgentBuilder.Transformer.ForAdvice()
.advice(
named("channelRead"),
HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
named("channelRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
}
}

View File

@ -33,7 +33,9 @@ public class Servlet3Advice {
final HttpServletRequest httpServletRequest = (HttpServletRequest) req;
final SpanContext extractedContext =
GlobalTracer.get()
.extract(Format.Builtin.HTTP_HEADERS, new HttpServletRequestExtractAdapter(httpServletRequest));
.extract(
Format.Builtin.HTTP_HEADERS,
new HttpServletRequestExtractAdapter(httpServletRequest));
final Scope scope =
GlobalTracer.get()

View File

@ -49,7 +49,7 @@ public class TagSettingAsyncListener implements AsyncListener {
Tags.HTTP_STATUS.set(span, 500);
}
Throwable throwable = event.getThrowable();
if(throwable instanceof ServletException && throwable.getCause() != null) {
if (throwable instanceof ServletException && throwable.getCause() != null) {
throwable = throwable.getCause();
}
DECORATE.onError(span, throwable);

View File

@ -59,23 +59,24 @@ abstract class AbstractServlet3Test<CONTEXT> extends HttpServerTest<Servlet3Deco
}
protected ServerEndpoint lastRequest
@Override
Request.Builder request(ServerEndpoint uri, String _method, String body) {
Request.Builder request(ServerEndpoint uri, String method, String body) {
lastRequest = uri
super.request(uri, _method, body)
super.request(uri, method, body)
}
@Override
void serverSpan(TraceAssert trace, int index, String _traceId = null, String _parentId = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {
serviceName expectedServiceName()
operationName expectedOperationName()
resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint.errored
if (_parentId != null) {
traceId _traceId
parentId _parentId
if (parentID != null) {
traceId traceID
parentId parentID
} else {
parent()
}
@ -87,9 +88,9 @@ abstract class AbstractServlet3Test<CONTEXT> extends HttpServerTest<Servlet3Deco
"$Tags.COMPONENT.key" serverDecorator.component()
if (endpoint.errored) {
"$Tags.ERROR.key" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body}
"error.type" { it == null || it == Exception.name}
"error.stack" { it == null || it instanceof String}
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }
}
"$Tags.HTTP_STATUS.key" endpoint.status
"$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
@ -97,7 +98,7 @@ abstract class AbstractServlet3Test<CONTEXT> extends HttpServerTest<Servlet3Deco
// "$DDTags.HTTP_QUERY" uri.query
// "$DDTags.HTTP_FRAGMENT" { it == null || it == uri.fragment } // Optional
// }
"$Tags.PEER_HOSTNAME.key" "localhost"
"$Tags.PEER_HOSTNAME.key" { it == "localhost" || it == "127.0.0.1" }
"$Tags.PEER_PORT.key" Integer
"$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
"$Tags.HTTP_METHOD.key" method

View File

@ -1,3 +1,4 @@
import datadog.opentracing.DDSpan
import datadog.trace.agent.test.asserts.ListWriterAssert
import datadog.trace.api.DDSpanTypes
import groovy.transform.stc.ClosureParams
@ -32,12 +33,16 @@ abstract class JettyServlet3Test extends AbstractServlet3Test<ServletContextHand
@Override
void startServer(int port) {
jettyServer = new Server(port)
jettyServer.connectors.each { it.resolveNames = true } // get localhost instead of 127.0.0.1
jettyServer.connectors.each {
if (it.hasProperty("resolveNames")) {
it.resolveNames = true // get localhost instead of 127.0.0.1
}
}
ServletContextHandler servletContext = new ServletContextHandler(null, "/$context")
servletContext.errorHandler = new ErrorHandler() {
protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException {
Throwable th = (Throwable)request.getAttribute("javax.servlet.error.exception");
Throwable th = (Throwable) request.getAttribute("javax.servlet.error.exception")
writer.write(th.message)
}
}
@ -137,24 +142,25 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest {
}
}
class JettyServlet3TestDispatchAsync extends JettyDispatchTest {
@Override
Class<Servlet> servlet() {
TestServlet3.Async
}
@Override
protected void setupServlets(ServletContextHandler context) {
super.setupServlets(context)
addServlet(context, "/dispatch" + SUCCESS.path, TestServlet3.DispatchAsync)
addServlet(context, "/dispatch" + ERROR.path, TestServlet3.DispatchAsync)
addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet3.DispatchAsync)
addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchAsync)
addServlet(context, "/dispatch" + AUTH_REQUIRED.path, TestServlet3.DispatchAsync)
addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
}
}
// FIXME: Async context propagation for org.eclipse.jetty.util.thread.QueuedThreadPool.dispatch currently broken.
//class JettyServlet3TestDispatchAsync extends JettyDispatchTest {
// @Override
// Class<Servlet> servlet() {
// TestServlet3.Async
// }
//
// @Override
// protected void setupServlets(ServletContextHandler context) {
// super.setupServlets(context)
//
// addServlet(context, "/dispatch" + SUCCESS.path, TestServlet3.DispatchAsync)
// addServlet(context, "/dispatch" + ERROR.path, TestServlet3.DispatchAsync)
// addServlet(context, "/dispatch" + EXCEPTION.path, TestServlet3.DispatchAsync)
// addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchAsync)
// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, TestServlet3.DispatchAsync)
// addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
// }
//}
abstract class JettyDispatchTest extends JettyServlet3Test {
@Override
@ -166,59 +172,65 @@ abstract class JettyDispatchTest extends JettyServlet3Test {
void cleanAndAssertTraces(
final int size,
@ClosureParams(value = SimpleType, options = "datadog.trace.agent.test.asserts.ListWriterAssert")
@DelegatesTo(value = ListWriterAssert.class, strategy = Closure.DELEGATE_FIRST)
@DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST)
final Closure spec) {
// If this is failing, make sure HttpServerTestAdvice is applied correctly.
TEST_WRITER.waitForTraces(size + 2)
TEST_WRITER.waitForTraces(size * 3) // (test, dispatch, and servlet/controller traces
// TEST_WRITER is a CopyOnWriteArrayList, which doesn't support remove()
def toRemove = TEST_WRITER.find() {
def toRemove = TEST_WRITER.findAll {
it.size() == 1 && it.get(0).operationName == "TEST_SPAN"
}
assertTrace(toRemove, 1) {
basicSpan(it, 0, "TEST_SPAN", "ServerEntry")
}
TEST_WRITER.remove(toRemove)
// Validate dispatch trace
def dispatchTrace = TEST_WRITER.find() {
it.size() == 1 && it.get(0).resourceName.contains("/dispatch/")
}
assertTrace(dispatchTrace, 1) {
def endpoint = lastRequest
span(0) {
serviceName expectedServiceName()
operationName expectedOperationName()
resourceName endpoint.status == 404 ? "404" : "GET ${endpoint.resolve(address).path}"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint.errored
// parent()
tags {
"servlet.context" "/$context"
"servlet.dispatch" endpoint.path
"span.origin.type" { it == TestServlet3.DispatchImmediate.name || it == TestServlet3.DispatchAsync.name || it == ApplicationFilterChain.name }
defaultTags(true)
"$Tags.COMPONENT.key" serverDecorator.component()
if (endpoint.errored) {
"$Tags.ERROR.key" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body}
"error.type" { it == null || it == Exception.name}
"error.stack" { it == null || it instanceof String}
}
"$Tags.HTTP_STATUS.key" endpoint.status
"$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
"$Tags.PEER_HOSTNAME.key" "localhost"
"$Tags.PEER_PORT.key" Integer
"$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
"$Tags.HTTP_METHOD.key" "GET"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
}
assert toRemove.size() == size
toRemove.each {
assertTrace(it, 1) {
basicSpan(it, 0, "TEST_SPAN", "ServerEntry")
}
}
TEST_WRITER.remove(dispatchTrace)
TEST_WRITER.removeAll(toRemove)
// Make sure that the trace has a span with the dispatchTrace as a parent.
assert TEST_WRITER.any { it.any { it.parentId == dispatchTrace[0].spanId } }
// Validate dispatch trace
def dispatchTraces = TEST_WRITER.findAll {
it.size() == 1 && it.get(0).resourceName.contains("/dispatch/")
}
assert dispatchTraces.size() == size
dispatchTraces.each { List<DDSpan> dispatchTrace ->
assertTrace(dispatchTrace, 1) {
def endpoint = lastRequest
span(0) {
serviceName expectedServiceName()
operationName expectedOperationName()
resourceName endpoint.status == 404 ? "404" : "GET ${endpoint.resolve(address).path}"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint.errored
// parent()
tags {
"servlet.context" "/$context"
"servlet.dispatch" endpoint.path
"span.origin.type" {
it == TestServlet3.DispatchImmediate.name || it == TestServlet3.DispatchAsync.name || it == ApplicationFilterChain.name
}
defaultTags(true)
"$Tags.COMPONENT.key" serverDecorator.component()
if (endpoint.errored) {
"$Tags.ERROR.key" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }
}
"$Tags.HTTP_STATUS.key" endpoint.status
"$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
"$Tags.PEER_HOSTNAME.key" { it == "localhost" || it == "127.0.0.1" }
"$Tags.PEER_PORT.key" Integer
"$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
"$Tags.HTTP_METHOD.key" "GET"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
}
}
}
// Make sure that the trace has a span with the dispatchTrace as a parent.
assert TEST_WRITER.any { it.any { it.parentId == dispatchTrace[0].spanId } }
}
}
}

View File

@ -11,14 +11,22 @@ public class ServletTestInstrumentation implements Instrumenter {
@Override
public AgentBuilder instrument(final AgentBuilder agentBuilder) {
return agentBuilder
// Jetty
.type(named("org.eclipse.jetty.server.AbstractHttpConnection"))
.transform(new AgentBuilder.Transformer.ForAdvice()
.advice(named("headerComplete"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()))
// Tomcat
.type(named("org.apache.catalina.connector.CoyoteAdapter"))
.transform(new AgentBuilder.Transformer.ForAdvice()
.advice(named("service"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()))
;
// Jetty 8
.type(named("org.eclipse.jetty.server.AbstractHttpConnection"))
.transform(
new AgentBuilder.Transformer.ForAdvice()
.advice(
named("headerComplete"),
HttpServerTestAdvice.ServerEntryAdvice.class.getName()))
// Jetty 9
.type(named("org.eclipse.jetty.server.HttpChannel"))
.transform(
new AgentBuilder.Transformer.ForAdvice()
.advice(named("handle"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()))
// Tomcat
.type(named("org.apache.catalina.connector.CoyoteAdapter"))
.transform(
new AgentBuilder.Transformer.ForAdvice()
.advice(named("service"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
}
}

View File

@ -1,4 +1,5 @@
import com.google.common.io.Files
import datadog.opentracing.DDSpan
import datadog.trace.agent.test.asserts.ListWriterAssert
import datadog.trace.api.DDSpanTypes
import groovy.transform.stc.ClosureParams
@ -20,6 +21,7 @@ import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
@ -207,64 +209,75 @@ abstract class TomcatDispatchTest extends TomcatServlet3Test {
void cleanAndAssertTraces(
final int size,
@ClosureParams(value = SimpleType, options = "datadog.trace.agent.test.asserts.ListWriterAssert")
@DelegatesTo(value = ListWriterAssert.class, strategy = Closure.DELEGATE_FIRST)
@DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST)
final Closure spec) {
// If this is failing, make sure HttpServerTestAdvice is applied correctly.
TEST_WRITER.waitForTraces(size * 2)
if (lastRequest == NOT_FOUND) {
TEST_WRITER.waitForTraces(size * 2) // (test and servlet/controller traces
} else {
TEST_WRITER.waitForTraces(size * 3) // (test, dispatch, and servlet/controller traces
}
// TEST_WRITER is a CopyOnWriteArrayList, which doesn't support remove()
def toRemove = TEST_WRITER.findAll() {
def toRemove = TEST_WRITER.findAll {
it.size() == 1 && it.get(0).operationName == "TEST_SPAN"
}
assert toRemove.size() == size
toRemove.each {
assertTrace(it, 1) {
basicSpan(it, 0, "TEST_SPAN", "ServerEntry")
}
}
assert toRemove.size() == size
TEST_WRITER.removeAll(toRemove)
if (lastRequest == NOT_FOUND) {
// Tomcat won't "dispatch" an unregistered url
assertTraces(size, spec)
return
}
// Validate dispatch trace
def dispatchTrace = TEST_WRITER.find() {
def dispatchTraces = TEST_WRITER.findAll {
it.size() == 1 && it.get(0).resourceName.contains("/dispatch/")
}
assertTrace(dispatchTrace, 1) {
def endpoint = lastRequest
span(0) {
serviceName expectedServiceName()
operationName expectedOperationName()
resourceName endpoint.status == 404 ? "404" : "GET ${endpoint.resolve(address).path}"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint.errored
// parent()
tags {
"servlet.context" "/$context"
"servlet.dispatch" endpoint.path
"span.origin.type" {
it == TestServlet3.DispatchImmediate.name || it == TestServlet3.DispatchAsync.name || it == ApplicationFilterChain.name
}
assert dispatchTraces.size() == size
dispatchTraces.each { List<DDSpan> dispatchTrace ->
assertTrace(dispatchTrace, 1) {
def endpoint = lastRequest
span(0) {
serviceName expectedServiceName()
operationName expectedOperationName()
resourceName endpoint.status == 404 ? "404" : "GET ${endpoint.resolve(address).path}"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint.errored
// parent()
tags {
"servlet.context" "/$context"
"servlet.dispatch" endpoint.path
"span.origin.type" {
it == TestServlet3.DispatchImmediate.name || it == TestServlet3.DispatchAsync.name || it == ApplicationFilterChain.name
}
defaultTags(true)
"$Tags.COMPONENT.key" serverDecorator.component()
if (endpoint.errored) {
"$Tags.ERROR.key" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body}
"error.type" { it == null || it == Exception.name}
"error.stack" { it == null || it instanceof String}
defaultTags(true)
"$Tags.COMPONENT.key" serverDecorator.component()
if (endpoint.errored) {
"$Tags.ERROR.key" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }
}
"$Tags.HTTP_STATUS.key" endpoint.status
"$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
"$Tags.PEER_HOSTNAME.key" "localhost"
"$Tags.PEER_PORT.key" Integer
"$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
"$Tags.HTTP_METHOD.key" "GET"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
}
"$Tags.HTTP_STATUS.key" endpoint.status
"$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
"$Tags.PEER_HOSTNAME.key" "localhost"
"$Tags.PEER_PORT.key" Integer
"$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
"$Tags.HTTP_METHOD.key" "GET"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
}
}
// Make sure that the trace has a span with the dispatchTrace as a parent.
assert TEST_WRITER.any { it.any { it.parentId == dispatchTrace[0].spanId } }
}
TEST_WRITER.remove(dispatchTrace)
// Make sure that the trace has a span with the dispatchTrace as a parent.
assert TEST_WRITER.any { it.any { it.parentId == dispatchTrace[0].spanId } }
}
}

View File

@ -12,7 +12,6 @@ import org.springframework.boot.test.context.SpringBootTest.WebEnvironment
import org.springframework.boot.test.web.client.TestRestTemplate
import org.springframework.http.HttpStatus
import org.springframework.web.bind.MethodArgumentNotValidException
import org.springframework.web.util.NestedServletException
import static test.Application.PASS
import static test.Application.USER
@ -246,7 +245,7 @@ class SpringBootBasedTest extends AgentTestRunner {
"component" "java-web-servlet"
"http.status_code" 500
"$DDTags.USER_NAME" USER
errorTags NestedServletException, "Request processing failed; nested exception is java.lang.RuntimeException: qwerty"
errorTags RuntimeException, "qwerty"
defaultTags()
}
}

View File

@ -1,3 +1,5 @@
import static datadog.trace.agent.test.AgentTestRunner.blockUntilChildSpansFinished;
import datadog.trace.api.Trace;
import io.vertx.core.AbstractVerticle;
import io.vertx.core.DeploymentOptions;
@ -77,6 +79,7 @@ public class VertxWebTestServer extends AbstractVerticle {
.setStatusCode(response.statusCode())
.end(buffer);
});
blockUntilChildSpansFinished(1);
})
.end(Optional.ofNullable(routingContext.getBody()).orElse(Buffer.buffer()));
});

View File

@ -27,6 +27,7 @@ import java.util.ServiceLoader;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
import net.bytebuddy.agent.ByteBuddyAgent;
import net.bytebuddy.agent.builder.AgentBuilder;
@ -202,7 +203,8 @@ public abstract class AgentTestRunner extends Specification {
ListWriterAssert.assertTraces(TEST_WRITER, size, spec);
}
public void blockUntilChildSpansFinished(final int numberOfSpans) throws InterruptedException {
@SneakyThrows
public static void blockUntilChildSpansFinished(final int numberOfSpans) {
final Span span = io.opentracing.util.GlobalTracer.get().activeSpan();
if (span instanceof DDSpan) {
final PendingTrace pendingTrace = ((DDSpan) span).context().getTrace();

View File

@ -145,7 +145,7 @@ abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends Age
where:
method = "GET"
body = null
count << [ 1, 4, 50 ] // make multiple requests.
count << [1, 4, 50] // make multiple requests.
}
def "test success with parent"() {
@ -251,7 +251,7 @@ abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends Age
// If this is failing, make sure HttpServerTestAdvice is applied correctly.
TEST_WRITER.waitForTraces(size * 2)
// TEST_WRITER is a CopyOnWriteArrayList, which doesn't support remove()
def toRemove = TEST_WRITER.findAll() {
def toRemove = TEST_WRITER.findAll {
it.size() == 1 && it.get(0).operationName == "TEST_SPAN"
}
toRemove.each {
@ -321,6 +321,7 @@ abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends Age
def setup() {
ENABLE_TEST_ADVICE.set(true)
}
def cleanup() {
ENABLE_TEST_ADVICE.set(false)
}

View File

@ -31,7 +31,7 @@ public abstract class HttpServerTestAdvice {
}
}
@Advice.OnMethodExit
@Advice.OnMethodExit(onThrowable = Throwable.class)
public static void methodExit(@Advice.Enter final Scope scope) {
scope.close();
}