diff --git a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy index 9825b95813..ec0db35692 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy @@ -1,6 +1,7 @@ import datadog.opentracing.DDSpan import datadog.opentracing.scopemanager.ContinuableScope import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.ConfigUtils import datadog.trace.api.Trace import datadog.trace.bootstrap.instrumentation.java.concurrent.CallableWrapper import datadog.trace.bootstrap.instrumentation.java.concurrent.RunnableWrapper @@ -25,7 +26,9 @@ import java.util.concurrent.TimeoutException class ExecutorInstrumentationTest extends AgentTestRunner { static { + ConfigUtils.makeConfigInstanceModifiable() System.setProperty("dd.trace.executors", "ExecutorInstrumentationTest\$CustomThreadPoolExecutor") + ConfigUtils.resetConfig() } @Shared diff --git a/dd-java-agent/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy b/dd-java-agent/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy index 1aeef8749d..57d49dde88 100644 --- a/dd-java-agent/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy +++ b/dd-java-agent/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy @@ -1,4 +1,5 @@ import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.ConfigUtils import datadog.trace.api.CorrelationIdentifier import io.opentracing.Scope import io.opentracing.util.GlobalTracer @@ -8,7 +9,9 @@ import java.util.concurrent.atomic.AtomicReference class Slf4jMDCTest extends AgentTestRunner { static { + ConfigUtils.makeConfigInstanceModifiable() System.setProperty("dd.logs.injection", "true") + ConfigUtils.resetConfig() } def "mdc shows trace and span ids for active scope"() { diff --git a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/ConfiguredTraceAnnotationsTest.groovy b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/ConfiguredTraceAnnotationsTest.groovy index 4ead07dd8d..210aba390f 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/ConfiguredTraceAnnotationsTest.groovy +++ b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/ConfiguredTraceAnnotationsTest.groovy @@ -1,4 +1,5 @@ import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.ConfigUtils import datadog.trace.instrumentation.trace_annotation.TraceAnnotationsInstrumentation import dd.test.trace.annotation.SayTracedHello @@ -10,8 +11,10 @@ import static datadog.trace.instrumentation.trace_annotation.TraceAnnotationsIns class ConfiguredTraceAnnotationsTest extends AgentTestRunner { static { + ConfigUtils.makeConfigInstanceModifiable() // nr annotation not included here, so should be disabled. System.setProperty("dd.trace.annotations", "package.Class\$Name;${OuterClass.InterestingMethod.name}") + ConfigUtils.resetConfig() } def specCleanup() { @@ -47,6 +50,7 @@ class ConfiguredTraceAnnotationsTest extends AgentTestRunner { def "test configuration #value"() { setup: def config = withSystemProperty("dd.trace.annotations", value) { + ConfigUtils.resetConfig() new TraceAnnotationsInstrumentation().additionalTraceAnnotations } diff --git a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy index 525ce9bdcf..1c13118f07 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy +++ b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy @@ -1,5 +1,6 @@ import datadog.opentracing.decorators.ErrorFlag import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.ConfigUtils import datadog.trace.api.Trace import dd.test.trace.annotation.SayTracedHello import io.opentracing.tag.Tags @@ -9,12 +10,15 @@ import java.util.concurrent.Callable class TraceAnnotationsTest extends AgentTestRunner { static { + ConfigUtils.makeConfigInstanceModifiable() System.clearProperty("dd.trace.annotations") + ConfigUtils.resetConfig(true) } def "test simple case annotations"() { setup: // Test single span in new trace + ConfigUtils.resetConfig(true) SayTracedHello.sayHello() expect: diff --git a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy index b0fc3485b0..fcf04ab0bd 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy +++ b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy @@ -1,4 +1,5 @@ import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.ConfigUtils import datadog.trace.instrumentation.trace_annotation.TraceConfigInstrumentation import java.util.concurrent.Callable @@ -8,11 +9,14 @@ import static datadog.trace.agent.test.utils.ConfigUtils.withSystemProperty class TraceConfigTest extends AgentTestRunner { static { + ConfigUtils.makeConfigInstanceModifiable() System.setProperty("dd.trace.methods", "package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]") + ConfigUtils.resetConfig() } def specCleanup() { System.clearProperty("dd.trace.methods") + ConfigUtils.resetConfig() } class ConfigTracedCallable implements Callable { @@ -44,6 +48,7 @@ class TraceConfigTest extends AgentTestRunner { setup: def config = null withSystemProperty("dd.trace.methods", value) { + ConfigUtils.resetConfig() def instrumentation = new TraceConfigInstrumentation() config = instrumentation.classMethodsToTrace } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/ConfigUtils.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/ConfigUtils.groovy index fb1a1fff16..698237bb7b 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/ConfigUtils.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/ConfigUtils.groovy @@ -20,6 +20,7 @@ class ConfigUtils { private static class ConfigInstance { // Wrapped in a static class to lazy load. static final FIELD = Config.getDeclaredField("INSTANCE") + static final RUNTIME_ID_FIELD = Config.getDeclaredField("runtimeId") } // TODO: ideally all users of this should switch to using Config object (and withConfigOverride) instead. @@ -60,15 +61,27 @@ class ConfigUtils { /** * Calling will reset the runtimeId too, so it might cause problems around runtimeId verification. + * If you are testing runtimeId provide preserveRuntimeId = false to copy the previous runtimeId + * tot he new config instance. */ - static void resetConfig() { + static void resetConfig(preserveRuntimeId = false) { // Ensure the class was retransformed properly in AgentTestRunner.makeConfigInstanceModifiable() assert Modifier.isPublic(ConfigInstance.FIELD.getModifiers()) assert Modifier.isStatic(ConfigInstance.FIELD.getModifiers()) assert Modifier.isVolatile(ConfigInstance.FIELD.getModifiers()) assert !Modifier.isFinal(ConfigInstance.FIELD.getModifiers()) - ConfigInstance.FIELD.set(null, new Config()) + assert Modifier.isPublic(ConfigInstance.RUNTIME_ID_FIELD.getModifiers()) + assert !Modifier.isStatic(ConfigInstance.RUNTIME_ID_FIELD.getModifiers()) + assert Modifier.isVolatile(ConfigInstance.RUNTIME_ID_FIELD.getModifiers()) + assert !Modifier.isFinal(ConfigInstance.RUNTIME_ID_FIELD.getModifiers()) + + def previousConfig = ConfigInstance.FIELD.get(null) + def newConfig = new Config() + ConfigInstance.FIELD.set(null, newConfig) + if (previousConfig != null && preserveRuntimeId) { + ConfigInstance.RUNTIME_ID_FIELD.set(newConfig, ConfigInstance.RUNTIME_ID_FIELD.get(previousConfig)) + } } static void makeConfigInstanceModifiable() { @@ -88,6 +101,12 @@ class ConfigUtils { .field(named("INSTANCE")) .transform(Transformer.ForField.withModifiers(PUBLIC, STATIC, VOLATILE)) } + // Making runtimeId modifiable so that it can be preserved when resetting config in tests + .transform { builder, typeDescription, classLoader, module -> + builder + .field(named("runtimeId")) + .transform(Transformer.ForField.withModifiers(PUBLIC, VOLATILE)) + } .installOn(instrumentation) final field = ConfigInstance.FIELD @@ -96,6 +115,12 @@ class ConfigUtils { assert Modifier.isVolatile(field.getModifiers()) assert !Modifier.isFinal(field.getModifiers()) + final runtimeIdField = ConfigInstance.RUNTIME_ID_FIELD + assert Modifier.isPublic(runtimeIdField.getModifiers()) + assert !Modifier.isStatic(ConfigInstance.RUNTIME_ID_FIELD.getModifiers()) + assert Modifier.isVolatile(runtimeIdField.getModifiers()) + assert !Modifier.isFinal(runtimeIdField.getModifiers()) + // No longer needed (Unless class gets retransformed somehow). instrumentation.removeTransformer(transformer) }