Review changes.

This commit is contained in:
Tyler Benson 2019-02-27 11:13:14 -08:00
parent 1644de3969
commit a413b0d08d
6 changed files with 118 additions and 45 deletions

View File

@ -84,10 +84,17 @@ public abstract class BaseDecorator {
return span;
}
/**
* This method is used to generate an acceptable span (operation) name based on a given method
* reference. Anonymous classes are named based on their parent.
*
* @param method
* @return
*/
public String spanNameForMethod(final Method method) {
final Class<?> declaringClass = method.getDeclaringClass();
String className = declaringClass.getSimpleName();
if (className.isEmpty()) {
String className;
if (declaringClass.isAnonymousClass()) {
className = declaringClass.getName();
if (declaringClass.getPackage() != null) {
final String pkgName = declaringClass.getPackage().getName();
@ -95,6 +102,8 @@ public abstract class BaseDecorator {
className = declaringClass.getName().replace(pkgName, "").substring(1);
}
}
} else {
className = declaringClass.getSimpleName();
}
return className + "." + method.getName();
}

View File

@ -154,6 +154,22 @@ class BaseDecoratorTest extends Specification {
true | "test2" | "" | true | 1.0
}
def "test spanNameForMethod"() {
when:
def result = decorator.spanNameForMethod(method)
then:
result == "${name}.run"
where:
target | name
SomeInnerClass | "SomeInnerClass"
SomeNestedClass | "SomeNestedClass"
SampleJavaClass.anonymousClass | "SampleJavaClass\$1"
method = target.getDeclaredMethod("run")
}
def newDecorator() {
return newDecorator(false)
}
@ -218,4 +234,12 @@ class BaseDecoratorTest extends Specification {
}
}
}
class SomeInnerClass implements Runnable {
void run() {}
}
static class SomeNestedClass implements Runnable {
void run() {}
}
}

View File

@ -0,0 +1,14 @@
package datadog.trace.agent.decorator;
/**
* Used by {@link BaseDecoratorTest}. Groovy with Java 10+ doesn't seem to treat it properly as an
* anonymous class, so use a Java class instead.
*/
public class SampleJavaClass {
public static Class anonymousClass =
new Runnable() {
@Override
public void run() {}
}.getClass();
}

View File

@ -1,14 +1,16 @@
package datadog.trace.agent.tooling
import datadog.trace.bootstrap.WeakMap
import datadog.trace.util.gc.GCUtils
import spock.lang.Retry
import spock.lang.Shared
import spock.lang.Specification
import java.lang.ref.WeakReference
import java.util.concurrent.TimeUnit
@Retry
// These tests fail sometimes in CI.
class WeakConcurrentSupplierTest extends Specification {
@Shared
def weakConcurrentSupplier = new WeakMapSuppliers.WeakConcurrent()

View File

@ -19,7 +19,7 @@ import javax.ws.rs.Path;
public class JaxRsAnnotationsDecorator extends BaseDecorator {
public static JaxRsAnnotationsDecorator DECORATE = new JaxRsAnnotationsDecorator();
private final WeakMap<Class, Map<String, String>> resourceNames = newWeakMap();
private final WeakMap<Class, Map<Method, String>> resourceNames = newWeakMap();
@Override
protected String[] instrumentationNames() {
@ -43,57 +43,74 @@ public class JaxRsAnnotationsDecorator extends BaseDecorator {
final Span span = scope.span();
Tags.COMPONENT.set(span, "jax-rs");
Class<?> target = method.getDeclaringClass();
Map<String, String> classMap = resourceNames.get(target);
final Class<?> target = method.getDeclaringClass();
Map<Method, String> classMap = resourceNames.get(target);
if (classMap == null) {
resourceNames.putIfAbsent(target, new ConcurrentHashMap<String, String>());
resourceNames.putIfAbsent(target, new ConcurrentHashMap<Method, String>());
classMap = resourceNames.get(target);
// classMap should not be null at this point because we have a
// strong reference to target and don't manually clear the map.
}
final String methodName = method.toString();
String resourceName = classMap.get(methodName);
String resourceName = classMap.get(method);
if (resourceName == null) {
final LinkedList<Path> paths = new LinkedList<>();
while (target != Object.class) {
final Path annotation = target.getAnnotation(Path.class);
if (annotation != null) {
paths.push(annotation);
}
target = target.getSuperclass();
}
final Path methodPath = method.getAnnotation(Path.class);
if (methodPath != null) {
paths.add(methodPath);
}
String httpMethod = null;
for (final Annotation ann : method.getDeclaredAnnotations()) {
if (ann.annotationType().getAnnotation(HttpMethod.class) != null) {
httpMethod = ann.annotationType().getSimpleName();
}
}
final StringBuilder resourceNameBuilder = new StringBuilder();
if (httpMethod != null) {
resourceNameBuilder.append(httpMethod);
resourceNameBuilder.append(" ");
}
Path last = null;
for (final Path path : paths) {
if (path.value().startsWith("/") || (last != null && last.value().endsWith("/"))) {
resourceNameBuilder.append(path.value());
} else {
resourceNameBuilder.append("/");
resourceNameBuilder.append(path.value());
}
last = path;
}
resourceName = resourceNameBuilder.toString().trim();
classMap.put(methodName, resourceName);
final String httpMethod = locateHttpMethod(method);
final LinkedList<Path> paths = gatherPaths(method);
resourceName = buildResourceName(httpMethod, paths);
classMap.put(method, resourceName);
}
if (!resourceName.isEmpty()) {
span.setTag(DDTags.RESOURCE_NAME, resourceName);
}
}
private String locateHttpMethod(final Method method) {
String httpMethod = null;
for (final Annotation ann : method.getDeclaredAnnotations()) {
if (ann.annotationType().getAnnotation(HttpMethod.class) != null) {
httpMethod = ann.annotationType().getSimpleName();
}
}
return httpMethod;
}
private LinkedList<Path> gatherPaths(final Method method) {
Class<?> target = method.getDeclaringClass();
final LinkedList<Path> paths = new LinkedList<>();
while (target != Object.class) {
final Path annotation = target.getAnnotation(Path.class);
if (annotation != null) {
paths.push(annotation);
}
target = target.getSuperclass();
}
final Path methodPath = method.getAnnotation(Path.class);
if (methodPath != null) {
paths.add(methodPath);
}
return paths;
}
private String buildResourceName(final String httpMethod, final LinkedList<Path> paths) {
final String resourceName;
final StringBuilder resourceNameBuilder = new StringBuilder();
if (httpMethod != null) {
resourceNameBuilder.append(httpMethod);
resourceNameBuilder.append(" ");
}
Path last = null;
for (final Path path : paths) {
if (path.value().startsWith("/") || (last != null && last.value().endsWith("/"))) {
resourceNameBuilder.append(path.value());
} else {
resourceNameBuilder.append("/");
resourceNameBuilder.append(path.value());
}
last = path;
}
resourceName = resourceNameBuilder.toString().trim();
return resourceName;
}
}

View File

@ -32,6 +32,13 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default
.or(safeHasSuperType(declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path"))))));
}
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.agent.decorator.BaseDecorator", packageName + ".JaxRsAnnotationsDecorator",
};
}
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(