Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes ByteBuddy AbstractMethodError which could occur when running tests from IDE #3327

Merged
merged 4 commits into from
Sep 21, 2023

Conversation

JonasKunz
Copy link
Contributor

We have now had for a time the strange behaviour that certain tests would fail when run from the IDE with the following error:

java.lang.AbstractMethodError: Receiver class co.elastic.apm.agent.bci.bytebuddy.MinimumClassFileVersionValidator does not define or inherit an implementation of the resolved method 'abstract net.bytebuddy.jar.asm.ClassVisitor wrap(net.bytebuddy.description.type.TypeDescription, net.bytebuddy.jar.asm.ClassVisitor, net.bytebuddy.implementation.Implementation$Context, net.bytebuddy.pool.TypePool, net.bytebuddy.description.field.FieldList, net.bytebuddy.description.method.MethodList, int, int)' of interface net.bytebuddy.asm.AsmVisitorWrapper.
	at net.bytebuddy.asm.AsmVisitorWrapper$Compound.wrap(AsmVisitorWrapper.java:746) ~[byte-buddy-1.14.8.jar:?]

I've been able to find the root cause of the issue and fix it with this PR.

Our MinimumClassFileVersionValidator implements AsmVisitorWrapper from byte-buddy-dep, which means it uses the unshaded org.objectweb.asm.ClassVisitor as parameter type.
The error message shows that at runtime the AsmVisitorWrapper interface is picked up from byte-buddy instead, which expects the shaded net.bytebuddy.jar.asm.ClassVisitor.

I assume that when the tests are run from maven the classpath order is slightly different, so that byte-buddy-dep wins over byte-buddy. In the IDE the order seems to be the other way around, causing the failure.

I've fixed it by excluding byte-buddy from the dependencies which pull it in transitively: assertJ and mockito.

What does this PR do?

Checklist

jackshirazi
jackshirazi previously approved these changes Sep 20, 2023
Copy link
Contributor

@jackshirazi jackshirazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think before merging, it's worth doing a manual test using a snapshot and just run through to see all the normal things expected are displayed in the UI

@JonasKunz
Copy link
Contributor Author

I think before merging, it's worth doing a manual test using a snapshot and just run through to see all the normal things expected are displayed in the UI

The shanges should (in theory) only have altered the testing classpath. However, I agree that it makes still sense to do a manual test.

@JonasKunz JonasKunz merged commit e28ad36 into elastic:main Sep 21, 2023
@JonasKunz JonasKunz deleted the fix-bytebuddy-test-dep-conflict branch September 21, 2023 13:43
schikin pushed a commit to schikin/apm-agent-java that referenced this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants