Improve AgentRunner and ConfigUtils class based on CR comments

This commit is contained in:
Luca Abbati 2019-06-13 12:40:10 +02:00
parent a522196b49
commit a8dd35ef57
No known key found for this signature in database
GPG Key ID: 74DBB952D9BA17F2
3 changed files with 15 additions and 14 deletions

View File

@ -13,7 +13,6 @@ class TraceAnnotationsTest extends AgentTestRunner {
ConfigUtils.updateConfig { ConfigUtils.updateConfig {
System.clearProperty("dd.trace.annotations") System.clearProperty("dd.trace.annotations")
} }
refreshTestTracer()
} }
def "test simple case annotations"() { def "test simple case annotations"() {

View File

@ -64,12 +64,12 @@ public abstract class AgentTestRunner extends Specification {
* *
* <p>Before the start of each test the reported traces will be reset. * <p>Before the start of each test the reported traces will be reset.
*/ */
public static ListWriter TEST_WRITER; public static final ListWriter TEST_WRITER;
// having a reference to io.opentracing.Tracer in test field // having a reference to io.opentracing.Tracer in test field
// loads opentracing before bootstrap classpath is setup // loads opentracing before bootstrap classpath is setup
// so we declare tracer as an object and cast when needed. // so we declare tracer as an object and cast when needed.
protected static Object TEST_TRACER; protected static final Object TEST_TRACER;
protected static final Set<String> TRANSFORMED_CLASSES = Sets.newConcurrentHashSet(); protected static final Set<String> TRANSFORMED_CLASSES = Sets.newConcurrentHashSet();
private static final AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger(); private static final AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger();
@ -85,10 +85,6 @@ public abstract class AgentTestRunner extends Specification {
((Logger) LoggerFactory.getLogger("datadog")).setLevel(Level.DEBUG); ((Logger) LoggerFactory.getLogger("datadog")).setLevel(Level.DEBUG);
ConfigUtils.makeConfigInstanceModifiable(); ConfigUtils.makeConfigInstanceModifiable();
refreshTestTracer();
}
public static void refreshTestTracer() {
TEST_WRITER = TEST_WRITER =
new ListWriter() { new ListWriter() {
@Override @Override

View File

@ -56,11 +56,9 @@ class ConfigUtils {
} }
/** /**
* Calling will reset the runtimeId too, so it might cause problems around runtimeId verification. * Reset the global configuration. Please note that Runtime ID is preserved to the pre-existing value.
* If you are testing runtimeId provide <code>preserveRuntimeId = false</code> to copy the previous runtimeId
* to the new config instance.
*/ */
static void resetConfig(preserveRuntimeId = true) { static void resetConfig() {
// Ensure the class was re-transformed properly in AgentTestRunner.makeConfigInstanceModifiable() // Ensure the class was re-transformed properly in AgentTestRunner.makeConfigInstanceModifiable()
assert Modifier.isPublic(ConfigInstance.FIELD.getModifiers()) assert Modifier.isPublic(ConfigInstance.FIELD.getModifiers())
assert Modifier.isStatic(ConfigInstance.FIELD.getModifiers()) assert Modifier.isStatic(ConfigInstance.FIELD.getModifiers())
@ -75,22 +73,29 @@ class ConfigUtils {
def previousConfig = ConfigInstance.FIELD.get(null) def previousConfig = ConfigInstance.FIELD.get(null)
def newConfig = new Config() def newConfig = new Config()
ConfigInstance.FIELD.set(null, newConfig) ConfigInstance.FIELD.set(null, newConfig)
if (previousConfig != null && preserveRuntimeId) { if (previousConfig != null) {
ConfigInstance.RUNTIME_ID_FIELD.set(newConfig, ConfigInstance.RUNTIME_ID_FIELD.get(previousConfig)) ConfigInstance.RUNTIME_ID_FIELD.set(newConfig, ConfigInstance.RUNTIME_ID_FIELD.get(previousConfig))
} }
} }
// Keep track of config instance already made modifiable
private static isConfigInstanceModifiable = false
static void makeConfigInstanceModifiable() { static void makeConfigInstanceModifiable() {
if (isConfigInstanceModifiable) {
return
}
def instrumentation = ByteBuddyAgent.install() def instrumentation = ByteBuddyAgent.install()
final transformer = final transformer =
new AgentBuilder.Default() new AgentBuilder.Default()
.with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
.with(AgentBuilder.RedefinitionStrategy.Listener.ErrorEscalating.FAIL_FAST) .with(AgentBuilder.RedefinitionStrategy.Listener.ErrorEscalating.FAIL_FAST)
// Config is injected into the bootstrap, so we need to provide a locator. // Config is injected into the bootstrap, so we need to provide a locator.
.with( .with(
new AgentBuilder.LocationStrategy.Simple( new AgentBuilder.LocationStrategy.Simple(
ClassFileLocator.ForClassLoader.ofSystemLoader())) ClassFileLocator.ForClassLoader.ofSystemLoader()))
.ignore(none()) // Allow transforming boostrap classes .ignore(none()) // Allow transforming bootstrap classes
.type(named("datadog.trace.api.Config")) .type(named("datadog.trace.api.Config"))
.transform { builder, typeDescription, classLoader, module -> .transform { builder, typeDescription, classLoader, module ->
builder builder
@ -104,6 +109,7 @@ class ConfigUtils {
.transform(Transformer.ForField.withModifiers(PUBLIC, VOLATILE)) .transform(Transformer.ForField.withModifiers(PUBLIC, VOLATILE))
} }
.installOn(instrumentation) .installOn(instrumentation)
isConfigInstanceModifiable = true
final field = ConfigInstance.FIELD final field = ConfigInstance.FIELD
assert Modifier.isPublic(field.getModifiers()) assert Modifier.isPublic(field.getModifiers())