The same classwalking is necessary for @Path

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassHierarchyIterable.java

@@ -0,0 +1,94 @@
+package datadog.trace.agent.tooling;
+
+import java.util.ArrayDeque;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Iterates over a class, its superclass, and its interfaces in the following breath-first-like
+ * manner:
+ *
+ * <p>1. BaseClass
+ *
+ * <p>2. BaseClass's Interfaces
+ *
+ * <p>3. BaseClass's superclass
+ *
+ * <p>4. BaseClass's Interfaces' Interfaces
+ *
+ * <p>5. Superclass's Interfaces
+ *
+ * <p>6. Superclass's superclass
+ *
+ * <p>...
+ */
+public class ClassHierarchyIterable implements Iterable<Class<?>> {
+  private final Class<?> baseClass;
+
+  public ClassHierarchyIterable(final Class baseClass) {
+    this.baseClass = baseClass;
+  }
+
+  @Override
+  public Iterator<Class<?>> iterator() {
+    return new ClassIterator();
+  }
+
+  public class ClassIterator implements Iterator<Class<?>> {
+    private Class<?> next;
+    private final Set<Class<?>> queuedInterfaces = new HashSet<>();
+    private final Queue<Class<?>> classesToExpand = new ArrayDeque<>();
+
+    public ClassIterator() {
+      classesToExpand.add(baseClass);
+    }
+
+    @Override
+    public boolean hasNext() {
+      calculateNextIfNecessary();
+
+      return next != null;
+    }
+
+    @Override
+    public Class<?> next() {
+      calculateNextIfNecessary();
+
+      if (next == null) {
+        throw new NoSuchElementException();
+      }
+
+      final Class<?> next = this.next;
+      this.next = null;
+      return next;
+    }
+
+    @Override
+    public void remove() {
+      throw new UnsupportedOperationException("remove");
+    }
+
+    private void calculateNextIfNecessary() {
+      if (next == null && !classesToExpand.isEmpty()) {
+        next = classesToExpand.remove();
+        queueNewInterfaces(next.getInterfaces());
+
+        final Class<?> superClass = next.getSuperclass();
+        if (superClass != null) {
+          classesToExpand.add(next.getSuperclass());
+        }
+      }
+    }
+
+    private void queueNewInterfaces(final Class[] interfaces) {
+      for (final Class clazz : interfaces) {
+        if (queuedInterfaces.add(clazz)) {
+          classesToExpand.add(clazz);
+        }
+      }
+    }
+  }
+}

dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java

@@ -3,6 +3,7 @@ package datadog.trace.instrumentation.jaxrs1;
 import static datadog.trace.bootstrap.WeakMap.Provider.newWeakMap;
 
 import datadog.trace.agent.decorator.BaseDecorator;
+import datadog.trace.agent.tooling.ClassHierarchyIterable;
 import datadog.trace.api.DDSpanTypes;
 import datadog.trace.api.DDTags;
 import datadog.trace.bootstrap.WeakMap;
