More fixes.

Notably added a transformer to make config easier to test with by making INSTANCE public static volatile.
This commit is contained in:
Tyler Benson 2019-05-29 22:24:19 -07:00
parent fc9f1d120c
commit a7c941c2ea
6 changed files with 83 additions and 22 deletions

View File

@ -28,6 +28,7 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest<AkkaHttpClientDec
.addHeaders(headers.collect { RawHeader.create(it.key, it.value) }) .addHeaders(headers.collect { RawHeader.create(it.key, it.value) })
def response = Http.get(system).singleRequest(request, materializer).toCompletableFuture().get() def response = Http.get(system).singleRequest(request, materializer).toCompletableFuture().get()
blockUntilChildSpansFinished(1)
callback?.call() callback?.call()
return response.status().intValue() return response.status().intValue()
} }

View File

@ -3,6 +3,7 @@ import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator
import javax.ws.rs.client.AsyncInvoker import javax.ws.rs.client.AsyncInvoker
import javax.ws.rs.client.Client import javax.ws.rs.client.Client
import javax.ws.rs.client.ClientBuilder import javax.ws.rs.client.ClientBuilder
import javax.ws.rs.client.Entity
import javax.ws.rs.client.WebTarget import javax.ws.rs.client.WebTarget
import javax.ws.rs.core.MediaType import javax.ws.rs.core.MediaType
import javax.ws.rs.core.Response import javax.ws.rs.core.Response
@ -19,8 +20,9 @@ abstract class JaxRsClientAsyncTest extends HttpClientTest<JaxRsClientDecorator>
def builder = service.request(MediaType.TEXT_PLAIN) def builder = service.request(MediaType.TEXT_PLAIN)
headers.each { builder.header(it.key, it.value) } headers.each { builder.header(it.key, it.value) }
AsyncInvoker request = builder.async() AsyncInvoker request = builder.async()
Response response = request.method(method).get() def body = BODY_METHODS.contains(method) ? Entity.text("") : null
Response response = request.method(method, (Entity) body).get()
callback?.call() callback?.call()
return response.status return response.status

View File

