opentelemetry-java-instrumentation
opentelemetry-java-instrumentation copied to clipboard
Force dynamic typing on AssignReturned annotations
When using invokedynamic-Advices, changes to the original method arguments, instrumented class fields etc have to be done via the bytebuddy @Advice.AssignReturned
annotations.
These annotations all have a typing
parameter, which defaults to STATIC
. This means bytebuddy will attempt to assign the returned value without a cast to the target, which can very easily yield unloadable bytecode:
E.g. assigning an entry from an Object[]
to an non-Object
parameter will cause the JVM bytecode verifier to reject the instrumented class. These error messages are hard to find the root cause for unless you deeply know how the instrumentations work and you know to look at the @AssignReturned.*
annotations. This will get even worse due to the type-erasure performed in #11873.
The alternative is to use DYNAMIC
for typing, which means bytebuddy will insert a cast before doing the assignment.
This change proposes to alter the behaviour so that we always use DYNAMIC
, independent of what is specified in the annotations. This should make writing instrumentations easier for beginners and prevent cluttering the annotations with typing = Assigner.Typing.DYNAMIC
, making them more readable due to removing noise.
Note that this is just a proposal based on my "taste" and what I personally think of whats best for ease-of-use, if you prefer the explicit typing via the annotations feel free to reject this PR.