@@ -84,16 +85,25 @@ public class JaxRsAnnotationsDecorator extends BaseDecorator {
       String httpMethod = null;
       Path methodPath = null;
       final Path classPath = findClassPath(target);
-      for (final Method current : new OverriddenMethodIterable(target, method)) {
+      for (final Class currentClass : new ClassHierarchyIterable(target)) {
-        if (httpMethod == null) {
+        final Method currentMethod;
-          httpMethod = locateHttpMethod(current);
+        if (currentClass.equals(target)) {
+          currentMethod = method;
+        } else {
+          currentMethod = findMatchingMethod(method, currentClass.getDeclaredMethods());
         }
-        if (methodPath == null) {
+
-          methodPath = findMethodPath(current);
+        if (currentMethod != null) {
-        }
+          if (httpMethod == null) {
-        // TODO figure out if these will ever be on different methods.
+            httpMethod = locateHttpMethod(currentMethod);
-        if (httpMethod != null && methodPath != null) {
+          }
-          break;
+          if (methodPath == null) {
+            methodPath = findMethodPath(currentMethod);
+          }
+
+          if (httpMethod != null && methodPath != null) {
+            break;
+          }
         }
       }
       resourceName = buildResourceName(httpMethod, classPath, methodPath);
@@ -117,14 +127,40 @@ public class JaxRsAnnotationsDecorator extends BaseDecorator {
     return method.getAnnotation(Path.class);
   }
 
-  private Path findClassPath(Class<Object> target) {
+  private Path findClassPath(final Class<Object> target) {
-    while (target != null && target != Object.class) {
+    for (final Class<?> currentClass : new ClassHierarchyIterable(target)) {
-      final Path annotation = target.getAnnotation(Path.class);
+      final Path annotation = currentClass.getAnnotation(Path.class);
       if (annotation != null) {
         // Annotation overridden, no need to continue.
         return annotation;
       }
-      target = target.getSuperclass();
+    }
+
+    return null;
+  }
+
+  private Method findMatchingMethod(final Method baseMethod, final Method[] methods) {
+    nextMethod:
+    for (final Method method : methods) {
+      if (!baseMethod.getReturnType().equals(method.getReturnType())) {
+        continue;
+      }
+
+      if (!baseMethod.getName().equals(method.getName())) {
+        continue;
+      }
+
+      final Class<?>[] baseParameterTypes = baseMethod.getParameterTypes();
+      final Class<?>[] parameterTypes = method.getParameterTypes();
+      if (baseParameterTypes.length != parameterTypes.length) {
+        continue;
+      }
+      for (int i = 0; i < baseParameterTypes.length; i++) {
+        if (!baseParameterTypes[i].equals(parameterTypes[i])) {
+          continue nextMethod;
+        }
+      }
+      return method;
     }
     return null;
   }

dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java

@@ -51,9 +51,9 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default
   public String[] helperClassNames() {
     return new String[] {
       "datadog.trace.agent.decorator.BaseDecorator",
+      "datadog.trace.agent.tooling.ClassHierarchyIterable",
+      "datadog.trace.agent.tooling.ClassHierarchyIterable$ClassIterator",
       packageName + ".JaxRsAnnotationsDecorator",
-      packageName + ".OverriddenMethodIterable",
-      packageName + ".OverriddenMethodIterable$MethodIterator",
     };
   }
 

dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/OverriddenMethodIterable.java

@@ -1,110 +0,0 @@
-package datadog.trace.instrumentation.jaxrs1;
-
-import java.lang.reflect.Method;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Queue;
-import java.util.Set;
-
-public class OverriddenMethodIterable implements Iterable<Method> {
-  private final Class<?> baseClass;
-  private final Method baseMethod;
-
-  public OverriddenMethodIterable(final Class target, final Method baseMethod) {
-    baseClass = target;
-    this.baseMethod = baseMethod;
-  }
-
-  @Override
-  public Iterator<Method> iterator() {
-    return new MethodIterator();
-  }
-
-  public class MethodIterator implements Iterator<Method> {
-    private Method next = baseMethod;
-    private final Set<Class<?>> queuedInterfaces = new HashSet<>();
-    private final Queue<Class<?>> classesToCheck = new LinkedList<>();
-
-    public MethodIterator() {
-      final List<Class<?>> currentInterfaces = Arrays.asList(baseClass.getInterfaces());
-      queuedInterfaces.addAll(currentInterfaces);
-      classesToCheck.addAll(currentInterfaces);
-
-      final Class<?> superclass = baseClass.getSuperclass();
-      if (superclass != null) {
-        classesToCheck.add(superclass);
-      }
-    }
-
-    @Override
-    public boolean hasNext() {
-      if (next == null) {
-        calculateNext();
-      }
-      return next != null;
-    }
-
-    @Override
-    public Method next() {
-      final Method next = this.next;
-      this.next = null;
-      return next;
-    }
-
-    @Override
-    public void remove() {
-      throw new UnsupportedOperationException();
-    }
-
-    private void calculateNext() {
-      while (next == null && !classesToCheck.isEmpty()) {
-        final Class currentClass = classesToCheck.remove();
-        queueNewInterfaces(currentClass.getInterfaces());
-
-        final Class<?> superClass = currentClass.getSuperclass();
-        if (superClass != null) {
-          classesToCheck.add(currentClass.getSuperclass());
-        }
-
-        next = findMatchingMethod(currentClass.getDeclaredMethods());
-      }
-    }
-
-    private Method findMatchingMethod(final Method[] methods) {
-      nextMethod:
-      for (final Method method : methods) {
-        if (!baseMethod.getReturnType().equals(method.getReturnType())) {
-          continue;
-        }
-
-        if (!baseMethod.getName().equals(method.getName())) {
-          continue;
-        }
-
-        final Class<?>[] baseParameterTypes = baseMethod.getParameterTypes();
-        final Class<?>[] parameterTypes = method.getParameterTypes();
-        if (baseParameterTypes.length != parameterTypes.length) {
-          continue;
-        }
-        for (int i = 0; i < baseParameterTypes.length; i++) {
-          if (!baseParameterTypes[i].equals(parameterTypes[i])) {
-            continue nextMethod;
-          }
-        }
-        return method;
-      }
-      return null;
-    }
-
-    private void queueNewInterfaces(final Class[] interfaces) {
-      for (final Class clazz : interfaces) {
-        if (queuedInterfaces.add(clazz)) {
-          classesToCheck.add(clazz);
-        }
-      }
-    }
-  }
-}

dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/java/Resource.java

@@ -31,9 +31,12 @@ public interface Resource {
     }
   }
 
-  class Test3 extends Test1 {
+  @Path("/test3")
+  class Test3 implements SubResource {
     @Override
-    public String hello(final String name) {
+    @POST
+    @Path("/hi/{name}")
+    public String hello(@PathParam("name") final String name) {
       return "Test3 " + name + "!";
     }
   }