Encapsulate config override reflection
MODIFY
dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy
dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy
@@ -19,10 +19,8 @@ import spock.lang.Shared
import java.util.concurrent.CompletionStage
import java.util.concurrent.ExecutionException
-import static datadog.trace.agent.test.TestUtils.setFinal
-import static datadog.trace.agent.test.TestUtils.setFinalStatic
-import static datadog.trace.agent.test.TestUtils.withSystemProperty
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
@@ -62,8 +60,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
HttpRequest request = HttpRequest.create(url.toString())
when:
- HttpResponse response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ HttpResponse response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
Http.get(system)
.singleRequest(request, materializer)
.toCompletableFuture().get()
@@ -118,8 +115,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
HttpRequest request = HttpRequest.create(url.toString())
CompletionStage<HttpResponse> responseFuture =
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
Http.get(system)
.singleRequest(request, materializer)
}
@@ -200,8 +196,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
.runWith(Sink.<Pair<Try<HttpResponse>, Integer>> head(), materializer)
when:
- HttpResponse response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ HttpResponse response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
sink.toCompletableFuture().get().first().get()
}
String message = readMessage(response)
@@ -253,8 +248,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
// Use port number that really should be closed
def url = new URL("http://localhost:$UNUSED_PORT/test")
- def response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ def response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
Source
.<Pair<HttpRequest, Integer>> single(new Pair(HttpRequest.create(url.toString()), 1))
.via(pool)
@@ -299,11 +293,4 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
String readMessage(HttpResponse response) {
response.entity().toStrict(TIMEOUT, materializer).toCompletableFuture().get().getData().utf8String()
}
-
- def resetConfig() {
- def runtimeId = Config.get().runtimeId
- setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
- setFinal(Config.getDeclaredField("runtimeId"), Config.get(), runtimeId)
- }
-
}

MODIFY
dd-java-agent/instrumentation/apache-httpclient-4/src/latestDepTest/groovy/ApacheHttpClientTest.groovy
dd-java-agent/instrumentation/apache-httpclient-4/src/latestDepTest/groovy/ApacheHttpClientTest.groovy
@@ -16,11 +16,9 @@ import org.apache.http.message.BasicHeader
import spock.lang.AutoCleanup
import spock.lang.Shared
-import static datadog.trace.agent.test.TestUtils.runUnderTrace
-import static datadog.trace.agent.test.TestUtils.setFinal
-import static datadog.trace.agent.test.TestUtils.setFinalStatic
-import static datadog.trace.agent.test.TestUtils.withSystemProperty
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
class ApacheHttpClientTest extends AgentTestRunner {
@@ -60,9 +58,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
def "trace request with propagation"() {
when:
- String response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ String response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
if (responseHandler) {
client.execute(new HttpGet(successUrl), responseHandler)
@@ -97,9 +93,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
request.setConfig(requestConfigBuilder.build())
when:
- HttpResponse response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ HttpResponse response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
client.execute(request)
}
@@ -129,9 +123,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
request.setConfig(requestConfigBuilder.build())
when:
- HttpResponse response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ HttpResponse response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
client.execute(request)
}
@@ -162,9 +154,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
request.setConfig(requestConfigBuilder.build())
when:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
client.execute(request)
}
@@ -192,9 +182,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
request.addHeader(new BasicHeader("is-dd-server", "false"))
when:
- HttpResponse response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ HttpResponse response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
client.execute(request)
}
@@ -253,10 +241,4 @@ class ApacheHttpClientTest extends AgentTestRunner {
}
}
}
-
- def resetConfig() {
- def runtimeId = Config.get().runtimeId
- setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
- setFinal(Config.getDeclaredField("runtimeId"), Config.get(), runtimeId)
- }
}

MODIFY
dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy
dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy
@@ -14,11 +14,9 @@ import org.apache.http.message.BasicHeader
import spock.lang.AutoCleanup
import spock.lang.Shared
-import static datadog.trace.agent.test.TestUtils.runUnderTrace
-import static datadog.trace.agent.test.TestUtils.setFinal
-import static datadog.trace.agent.test.TestUtils.setFinalStatic
-import static datadog.trace.agent.test.TestUtils.withSystemProperty
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
class ApacheHttpClientTest extends AgentTestRunner {
@@ -57,8 +55,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
def "trace request with propagation"() {
when:
- String response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ String response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
if (responseHandler) {
client.execute(new HttpGet(successUrl), responseHandler)
@@ -90,8 +87,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
request.addHeader(new BasicHeader("is-dd-server", "false"))
when:
- HttpResponse response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ HttpResponse response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("parent") {
client.execute(request)
}
@@ -150,10 +146,4 @@ class ApacheHttpClientTest extends AgentTestRunner {
}
}
}
-
- def resetConfig() {
- def runtimeId = Config.get().runtimeId
- setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
- setFinal(Config.getDeclaredField("runtimeId"), Config.get(), runtimeId)
- }
}