@ -2,6 +2,7 @@ import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator
import javax.ws.rs.client.Client import javax.ws.rs.client.Client
import javax.ws.rs.client.ClientBuilder import javax.ws.rs.client.ClientBuilder
import javax.ws.rs.client.Entity
import javax.ws.rs.client.Invocation import javax.ws.rs.client.Invocation
import javax.ws.rs.client.WebTarget import javax.ws.rs.client.WebTarget
import javax.ws.rs.core.MediaType import javax.ws.rs.core.MediaType
@ -19,7 +20,8 @@ abstract class JaxRsClientTest extends HttpClientTest<JaxRsClientDecorator> {
WebTarget service = client.target(uri) WebTarget service = client.target(uri)
Invocation.Builder request = service.request(MediaType.TEXT_PLAIN) Invocation.Builder request = service.request(MediaType.TEXT_PLAIN)
headers.each { request.header(it.key, it.value) } headers.each { request.header(it.key, it.value) }
Response response = request.method(method) def body = BODY_METHODS.contains(method) ? Entity.text("") : null
Response response = request.method(method, (Entity) body)
callback?.call() callback?.call()
return response.status return response.status

View File

@ -1,5 +1,8 @@
package datadog.trace.agent.test; package datadog.trace.agent.test;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.none;
import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.Logger;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
@ -10,6 +13,7 @@ import datadog.trace.agent.test.asserts.ListWriterAssert;
import datadog.trace.agent.test.utils.GlobalTracerUtils; import datadog.trace.agent.test.utils.GlobalTracerUtils;
import datadog.trace.agent.tooling.AgentInstaller; import datadog.trace.agent.tooling.AgentInstaller;
import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.Config;
import datadog.trace.api.GlobalTracer; import datadog.trace.api.GlobalTracer;
import datadog.trace.common.writer.ListWriter; import datadog.trace.common.writer.ListWriter;
import datadog.trace.common.writer.Writer; import datadog.trace.common.writer.Writer;
@ -20,6 +24,8 @@ import groovy.transform.stc.SimpleType;
import io.opentracing.Tracer; import io.opentracing.Tracer;
import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.Instrumentation; import java.lang.instrument.Instrumentation;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.List; import java.util.List;
import java.util.ServiceLoader; import java.util.ServiceLoader;
import java.util.Set; import java.util.Set;
@ -28,8 +34,14 @@ import java.util.concurrent.atomic.AtomicInteger;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import net.bytebuddy.agent.ByteBuddyAgent; import net.bytebuddy.agent.ByteBuddyAgent;
import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.agent.builder.ResettableClassFileTransformer;
import net.bytebuddy.description.modifier.FieldManifestation;
import net.bytebuddy.description.modifier.Ownership;
import net.bytebuddy.description.modifier.Visibility;
import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.dynamic.ClassFileLocator;
import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.DynamicType;
import net.bytebuddy.dynamic.Transformer;
import net.bytebuddy.utility.JavaModule; import net.bytebuddy.utility.JavaModule;
import org.junit.AfterClass; import org.junit.AfterClass;
import org.junit.Before; import org.junit.Before;
@ -83,6 +95,12 @@ public abstract class AgentTestRunner extends Specification {
((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.WARN); ((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.WARN);
((Logger) LoggerFactory.getLogger("datadog")).setLevel(Level.DEBUG); ((Logger) LoggerFactory.getLogger("datadog")).setLevel(Level.DEBUG);
try {
makeConfigInstanceModifiable();
} catch (final Exception e) {
throw new RuntimeException(e);
}
TEST_WRITER = TEST_WRITER =
new ListWriter() { new ListWriter() {
@Override @Override
@ -188,6 +206,44 @@ public abstract class AgentTestRunner extends Specification {
: INSTRUMENTATION_ERROR_COUNT.get() + " Instrumentation errors during test"; : INSTRUMENTATION_ERROR_COUNT.get() + " Instrumentation errors during test";
} }
private static void makeConfigInstanceModifiable()
throws ClassNotFoundException, NoSuchFieldException {
final ResettableClassFileTransformer transformer =
new AgentBuilder.Default()
// Config is injected into the bootstrap, so we need to provide a locator.
.with(
new AgentBuilder.LocationStrategy.Simple(
ClassFileLocator.ForClassLoader.ofSystemLoader()))
.ignore(none()) // Allow transforming boostrap classes
.type(named("datadog.trace.api.Config"))
.transform(
new AgentBuilder.Transformer() {
@Override
public DynamicType.Builder<?> transform(
final DynamicType.Builder<?> builder,
final TypeDescription typeDescription,
final ClassLoader classLoader,
final JavaModule module) {
// Add transformer to modify INSTANCE field access.
return builder
.field(named("INSTANCE"))
.transform(
Transformer.ForField.withModifiers(
Visibility.PUBLIC, Ownership.STATIC, FieldManifestation.VOLATILE));
}
})
.installOn(INSTRUMENTATION);
final Field field = Config.class.getDeclaredField("INSTANCE");
assert Modifier.isPublic(field.getModifiers());
assert Modifier.isStatic(field.getModifiers());
assert Modifier.isVolatile(field.getModifiers());
assert !Modifier.isFinal(field.getModifiers());
// No longer needed (Unless class gets retransformed somehow).
INSTRUMENTATION.removeTransformer(transformer);
}
public static void assertTraces( public static void assertTraces(
final int size, final int size,
@ClosureParams( @ClosureParams(

View File

@ -68,7 +68,7 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
} }
@Unroll @Unroll
def "basic #method request #url"() { def "basic #method request #url - tagQueryString=#tagQueryString"() {
when: when:
def status = withConfigOverride(Config.HTTP_CLIENT_TAG_QUERY_STRING, "$tagQueryString") { def status = withConfigOverride(Config.HTTP_CLIENT_TAG_QUERY_STRING, "$tagQueryString") {
doRequest(method, url) doRequest(method, url)

View File

@ -7,11 +7,12 @@ import io.opentracing.Scope
import io.opentracing.util.GlobalTracer import io.opentracing.util.GlobalTracer
import lombok.SneakyThrows import lombok.SneakyThrows
import java.lang.reflect.Field
import java.lang.reflect.Modifier import java.lang.reflect.Modifier
import java.util.concurrent.Callable import java.util.concurrent.Callable
class TraceUtils { class TraceUtils {
static final def CONFIG_INSTANCE_FIELD = Config.getDeclaredField("INSTANCE")
private static final BaseDecorator DECORATOR = new BaseDecorator() { private static final BaseDecorator DECORATOR = new BaseDecorator() {
protected String[] instrumentationNames() { protected String[] instrumentationNames() {
return new String[0] return new String[0]
@ -61,14 +62,21 @@ class TraceUtils {
@SneakyThrows @SneakyThrows
synchronized static <T extends Object> Object withConfigOverride(final String name, final String value, final Callable<T> r) { synchronized static <T extends Object> Object withConfigOverride(final String name, final String value, final Callable<T> r) {
def existingConfig = Config.get() // We can't reference INSTANCE directly or the reflection below will fail. // Ensure the class was retransformed properly in AgentTestRunner.makeConfigInstanceModifiable()
assert Modifier.isPublic(CONFIG_INSTANCE_FIELD.getModifiers())
assert Modifier.isStatic(CONFIG_INSTANCE_FIELD.getModifiers())
assert Modifier.isVolatile(CONFIG_INSTANCE_FIELD.getModifiers())
assert !Modifier.isFinal(CONFIG_INSTANCE_FIELD.getModifiers())
def existingConfig = Config.get()
Properties properties = new Properties() Properties properties = new Properties()
properties.put(name, value) properties.put(name, value)
setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config(properties, existingConfig)) CONFIG_INSTANCE_FIELD.set(null, new Config(properties, existingConfig))
assert Config.get() != existingConfig
try { try {
return r.call() return r.call()
} finally { } finally {
setFinalStatic(Config.getDeclaredField("INSTANCE"), existingConfig) CONFIG_INSTANCE_FIELD.set(null, existingConfig)
} }
} }
@ -76,20 +84,12 @@ class TraceUtils {
* Calling will reset the runtimeId too, so it might cause problems around runtimeId verification. * Calling will reset the runtimeId too, so it might cause problems around runtimeId verification.
*/ */
static void resetConfig() { static void resetConfig() {
setFinalStatic(Config.getDeclaredField("INSTANCE"), new Config()) // Ensure the class was retransformed properly in AgentTestRunner.makeConfigInstanceModifiable()
} assert Modifier.isPublic(CONFIG_INSTANCE_FIELD.getModifiers())
assert Modifier.isStatic(CONFIG_INSTANCE_FIELD.getModifiers())
assert Modifier.isVolatile(CONFIG_INSTANCE_FIELD.getModifiers())
assert !Modifier.isFinal(CONFIG_INSTANCE_FIELD.getModifiers())
private static void setFinalStatic(final Field field, final Object newValue) throws Exception { CONFIG_INSTANCE_FIELD.set(null, new Config())
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.getDeclaredField("modifiers")
modifiersField.setAccessible(true)
modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL)
field.set(instance, newValue)
} }
} }