diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy index 980a1425d8..143e0394d0 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy @@ -166,6 +166,46 @@ class JettyServletTest extends AgentTestRunner { "sync" | "Hello Sync" } + @Unroll + def "test #path non-throwing-error servlet call"() { + 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 + writer.size() == 2 // second (parent) trace is the okhttp call above... + 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 // parent should be the okhttp call. + 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 + + where: + path | expectedResponse + "sync" | "Hello Sync" + } + private static int randomOpenPort() { new ServerSocket(0).withCloseable { it.setReuseAddress(true) diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet.groovy index fa25eaaa1d..cf62ea9896 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet.groovy @@ -13,6 +13,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") } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy index 99f51442a7..2ac4cc4061 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy @@ -166,6 +166,46 @@ class TomcatServletTest extends AgentTestRunner { "sync" | "Hello Sync" } + @Unroll + def "test #path error servlet call for non-throwing error"() { + 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 + writer.size() == 2 // second (parent) trace is the okhttp call above... + 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 // parent should be the okhttp call. + 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 + + where: + path | expectedResponse + "sync" | "Hello Sync" + } + private static int randomOpenPort() { new ServerSocket(0).withCloseable { it.setReuseAddress(true) diff --git a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy index 93be3b64b6..38283272a5 100644 --- a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy @@ -148,7 +148,6 @@ class SpringBootBasedTest extends AgentTestRunner { span1.context().operationName == "servlet.request" span1.context().resourceName == "GET /error" span1.context().spanType == DDSpanTypes.WEB_SERVLET - !span1.context().getErrorFlag() span1.context().parentId == 0 span1.context().tags["http.url"] == "http://localhost:$port/error" span1.context().tags["http.method"] == "GET" @@ -156,9 +155,10 @@ class SpringBootBasedTest extends AgentTestRunner { span1.context().tags["span.type"] == "web" span1.context().tags["component"] == "java-web-servlet" span1.context().tags["http.status_code"] == 500 + span1.context().getErrorFlag() span1.context().tags["thread.name"] != null span1.context().tags["thread.id"] != null - span1.context().tags.size() == 8 + span1.context().tags.size() == 9 } def "validated form"() { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java index 94962bcbff..0786cd8c48 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java @@ -25,6 +25,7 @@ public class DDDecoratorsFactory { builtin.add(new OperationDecorator()); builtin.add(new Status404Decorator()); builtin.add(new URLAsResourceName()); + builtin.add(new Status5XXDecorator()); return builtin; } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java new file mode 100644 index 0000000000..6a094e80c4 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java @@ -0,0 +1,24 @@ +package datadog.opentracing.decorators; + +import datadog.opentracing.DDSpanContext; +import io.opentracing.tag.Tags; + +/** Mark all 5xx status codes as an error */ +public class Status5XXDecorator extends AbstractDecorator { + public Status5XXDecorator() { + super(); + this.setMatchingTag(Tags.HTTP_STATUS.getKey()); + } + + @Override + public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) { + if (Tags.HTTP_STATUS.getKey().equals(tag)) { + final int responseCode = Integer.parseInt(value.toString()); + if (500 <= responseCode && responseCode < 600) { + context.setTag(Tags.ERROR.getKey(), true); + return true; + } + } + return false; + } +}