MODIFY
dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy
dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy
@@ -8,11 +8,9 @@ import org.springframework.web.client.RestTemplate
import spock.lang.AutoCleanup
import spock.lang.Shared
-import static datadog.trace.agent.test.TestUtils.runUnderTrace
-import static datadog.trace.agent.test.TestUtils.setFinal
-import static datadog.trace.agent.test.TestUtils.setFinalStatic
-import static datadog.trace.agent.test.TestUtils.withSystemProperty
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.COMPONENT_NAME
import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME
@@ -35,9 +33,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "trace request with propagation (useCaches: #useCaches)"() {
setup:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
HttpURLConnection connection = server.address.toURL().openConnection()
connection.useCaches = useCaches
@@ -118,9 +114,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "trace request without propagation (useCaches: #useCaches)"() {
setup:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
HttpURLConnection connection = server.address.toURL().openConnection()
connection.useCaches = useCaches
@@ -201,9 +195,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "test response code"() {
setup:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
HttpURLConnection connection = server.address.toURL().openConnection()
connection.setRequestMethod("HEAD")
@@ -250,9 +242,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "test broken API usage"() {
setup:
- HttpURLConnection conn = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ HttpURLConnection conn = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
HttpURLConnection connection = server.address.toURL().openConnection()
connection.setRequestProperty("Connection", "close")
@@ -304,9 +294,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "test post request"() {
setup:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
HttpURLConnection connection = server.address.toURL().openConnection()
connection.setRequestMethod("POST")
@@ -395,9 +383,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "top level httpurlconnection tracing disabled"() {
setup:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
HttpURLConnection connection = server.address.toURL().openConnection()
connection.addRequestProperty("is-dd-server", "false")
def stream = connection.inputStream
@@ -436,9 +422,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
def "rest template"() {
setup:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
RestTemplate restTemplate = new RestTemplate()
String res = restTemplate.postForObject(server.address.toString(), "Hello", String)
@@ -481,10 +465,4 @@ class HttpUrlConnectionTest extends AgentTestRunner {
where:
renameService << [false, true]
}
-
- def resetConfig() {
- def runtimeId = Config.get().runtimeId
- setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
- setFinal(Config.getDeclaredField("runtimeId"), Config.get(), runtimeId)
- }
}

MODIFY
dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy
dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy
import datadog.trace.agent.test.AgentTestRunner
-import datadog.trace.agent.test.TestUtils
import datadog.trace.api.Config
import datadog.trace.api.DDSpanTypes
import datadog.trace.api.DDTags
@@ -7,22 +6,18 @@ import datadog.trace.instrumentation.http_url_connection.UrlInstrumentation
import io.opentracing.tag.Tags
import io.opentracing.util.GlobalTracer
-import static datadog.trace.agent.test.TestUtils.runUnderTrace
-import static datadog.trace.agent.test.TestUtils.setFinal
-import static datadog.trace.agent.test.TestUtils.setFinalStatic
-import static datadog.trace.agent.test.TestUtils.withSystemProperty
+import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.COMPONENT_NAME
import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME
class UrlConnectionTest extends AgentTestRunner {
- private static final int INVALID_PORT = TestUtils.randomOpenPort()
+ private static final int UNUSED_PORT = 61 // this port should always be closed
def "trace request with connection failure #scheme"() {
when:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
URLConnection connection = url.openConnection()
connection.setConnectTimeout(10000)
@@ -59,7 +54,7 @@ class UrlConnectionTest extends AgentTestRunner {
"$Tags.HTTP_URL.key" "$url"
"$Tags.HTTP_METHOD.key" "GET"
"$Tags.PEER_HOSTNAME.key" "localhost"
- "$Tags.PEER_PORT.key" INVALID_PORT
+ "$Tags.PEER_PORT.key" UNUSED_PORT
errorTags ConnectException, String
defaultTags()
}
@@ -72,7 +67,7 @@ class UrlConnectionTest extends AgentTestRunner {
"http" | true
"https" | false
- url = new URI("$scheme://localhost:$INVALID_PORT").toURL()
+ url = new URI("$scheme://localhost:$UNUSED_PORT").toURL()
}
def "trace request with connection failure to a local file with broken url path"() {
@@ -80,9 +75,7 @@ class UrlConnectionTest extends AgentTestRunner {
def url = new URI("file:/some-random-file%abc").toURL()
when:
- withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
-
+ withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
runUnderTrace("someTrace") {
url.openConnection()
}
@@ -125,10 +118,4 @@ class UrlConnectionTest extends AgentTestRunner {
where:
renameService << [false, true]
}
-
- def resetConfig() {
- def runtimeId = Config.get().runtimeId
- setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
- setFinal(Config.getDeclaredField("runtimeId"), Config.get(), runtimeId)
- }
}

MODIFY
dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy
dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy
@@ -5,10 +5,8 @@ import io.opentracing.tag.Tags
import okhttp3.OkHttpClient
import okhttp3.Request
-import static datadog.trace.agent.test.TestUtils.setFinal
-import static datadog.trace.agent.test.TestUtils.setFinalStatic
-import static datadog.trace.agent.test.TestUtils.withSystemProperty
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
class OkHttp3Test extends AgentTestRunner {
@@ -26,8 +24,7 @@ class OkHttp3Test extends AgentTestRunner {
.url("http://localhost:$server.address.port/ping")
.build()
- def response = withSystemProperty("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
- resetConfig()
+ def response = withConfigOverride("dd.$Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN", "$renameService") {
client.newCall(request).execute()
}
@@ -79,10 +76,4 @@ class OkHttp3Test extends AgentTestRunner {
where:
renameService << [false, true]
}
-
- def resetConfig() {
- def runtimeId = Config.get().runtimeId
- setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
- setFinal(Config.getDeclaredField("runtimeId"), Config.get(), runtimeId)
- }
}

ADD
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy
+package datadog.trace.agent.test.utils
+
+import datadog.trace.api.Config
+import datadog.trace.context.TraceScope
+import io.opentracing.Scope
+import io.opentracing.Span
+import io.opentracing.tag.Tags
+import io.opentracing.util.GlobalTracer
+import lombok.SneakyThrows
+
+import java.lang.reflect.Field
+import java.lang.reflect.Modifier
+import java.util.concurrent.Callable
+
+import static io.opentracing.log.Fields.ERROR_OBJECT
+
+class TraceUtils {
+
+ @SneakyThrows
+ static <T extends Object> Object runUnderTrace(final String rootOperationName, final Callable<T> r) {
+ final Scope scope = GlobalTracer.get().buildSpan(rootOperationName).startActive(true)
+ ((TraceScope) scope).setAsyncPropagation(true)
+
+ try {
+ return r.call()
+ } catch (final Exception e) {
+ final Span span = scope.span()
+ Tags.ERROR.set(span, true)
+ span.log(Collections.singletonMap(ERROR_OBJECT, e))
+
+ throw e
+ } finally {
+ ((TraceScope) scope).setAsyncPropagation(false)
+ scope.close()
+ }
+ }
+
+ @SneakyThrows
+ static <T extends Object> Object withSystemProperty(final String name, final String value, final Callable<T> r) {
+ if (value == null) {
+ System.clearProperty(name)
+ } else {
+ System.setProperty(name, value)
+ }
+ try {
+ return r.call()
+ } finally {
+ System.clearProperty(name)
+ }
+ }
+
+ @SneakyThrows
+ static <T extends Object> Object withConfigOverride(final String name, final String value, final Callable<T> r) {
+ return withSystemProperty(name, value) {
+ def existingConfig = Config.get() // We can't reference INSTANCE directly or the reflection below will fail.
+ setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config())
+ setFinal(Config.getDeclaredField("runtimeId"), Config.get(), existingConfig.runtimeId)
+ try {
+ return r.call()
+ } finally {
+ setFinalStatic(Config.getDeclaredField("INSTANCE"), existingConfig)
+ }
+ }
+ }
+
+ private static void setFinalStatic(final Field field, final Object newValue) throws Exception {
+ setFinal(field, null, newValue)
+ }
+
+ private static void setFinal(final Field field, final Object instance, final Object newValue) throws Exception {
+ field.setAccessible(true)
+
+ final Field modifiersField = Field.class.getDeclaredField("modifiers")
+ modifiersField.setAccessible(true)
+ modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL)
+
+ field.set(instance, newValue)
+ }
+}

Encapsulate config override reflection