Skip to content

Conversation

@SylvainJuge
Copy link
Contributor

@SylvainJuge SylvainJuge commented Sep 12, 2025

Part of #13031

  • internal-class-loader
  • internal-eclipse-osgi-3.6
  • internal-reflection
  • internal-url-class-loader

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.java still has direct VirtualField.find calls
  • instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java does not yet implement isIndyReady method.

@SylvainJuge SylvainJuge self-assigned this Sep 12, 2025
return null;
}

@AssignReturned.ToReturned
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@SylvainJuge SylvainJuge marked this pull request as ready for review September 22, 2025 14:37
@SylvainJuge SylvainJuge requested a review from a team as a code owner September 22, 2025 14:37
@laurit laurit enabled auto-merge (squash) September 24, 2025 07:50
@laurit laurit merged commit c0db6ba into open-telemetry:main Sep 24, 2025
89 checks passed
@SylvainJuge SylvainJuge deleted the indy-internal branch September 24, 2025 13:07
mznet pushed a commit to mznet/opentelemetry-java-instrumentation that referenced this pull request Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants