Skip to content

Commit 5edd65d

Browse files
realarkrealark
authored
Merge pull request #261 from DataDog/ark/500_errors
Mark 5xx status codes as an error
2 parents d4ad0a0 + 10e12a6 commit 5edd65d

9 files changed

Lines changed: 123 additions & 21 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class JettyServletTest extends AgentTestRunner {
9999

100100
span.context().serviceName == "unnamed-java-app"
101101
span.context().operationName == "servlet.request"
102-
span.context().resourceName == "servlet.request"
102+
span.context().resourceName == "GET /$path"
103103
span.context().spanType == DDSpanTypes.WEB_SERVLET
104104
!span.context().getErrorFlag()
105105
span.context().parentId != 0 // parent should be the okhttp call.
@@ -134,7 +134,7 @@ class JettyServletTest extends AgentTestRunner {
134134
def span = trace[0]
135135

136136
span.context().operationName == "servlet.request"
137-
span.context().resourceName == "servlet.request"
137+
span.context().resourceName == "GET /$path"
138138
span.context().spanType == DDSpanTypes.WEB_SERVLET
139139
span.context().getErrorFlag()
140140
span.context().parentId != 0 // parent should be the okhttp call.

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

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class JettyServletTest extends AgentTestRunner {
100100

101101
span.context().serviceName == "unnamed-java-app"
102102
span.context().operationName == "servlet.request"
103-
span.context().resourceName == "servlet.request"
103+
span.context().resourceName == "GET /$path"
104104
span.context().spanType == DDSpanTypes.WEB_SERVLET
105105
!span.context().getErrorFlag()
106106
span.context().parentId != 0 // parent should be the okhttp call.
@@ -137,7 +137,7 @@ class JettyServletTest extends AgentTestRunner {
137137

138138
span.context().serviceName == "unnamed-java-app"
139139
span.context().operationName == "servlet.request"
140-
span.context().resourceName == "servlet.request"
140+
span.context().resourceName == "GET /$path"
141141
span.context().spanType == DDSpanTypes.WEB_SERVLET
142142
span.context().getErrorFlag()
143143
span.context().parentId != 0 // parent should be the okhttp call.
@@ -160,6 +160,46 @@ class JettyServletTest extends AgentTestRunner {
160160
"sync" | "Hello Sync"
161161
}
162162

163+
@Unroll
164+
def "test #path non-throwing-error servlet call"() {
165+
setup:
166+
def request = new Request.Builder()
167+
.url("http://localhost:$PORT/$path?non-throwing-error=true")
168+
.get()
169+
.build()
170+
def response = client.newCall(request).execute()
171+
172+
expect:
173+
response.body().string().trim() != expectedResponse
174+
writer.size() == 2 // second (parent) trace is the okhttp call above...
175+
def trace = writer.firstTrace()
176+
trace.size() == 1
177+
def span = trace[0]
178+
179+
span.context().serviceName == "unnamed-java-app"
180+
span.context().operationName == "servlet.request"
181+
span.context().resourceName == "GET /$path"
182+
span.context().spanType == DDSpanTypes.WEB_SERVLET
183+
span.context().getErrorFlag()
184+
span.context().parentId != 0 // parent should be the okhttp call.
185+
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
186+
span.context().tags["http.method"] == "GET"
187+
span.context().tags["span.kind"] == "server"
188+
span.context().tags["component"] == "java-web-servlet"
189+
span.context().tags["http.status_code"] == 500
190+
span.context().tags["thread.name"] != null
191+
span.context().tags["thread.id"] != null
192+
span.context().tags["error"] == true
193+
span.context().tags["error.msg"] == null
194+
span.context().tags["error.type"] == null
195+
span.context().tags["error.stack"] == null
196+
span.context().tags.size() == 9
197+
198+
where:
199+
path | expectedResponse
200+
"sync" | "Hello Sync"
201+
}
202+
163203
private static int randomOpenPort() {
164204
new ServerSocket(0).withCloseable {
165205
it.setReuseAddress(true)

‎dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet.groovy‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ class TestServlet {
1313
if (req.getParameter("error") != null) {
1414
throw new RuntimeException("some sync error")
1515
}
16+
if (req.getParameter("non-throwing-error") != null) {
17+
resp.sendError(500, "some sync error")
18+
return
19+
}
1620
resp.writer.print("Hello Sync")
1721
}
1822
}

‎dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class TomcatServletTest extends AgentTestRunner {
9999

100100
span.context().serviceName == "unnamed-java-app"
101101
span.context().operationName == "servlet.request"
102-
span.context().resourceName == "servlet.request"
102+
span.context().resourceName == "GET /$path"
103103
span.context().spanType == DDSpanTypes.WEB_SERVLET
104104
!span.context().getErrorFlag()
105105
span.context().parentId != 0 // parent should be the okhttp call.
@@ -136,7 +136,7 @@ class TomcatServletTest extends AgentTestRunner {
136136

137137
span.context().serviceName == "unnamed-java-app"
138138
span.context().operationName == "servlet.request"
139-
span.context().resourceName == "servlet.request"
139+
span.context().resourceName == "GET /$path"
140140
span.context().spanType == DDSpanTypes.WEB_SERVLET
141141
span.context().getErrorFlag()
142142
span.context().parentId != 0 // parent should be the okhttp call.
@@ -159,6 +159,46 @@ class TomcatServletTest extends AgentTestRunner {
159159
"sync" | "Hello Sync"
160160
}
161161

162+
@Unroll
163+
def "test #path error servlet call for non-throwing error"() {
164+
setup:
165+
def request = new Request.Builder()
166+
.url("http://localhost:$PORT/$path?non-throwing-error=true")
167+
.get()
168+
.build()
169+
def response = client.newCall(request).execute()
170+
171+
expect:
172+
response.body().string().trim() != expectedResponse
173+
writer.size() == 2 // second (parent) trace is the okhttp call above...
174+
def trace = writer.firstTrace()
175+
trace.size() == 1
176+
def span = trace[0]
177+
178+
span.context().serviceName == "unnamed-java-app"
179+
span.context().operationName == "servlet.request"
180+
span.context().resourceName == "GET /$path"
181+
span.context().spanType == DDSpanTypes.WEB_SERVLET
182+
span.context().getErrorFlag()
183+
span.context().parentId != 0 // parent should be the okhttp call.
184+
span.context().tags["http.url"] == "http://localhost:$PORT/$path"
185+
span.context().tags["http.method"] == "GET"
186+
span.context().tags["span.kind"] == "server"
187+
span.context().tags["component"] == "java-web-servlet"
188+
span.context().tags["http.status_code"] == 500
189+
span.context().tags["thread.name"] != null
190+
span.context().tags["thread.id"] != null
191+
span.context().tags["error"] == true
192+
span.context().tags["error.msg"] == null
193+
span.context().tags["error.type"] == null
194+
span.context().tags["error.stack"] == null
195+
span.context().tags.size() == 9
196+
197+
where:
198+
path | expectedResponse
199+
"sync" | "Hello Sync"
200+
}
201+
162202
private static int randomOpenPort() {
163203
new ServerSocket(0).withCloseable {
164204
it.setReuseAddress(true)

‎dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,17 +148,17 @@ class SpringBootBasedTest extends AgentTestRunner {
148148
span1.context().operationName == "servlet.request"
149149
span1.context().resourceName == "GET /error"
150150
span1.context().spanType == DDSpanTypes.WEB_SERVLET
151-
!span1.context().getErrorFlag()
152151
span1.context().parentId == 0
153152
span1.context().tags["http.url"] == "http://localhost:$port/error"
154153
span1.context().tags["http.method"] == "GET"
155154
span1.context().tags["span.kind"] == "server"
156155
span1.context().tags["span.type"] == "web"
157156
span1.context().tags["component"] == "java-web-servlet"
158157
span1.context().tags["http.status_code"] == 500
158+
span1.context().getErrorFlag()
159159
span1.context().tags["thread.name"] != null
160160
span1.context().tags["thread.id"] != null
161-
span1.context().tags.size() == 8
161+
span1.context().tags.size() == 9
162162
}
163163

164164
def "validated form"() {

‎dd-java-agent/testing/src/main/java/datadog/trace/agent/test/AgentTestRunner.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
import ch.qos.logback.classic.Logger;
55
import datadog.opentracing.DDSpan;
66
import datadog.opentracing.DDTracer;
7-
import datadog.opentracing.decorators.AbstractDecorator;
8-
import datadog.opentracing.decorators.DDDecoratorsFactory;
97
import datadog.trace.agent.tooling.AgentInstaller;
108
import datadog.trace.agent.tooling.Instrumenter;
119
import datadog.trace.common.writer.ListWriter;
@@ -78,10 +76,6 @@ public boolean add(final List<DDSpan> trace) {
7876
};
7977
TEST_TRACER = new DDTracer(TEST_WRITER);
8078

81-
final List<AbstractDecorator> decorators = DDDecoratorsFactory.createBuiltinDecorators();
82-
for (final AbstractDecorator decorator : decorators) {
83-
((DDTracer) TEST_TRACER).addDecorator(decorator);
84-
}
8579
ByteBuddyAgent.install();
8680
instrumentation = ByteBuddyAgent.getInstrumentation();
8781
}

‎dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,6 @@ public DDTracer(final Properties config) {
9292
Sampler.Builder.forConfig(config),
9393
DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.SPAN_TAGS)));
9494
log.debug("Using config: {}", config);
95-
96-
// Create decorators from resource files
97-
final List<AbstractDecorator> decorators = DDDecoratorsFactory.createBuiltinDecorators();
98-
for (final AbstractDecorator decorator : decorators) {
99-
log.debug("Loading decorator: {}", decorator.getClass().getSimpleName());
100-
addDecorator(decorator);
101-
}
10295
}
10396

10497
public DDTracer(final String serviceName, final Writer writer, final Sampler sampler) {
@@ -126,6 +119,12 @@ public DDTracer(
126119

127120
registerClassLoader(ClassLoader.getSystemClassLoader());
128121

122+
final List<AbstractDecorator> decorators = DDDecoratorsFactory.createBuiltinDecorators();
123+
for (final AbstractDecorator decorator : decorators) {
124+
log.debug("Loading decorator: {}", decorator.getClass().getSimpleName());
125+
addDecorator(decorator);
126+
}
127+
129128
log.info("New instance: {}", this);
130129
}
131130

‎dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public static List<AbstractDecorator> createBuiltinDecorators() {
2525
builtin.add(new OperationDecorator());
2626
builtin.add(new Status404Decorator());
2727
builtin.add(new URLAsResourceName());
28+
builtin.add(new Status5XXDecorator());
2829

2930
return builtin;
3031
}

‎dd-trace-ot/src/main/java/datadog/opentracing/decorators/Status5XXDecorator.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package datadog.opentracing.decorators;
2+
3+
import datadog.opentracing.DDSpanContext;
4+
import io.opentracing.tag.Tags;
5+
6+
/** Mark all 5xx status codes as an error */
7+
public class Status5XXDecorator extends AbstractDecorator {
8+
public Status5XXDecorator() {
9+
super();
10+
this.setMatchingTag(Tags.HTTP_STATUS.getKey());
11+
}
12+
13+
@Override
14+
public boolean afterSetTag(final DDSpanContext context, final String tag, final Object value) {
15+
if (Tags.HTTP_STATUS.getKey().equals(tag)) {
16+
final int responseCode = Integer.parseInt(value.toString());
17+
if (500 <= responseCode && responseCode < 600) {
18+
context.setTag(Tags.ERROR.getKey(), true);
19+
return true;
20+
}
21+
}
22+
return false;
23+
}
24+
}

0 commit comments

Comments
 (0)