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

Implemented factory for generating invokedynamic proxy classes #9502

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

JonasKunz
Copy link
Contributor

The recently added invokedynamic-based InstrumentationModule mechanism changes where helper classes are loaded:
Previously, helper classes were injected into the application classloader. With the new mechanism, they instead are loaded in an isolated InstrumentationModuleClassloader. While this helps us with the long-term goal of removing shading and improving isolation and debugability of instrumentations, there are some instrumentation where the application classloader actually needs to see certain helper classes.

I've identified the following cases by looking at InstrumentationModules which register helper resources:

We can't just stay with injecting those classes directly: They make use of Otel-SDK/API classes, which we want to remove from the bootstrap in the long term in order to get rid of shading.

To allow the migration of those InstrumentationModules to invokedynamic, I've started implementing the proxy approach suggested in this discussion.

The idea is to not inject the classes listed above directly, but instead to inject runtime-generated proxies for those classes.
These proxies do not have any direct dependency on classes provided by the agent and therefore can be injected without requiring anything from the bootstrap. The "original" classes for which the proxies are generated are loaded in the InstrumentationModuleClassloader and invoked from the proxy classes via invokedynamic.

This PR only adds the proxy generation and corresponding unit tests. It does not yet add the functionality to InstrumentationModules.
This will be added with a follow-up PR. I'm thinking of adding a new method to InstrumentationModule, which could look somewhat like this:

public void injectClassesToAppClassloader(ClassInjector injector) {
   injector
       .createProxy("my.impl.ImplClass", "name.of.generated.Proxy")
       .inject(InjectionMode.CLASS_AND_RESOURCE);
}

The suggested API should be muzzle friendly and would allow us to extend the functionality in the future without having to introduce more new methods to InstrumentationModule.

@JonasKunz JonasKunz requested a review from a team September 19, 2023 11:11
Copy link
Contributor

@laurit laurit 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 this is fine for start. Testing this on the real instrumentations should show whether this will work or not.

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