- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1k
make internal indy-ready #14646
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
make internal indy-ready #14646
Conversation
…nstrumentation into indy-internal
…nstrumentation into indy-internal
| return null; | ||
| } | ||
|  | ||
| @AssignReturned.ToReturned | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both the BootDelegationInstrumentation and LoadInjectedClassInstrumentation enforce inlining, even if indy is enabled. This is done in the transform method above. We do this, to avoid recursion problems with the indy bootstrap.
For this reason they should not be converted to the @AssignReturned notation.
Interesting that your change doesn't cause a test failure though, I would have expected the @AssignReturned annotations to not work, but apparently they do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I remember, @AssignReturned usage is completely independent of inlining the code or not, so this is more about code style and consistency here.
        
          
                ...lemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java
          
            Show resolved
            Hide resolved
        
      …nstrumentation into indy-internal
        
          
                ...lemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ava/io/opentelemetry/javaagent/instrumentation/internal/osgi/EclipseOsgiInstrumentation.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …nstrumentation into indy-internal
Part of #13031
For reviewer: there is still a couple of visibility/virtual fields issues that we can deal with later when removing inlined advices
instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestTypeInstrumentation.javastill has directVirtualField.findcallsinstrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.javadoes not yet implementisIndyReadymethod.