Null or empty tags should remove previously set values.
Add context to servlet2 test.
Clean up some declared tracers that mess up the classpath.
MODIFY
dd-java-agent/instrumentation/jax-rs-annotations/src/test/groovy/JerseyTest.groovy
dd-java-agent/instrumentation/jax-rs-annotations/src/test/groovy/JerseyTest.groovy
@@ -3,6 +3,8 @@ import io.dropwizard.testing.junit.ResourceTestRule
import org.junit.ClassRule
import spock.lang.Shared
+import static datadog.trace.agent.test.TestUtils.runUnderTrace
+
class JerseyTest extends AgentTestRunner {
@Shared
@@ -12,9 +14,9 @@ class JerseyTest extends AgentTestRunner {
def "test resource"() {
setup:
// start a trace because the test doesn't go through any servlet or other instrumentation.
- def scope = getTestTracer().buildSpan("test.span").startActive(true)
- def response = resources.client().resource("/test/hello/bob").post(String)
- scope.close()
+ def response = runUnderTrace("test.span") {
+ resources.client().resource("/test/hello/bob").post(String)
+ }
expect:
response == "Hello bob!"

MODIFY
dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy
dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy
-import datadog.opentracing.DDSpan
-import datadog.opentracing.DDTracer
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.agent.test.OkHttpUtils
import datadog.trace.agent.test.TestUtils
import datadog.trace.api.DDSpanTypes
-import datadog.trace.common.writer.ListWriter
-import io.opentracing.util.GlobalTracer
import okhttp3.Credentials
import okhttp3.Interceptor
import okhttp3.OkHttpClient
@@ -21,23 +17,15 @@ import org.eclipse.jetty.security.authentication.BasicAuthenticator
import org.eclipse.jetty.server.Server
import org.eclipse.jetty.servlet.ServletContextHandler
-import java.lang.reflect.Field
-import java.util.concurrent.CountDownLatch
-import java.util.concurrent.TimeUnit
-
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class JettyServlet2Test extends AgentTestRunner {
- // Jetty needs this to ensure consistent ordering for async.
- CountDownLatch latch = new CountDownLatch(1)
-
- OkHttpClient client = OkHttpUtils.clientBuilder()
- .addNetworkInterceptor(new Interceptor() {
+ OkHttpClient client = OkHttpUtils.clientBuilder().addNetworkInterceptor(new Interceptor() {
@Override
Response intercept(Interceptor.Chain chain) throws IOException {
def response = chain.proceed(chain.request())
- latch.await(30, TimeUnit.SECONDS) // don't block forever or test never fails.
+ TEST_WRITER.waitForTraces(1)
return response
}
})
@@ -47,19 +35,11 @@ class JettyServlet2Test extends AgentTestRunner {
private Server jettyServer
private ServletContextHandler servletContext
- ListWriter writer = new ListWriter() {
- @Override
- void write(final List<DDSpan> trace) {
- add(trace)
- JettyServlet2Test.this.latch.countDown()
- }
- }
- DDTracer tracer = new DDTracer(writer)
-
def setup() {
port = TestUtils.randomOpenPort()
jettyServer = new Server(port)
servletContext = new ServletContextHandler()
+ servletContext.contextPath = "/ctx"
ConstraintSecurityHandler security = setupAuthentication(jettyServer)
@@ -72,17 +52,6 @@ class JettyServlet2Test extends AgentTestRunner {
System.out.println(
"Jetty server: http://localhost:" + port + "/")
-
- try {
- GlobalTracer.register(tracer)
- } catch (final Exception e) {
- // Force it anyway using reflection
- final Field field = GlobalTracer.getDeclaredField("tracer")
- field.setAccessible(true)
- field.set(null, tracer)
- }
- writer.start()
- assert GlobalTracer.isRegistered()
}
def cleanup() {
@@ -93,7 +62,7 @@ class JettyServlet2Test extends AgentTestRunner {
def "test #path servlet call"() {
setup:
def requestBuilder = new Request.Builder()
- .url("http://localhost:$port/$path")
+ .url("http://localhost:$port/ctx/$path")
.get()
if (auth) {
requestBuilder.header(HttpHeaders.AUTHORIZATION, Credentials.basic("user", "password"))
@@ -103,22 +72,22 @@ class JettyServlet2Test extends AgentTestRunner {
expect:
response.body().string().trim() == expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
- serviceName "unnamed-java-app"
+ serviceName "ctx"
operationName "servlet.request"
- resourceName "GET /$path"
+ resourceName "GET /ctx/$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
- "http.url" "http://localhost:$port/$path"
+ "http.url" "http://localhost:$port/ctx/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
- "servlet.context" ""
+ "servlet.context" "/ctx"
if (auth) {
"user.principal" "user"
}
@@ -137,7 +106,7 @@ class JettyServlet2Test extends AgentTestRunner {
def "test #path error servlet call"() {
setup:
def request = new Request.Builder()
- .url("http://localhost:$port/$path?error=true")
+ .url("http://localhost:$port/ctx/$path?error=true")
.get()
.build()
def response = client.newCall(request).execute()
@@ -145,22 +114,22 @@ class JettyServlet2Test extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
- serviceName "unnamed-java-app"
+ serviceName "ctx"
operationName "servlet.request"
- resourceName "GET /$path"
+ resourceName "GET /ctx/$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
- "http.url" "http://localhost:$port/$path"
+ "http.url" "http://localhost:$port/ctx/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
- "servlet.context" ""
+ "servlet.context" "/ctx"
errorTags(RuntimeException, "some $path error")
defaultTags()
}
@@ -177,7 +146,7 @@ class JettyServlet2Test extends AgentTestRunner {
// 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")
+ .url("http://localhost:$port/ctx/$path?non-throwing-error=true")
.get()
.build()
def response = client.newCall(request).execute()
@@ -185,22 +154,22 @@ class JettyServlet2Test extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
- serviceName "unnamed-java-app"
+ serviceName "ctx"
operationName "servlet.request"
- resourceName "GET /$path"
+ resourceName "GET /ctx/$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
- "http.url" "http://localhost:$port/$path"
+ "http.url" "http://localhost:$port/ctx/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
- "servlet.context" ""
+ "servlet.context" "/ctx"
defaultTags()
}
}

MODIFY
dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy
dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy
-import datadog.opentracing.DDSpan
-import datadog.opentracing.DDTracer
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.agent.test.OkHttpUtils
import datadog.trace.agent.test.TestUtils
import datadog.trace.api.DDSpanTypes
-import datadog.trace.common.writer.ListWriter
-import io.opentracing.util.GlobalTracer
import okhttp3.Credentials
import okhttp3.Interceptor
import okhttp3.OkHttpClient
@@ -21,23 +17,15 @@ import org.eclipse.jetty.server.Server
import org.eclipse.jetty.servlet.ServletContextHandler
import org.eclipse.jetty.util.security.Constraint
-import java.lang.reflect.Field
-import java.util.concurrent.CountDownLatch
-import java.util.concurrent.TimeUnit
-
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class JettyServlet3Test extends AgentTestRunner {
- // Jetty needs this to ensure consistent ordering for async.
- CountDownLatch latch = new CountDownLatch(1)
-
- OkHttpClient client = OkHttpUtils.clientBuilder()
- .addNetworkInterceptor(new Interceptor() {
+ OkHttpClient client = OkHttpUtils.clientBuilder().addNetworkInterceptor(new Interceptor() {
@Override
Response intercept(Interceptor.Chain chain) throws IOException {
def response = chain.proceed(chain.request())
- latch.await(30, TimeUnit.SECONDS) // don't block forever or test never fails.
+ TEST_WRITER.waitForTraces(1)
return response
}
})
@@ -47,15 +35,6 @@ class JettyServlet3Test extends AgentTestRunner {
private Server jettyServer
private ServletContextHandler servletContext
- ListWriter writer = new ListWriter() {
- @Override
- void write(final List<DDSpan> trace) {
- add(trace)
- JettyServlet3Test.this.latch.countDown()
- }
- }
- DDTracer tracer = new DDTracer(writer)
-
def setup() {
port = TestUtils.randomOpenPort()
jettyServer = new Server(port)
@@ -74,17 +53,6 @@ class JettyServlet3Test extends AgentTestRunner {
System.out.println(
"Jetty server: http://localhost:" + port + "/")
-
- try {
- GlobalTracer.register(tracer)
- } catch (final Exception e) {
- // Force it anyway using reflection
- final Field field = GlobalTracer.getDeclaredField("tracer")
- field.setAccessible(true)
- field.set(null, tracer)
- }
- writer.start()
- assert GlobalTracer.isRegistered()
}
def cleanup() {
@@ -105,7 +73,7 @@ class JettyServlet3Test extends AgentTestRunner {
expect:
response.body().string().trim() == expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
@@ -120,7 +88,6 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
- "servlet.context" ""
"http.status_code" 200
if (auth) {
"user.principal" "user"
@@ -151,7 +118,7 @@ class JettyServlet3Test extends AgentTestRunner {
}
expect:
- assertTraces(writer, numTraces) {
+ assertTraces(TEST_WRITER, numTraces) {
for (int i = 0; i < numTraces; ++i) {
trace(i, 1) {
span(0) {
@@ -175,7 +142,7 @@ class JettyServlet3Test extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
@@ -190,7 +157,6 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
- "servlet.context" ""
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
defaultTags()
@@ -216,7 +182,7 @@ class JettyServlet3Test extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
@@ -231,7 +197,6 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
- "servlet.context" ""
"http.status_code" 500
"error" true
defaultTags()

MODIFY
dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy
dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy
import com.google.common.io.Files
-import datadog.opentracing.DDTracer
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.agent.test.OkHttpUtils
import datadog.trace.agent.test.TestUtils
import datadog.trace.api.DDSpanTypes
-import datadog.trace.common.writer.ListWriter
-import io.opentracing.util.GlobalTracer
import okhttp3.OkHttpClient
import okhttp3.Request
import org.apache.catalina.Context
@@ -13,8 +10,6 @@ import org.apache.catalina.startup.Tomcat
import org.apache.tomcat.JarScanFilter
import org.apache.tomcat.JarScanType
-import java.lang.reflect.Field
-
import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class TomcatServlet3Test extends AgentTestRunner {
@@ -25,9 +20,6 @@ class TomcatServlet3Test extends AgentTestRunner {
Tomcat tomcatServer
Context appContext
- ListWriter writer = new ListWriter()
- DDTracer tracer = new DDTracer(writer)
-
def setup() {
port = TestUtils.randomOpenPort()
tomcatServer = new Tomcat()
@@ -59,18 +51,6 @@ class TomcatServlet3Test extends AgentTestRunner {
tomcatServer.start()
System.out.println(
"Tomcat server: http://" + tomcatServer.getHost().getName() + ":" + port + "/")
-
-
- try {
- GlobalTracer.register(tracer)
- } catch (final Exception e) {
- // Force it anyway using reflection
- final Field field = GlobalTracer.getDeclaredField("tracer")
- field.setAccessible(true)
- field.set(null, tracer)
- }
- writer.start()
- assert GlobalTracer.isRegistered()
}
def cleanup() {
@@ -89,7 +69,7 @@ class TomcatServlet3Test extends AgentTestRunner {
expect:
response.body().string().trim() == expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
serviceName "my-context"
@@ -129,7 +109,7 @@ class TomcatServlet3Test extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
serviceName "my-context"
@@ -170,7 +150,7 @@ class TomcatServlet3Test extends AgentTestRunner {
expect:
response.body().string().trim() != expectedResponse
- assertTraces(writer, 1) {
+ assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
serviceName "my-context"

MODIFY
dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy
dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy
@@ -49,11 +49,10 @@ class SpringBootBasedTest extends AgentTestRunner {
span.context().tags["span.kind"] == "server"
span.context().tags["span.type"] == "web"
span.context().tags["component"] == "java-web-servlet"
- span.context().tags["servlet.context"] == ""
span.context().tags["http.status_code"] == 200
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
- span.context().tags.size() == 9
+ span.context().tags.size() == 8
}
def "generates 404 spans"() {
@@ -79,11 +78,10 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["span.kind"] == "server"
span0.context().tags["span.type"] == "web"
span0.context().tags["component"] == "java-web-servlet"
- span0.context().tags["servlet.context"] == ""
span0.context().tags["http.status_code"] == 404
span0.context().tags["thread.name"] != null
span0.context().tags["thread.id"] != null
- span0.context().tags.size() == 9
+ span0.context().tags.size() == 8
and: // trace 1
def trace1 = TEST_WRITER.get(1)
@@ -100,11 +98,10 @@ class SpringBootBasedTest extends AgentTestRunner {
span1.context().tags["span.kind"] == "server"
span1.context().tags["span.type"] == "web"
span1.context().tags["component"] == "java-web-servlet"
- span1.context().tags["servlet.context"] == ""
span1.context().tags["http.status_code"] == 404
span1.context().tags["thread.name"] != null
span1.context().tags["thread.id"] != null
- span1.context().tags.size() == 9
+ span1.context().tags.size() == 8
}
def "generates error spans"() {
@@ -132,7 +129,6 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["span.kind"] == "server"
span0.context().tags["span.type"] == "web"
span0.context().tags["component"] == "java-web-servlet"
- span0.context().tags["servlet.context"] == ""
span0.context().tags["http.status_code"] == 500
span0.context().tags["thread.name"] != null
span0.context().tags["thread.id"] != null
@@ -140,7 +136,7 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["error.msg"] == "Request processing failed; nested exception is java.lang.RuntimeException: qwerty"
span0.context().tags["error.type"] == NestedServletException.getName()
span0.context().tags["error.stack"] != null
- span0.context().tags.size() == 13
+ span0.context().tags.size() == 12
and: // trace 1
def trace1 = TEST_WRITER.get(1)
@@ -156,12 +152,11 @@ class SpringBootBasedTest extends AgentTestRunner {
span1.context().tags["span.kind"] == "server"
span1.context().tags["span.type"] == "web"
span1.context().tags["component"] == "java-web-servlet"
- span1.context().tags["servlet.context"] == ""
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() == 10
+ span1.context().tags.size() == 9
}
def "validated form"() {
@@ -184,11 +179,10 @@ class SpringBootBasedTest extends AgentTestRunner {
span.context().tags["span.kind"] == "server"
span.context().tags["span.type"] == "web"
span.context().tags["component"] == "java-web-servlet"
- span.context().tags["servlet.context"] == ""
span.context().tags["http.status_code"] == 200
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
- span.context().tags.size() == 9
+ span.context().tags.size() == 8
}
def "invalid form"() {
@@ -216,7 +210,6 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["span.kind"] == "server"
span0.context().tags["span.type"] == "web"
span0.context().tags["component"] == "java-web-servlet"
- span0.context().tags["servlet.context"] == ""
span0.context().tags["http.status_code"] == 400
span0.context().tags["thread.name"] != null
span0.context().tags["thread.id"] != null
@@ -224,7 +217,7 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["error.msg"].toString().startsWith("Validation failed")
span0.context().tags["error.type"] == MethodArgumentNotValidException.getName()
span0.context().tags["error.stack"] != null
- span0.context().tags.size() == 13
+ span0.context().tags.size() == 12
and: // trace 1
def trace1 = TEST_WRITER.get(1)
@@ -241,10 +234,9 @@ class SpringBootBasedTest extends AgentTestRunner {
span1.context().tags["span.kind"] == "server"
span1.context().tags["span.type"] == "web"
span1.context().tags["component"] == "java-web-servlet"
- span1.context().tags["servlet.context"] == ""
span1.context().tags["http.status_code"] == 400
span1.context().tags["thread.name"] != null
span1.context().tags["thread.id"] != null
- span1.context().tags.size() == 9
+ span1.context().tags.size() == 8
}
}

MODIFY
dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
@@ -449,10 +449,16 @@ public class DDTracer implements io.opentracing.Tracer {
// Private methods
private DDSpanBuilder withTag(final String tag, final Object value) {
+ if (value == null || (value instanceof String && ((String) value).isEmpty())) {
+ if (this.tags.containsKey(tag)) {
+ this.tags.remove(tag);
+ }
+ } else {
if (this.tags.isEmpty()) {
this.tags = new HashMap<>();
}
this.tags.put(tag, value);
+ }
return this;
}

MODIFY
dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy
dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy
@@ -82,6 +82,34 @@ class DDSpanBuilderTest extends Specification {
context.tags[DDTags.THREAD_ID] == Thread.currentThread().getId()
}
+ def "setting #name should remove"() {
+ setup:
+ final DDSpan span = tracer.buildSpan("op name")
+ .withTag(name, "tag value")
+ .withTag(name, value)
+ .start()
+
+ expect:
+ span.tags[name] == null
+
+ when:
+ span.setTag(name, "a tag")
+
+ then:
+ span.tags[name] == "a tag"
+
+ when:
+ span.setTag(name, (String) value)
+
+ then:
+ span.tags[name] == null
+
+ where:
+ name | value
+ "null.tag" | null
+ "empty.tag" | ""
+ }
+
def "should build span timestamp in nano"() {
setup:
// time in micro

Null or empty tags should remove previously set values.

Add context to servlet2 test.

Clean up some declared tracers that mess up the classpath.