Skip to content

Commit

Permalink
Fix inheritance for API annotations (#3551)
Browse files Browse the repository at this point in the history
  • Loading branch information
@JonasKunz
JonasKunz authored Mar 11, 2024
1 parent cb50984 commit 6d4b20e
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 34 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
=== Unreleased
[float]
===== Bug fixes
* Fixed problems with public API annotation inheritance - {pull}3551[#3551]
[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
63 changes: 46 additions & 17 deletions ...src/main/java/co/elastic/apm/agent/sdk/bytebuddy/AnnotationValueOffsetMappingFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import co.elastic.apm.agent.sdk.logging.LoggerFactory;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.annotation.AnnotationDescription;
import net.bytebuddy.description.annotation.AnnotationValue;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.method.ParameterDescription;
import net.bytebuddy.description.type.TypeDescription;
Expand All @@ -32,6 +33,11 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.ArrayDeque;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Queue;
import java.util.Set;

import static net.bytebuddy.matcher.ElementMatchers.named;

Expand All @@ -58,20 +64,31 @@ public Target resolve(TypeDescription instrumentedType, MethodDescription instru

@Nullable
private Object getAnnotationValue(MethodDescription instrumentedMethod, AnnotationValueExtractor annotationValueExtractor) {
MethodDescription methodDescription = instrumentedMethod;
do {
for (TypeDescription typeDescription : methodDescription.getDeclaredAnnotations().asTypeList()) {
if (named(annotationValueExtractor.annotationClassName()).matches(typeDescription)) {
for (MethodDescription.InDefinedShape annotationMethod : typeDescription.getDeclaredMethods()) {
if (annotationMethod.getName().equals(annotationValueExtractor.method())) {
return methodDescription.getDeclaredAnnotations().ofType(typeDescription).getValue(annotationMethod).resolve();
}
Queue<TypeDescription> typesToCheck = new ArrayDeque<>();
typesToCheck.add(instrumentedMethod.getDeclaringType().asErasure());
Set<TypeDescription> alreadyCheckedTypes = Collections.newSetFromMap(new IdentityHashMap<TypeDescription, Boolean>());

while (!typesToCheck.isEmpty()) {
TypeDescription type = typesToCheck.poll();
if (alreadyCheckedTypes.add(type)) {
MethodDescription method = findMethodWithSameSignature(type, instrumentedMethod);
if (method != null) {
AnnotationValue<?, ?> value = findValueOnMethod(method, annotationValueExtractor);
if (value != null) {
return value.resolve();
}
}

TypeDescription.Generic superClass = type.getSuperClass();
if (superClass != null) {
typesToCheck.add(superClass.asErasure());
}
for (TypeDescription.Generic interfaceType : type.getInterfaces()) {
typesToCheck.add(interfaceType.asErasure());
}
}
}

methodDescription = findInstrumentedMethodInSuperClass(methodDescription.getDeclaringType().getSuperClass(), instrumentedMethod);
} while (methodDescription != null);
Class<? extends DefaultValueProvider> defaultValueProvider = annotationValueExtractor.defaultValueProvider();
try {
return defaultValueProvider.getDeclaredConstructor().newInstance().getDefaultValue();
Expand All @@ -89,12 +106,8 @@ private Object getAnnotationValue(MethodDescription instrumentedMethod, Annotati
}

@Nullable
private MethodDescription findInstrumentedMethodInSuperClass(@Nullable TypeDescription.Generic superClass, MethodDescription instrumentedMethod) {
if (superClass == null) {
return null;

}
for (MethodDescription declaredMethod : superClass.getDeclaredMethods()) {
private MethodDescription findMethodWithSameSignature(TypeDescription declaringType, MethodDescription instrumentedMethod) {
for (MethodDescription declaredMethod : declaringType.getDeclaredMethods()) {
if (instrumentedMethod.getInternalName().equals(declaredMethod.getInternalName())
&& instrumentedMethod.getParameters().asTypeList().asErasures().equals(declaredMethod.getParameters().asTypeList().asErasures())) {
return declaredMethod;
Expand All @@ -103,6 +116,21 @@ private MethodDescription findInstrumentedMethodInSuperClass(@Nullable TypeDescr
return null;
}

@Nullable
private static AnnotationValue<?, ?> findValueOnMethod(MethodDescription method, AnnotationValueExtractor valueExtractor) {
for (TypeDescription typeDescription : method.getDeclaredAnnotations().asTypeList()) {
if (named(valueExtractor.annotationClassName()).matches(typeDescription)) {
for (MethodDescription.InDefinedShape annotationMethod : typeDescription.getDeclaredMethods()) {
if (annotationMethod.getName().equals(valueExtractor.method())) {
return method.getDeclaredAnnotations().ofType(typeDescription).getValue(annotationMethod);
}
}
}
}
return null;
}


@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.PARAMETER)
public @interface AnnotationValueExtractor {
Expand All @@ -114,7 +142,8 @@ private MethodDescription findInstrumentedMethodInSuperClass(@Nullable TypeDescr
}

public interface DefaultValueProvider {
@Nullable Object getDefaultValue();
@Nullable
Object getDefaultValue();
}

public static class NullDefaultValueProvider implements DefaultValueProvider {
Expand Down
105 changes: 88 additions & 17 deletions ...nt-plugins/apm-api-plugin/src/test/java/co/elastic/apm/api/AnnotationInheritanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.doReturn;
Expand Down Expand Up @@ -94,6 +99,7 @@ void testClassWithoutAnnotations() {
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class EnabledPublicApiAnnotationInheritance {


@BeforeAll
void beforeAll() {
init(true);
Expand All @@ -104,43 +110,57 @@ void afterAll() {
reset();
}

private TestClassBase createTestClassInstance(Class<? extends TestClassBase> testClass)
throws InstantiationException, IllegalAccessException, InvocationTargetException, NoSuchMethodException {
Constructor<? extends TestClassBase> declaredConstructor = testClass.getDeclaredConstructor();
declaredConstructor.setAccessible(true);
return declaredConstructor.newInstance();
}

@Test
void testClassWithAnnotations() {
invokeApiMethods(new ClassWithAnnotations());
assertThat(reporter.getTransactions()).hasSize(3);
assertThat(reporter.getSpans()).hasSize(1);
}

@Test
void testInheritedCaptureTransaction() {
new ClassWithoutAnnotations().captureTransaction();
checkTransaction("ClassWithoutAnnotations#captureTransaction");
@ParameterizedTest
@ValueSource(classes = {ClassWithoutAnnotations.class, TransitiveClassWithoutAnnotations.class, InterfaceImplementor.class})
void testInheritedCaptureTransaction(Class<? extends TestClassBase> testClass) throws Exception {
TestClassBase instance = createTestClassInstance(testClass);
instance.captureTransaction();
checkTransaction(testClass.getSimpleName() + "#captureTransaction");
}

@Test
void testInheritedCaptureSpan() {

@ParameterizedTest
@ValueSource(classes = {ClassWithoutAnnotations.class, TransitiveClassWithoutAnnotations.class, InterfaceImplementor.class})
void testInheritedCaptureSpan(Class<? extends TestClassBase> testClass) throws Exception {
TestClassBase instance = createTestClassInstance(testClass);
Transaction transaction = ElasticApm.startTransaction();
try (Scope scope = transaction.activate()) {
new ClassWithoutAnnotations().captureSpan();
instance.captureSpan();
}
transaction.end();
checkSpan("ClassWithoutAnnotations#captureSpan");
checkSpan(testClass.getSimpleName() + "#captureSpan");
}

@Test
void testInheritedTracedWithoutActiveTransaction() {
new ClassWithoutAnnotations().traced();
checkTransaction("ClassWithoutAnnotations#traced");
@ParameterizedTest
@ValueSource(classes = {ClassWithoutAnnotations.class, TransitiveClassWithoutAnnotations.class, InterfaceImplementor.class})
void testInheritedTracedWithoutActiveTransaction(Class<? extends TestClassBase> testClass) throws Exception {
createTestClassInstance(testClass).traced();
checkTransaction(testClass.getSimpleName() + "#traced");
}

@Test
void testInheritedTracedWithActiveTransaction() {
@ParameterizedTest
@ValueSource(classes = {ClassWithoutAnnotations.class, TransitiveClassWithoutAnnotations.class, InterfaceImplementor.class})
void testInheritedTracedWithActiveTransaction(Class<? extends TestClassBase> testClass) throws Exception {
Transaction transaction = ElasticApm.startTransaction();
try (Scope scope = transaction.activate()) {
new ClassWithoutAnnotations().traced();
createTestClassInstance(testClass).traced();
}
transaction.end();
checkSpan("ClassWithoutAnnotations#traced");
checkSpan(testClass.getSimpleName() + "#traced");
}

private void checkTransaction(String name) {
Expand All @@ -166,7 +186,16 @@ private void invokeApiMethods(ClassWithAnnotations classWithAnnotations) {
classWithAnnotations.traced();
}

static class ClassWithAnnotations {

abstract static class TestClassBase {
abstract void captureTransaction();

abstract void captureSpan();

abstract void traced();
}

static class ClassWithAnnotations extends TestClassBase {
@CaptureTransaction
void captureTransaction() {
}
Expand All @@ -193,4 +222,46 @@ void captureSpan() {
void traced() {
}
}

static class EmptyClass extends ClassWithAnnotations {
}

static class TransitiveClassWithoutAnnotations extends EmptyClass {
@Override
void captureTransaction() {
}

@Override
void captureSpan() {
}

@Override
void traced() {
}
}

interface InterfaceWithAnnotations {
@CaptureTransaction
void captureTransaction();

@CaptureSpan
void captureSpan();

@Traced
void traced();
}

static class InterfaceImplementor extends TestClassBase implements InterfaceWithAnnotations {

public void captureTransaction() {
}

public void captureSpan() {
}

@Override
public void traced() {
}
}

}

0 comments on commit 6d4b20e

Please sign in to comment.