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

Update ByteBuddy to 1.10.18, adjust to new ByteBuddy gradle plugin #1596

Merged
merged 3 commits into from
Nov 12, 2020

Conversation

agoallikmaa
Copy link
Contributor

Updating to ByteBuddy 1.10.18. The motivation is to include the following fix for WebLogic Server:
raphw/byte-buddy#965

In version 1.10.15, ByteBuddy changed how the Gradle plugin works. It is now automatically configured in a way which is incompatible with the current use case, therefore I configure the transformation task from the plugin manually. The specifics are included as comments for the new buildSrc classes, but I will include them in the decription as well:

ByteBuddyPluginConfigurator:
Starting from version 1.10.15, ByteBuddy gradle plugin transformation task autoconfiguration is hardcoded to be applied to javaCompile task. This causes the dependencies to be resolved during an afterEvaluate that runs before any afterEvaluate specified in the build script, which in turn makes it impossible to add dependencies in afterEvaluate. Additionally the autoconfiguration will attempt to scan the entire project for tasks which depend on the compile task, to make each task that depends on compile also depend on the transformation task. This is an extremely inefficient operation in this project to the point of causing a stack overflow in some environments.

To avoid all the issues with autoconfiguration, this class manually configures the ByteBuddy transformation task. This also allows it to be applied to source languages other than Java. The transformation task is configured to run between the compile and the classes tasks, assuming no other task depends directly on the compile task, but instead other tasks depend on classes task. Contrary to how the ByteBuddy plugin worked in versions up to 1.10.14, this changes the compile task output directory, as starting from 1.10.15, the plugin does not allow the source and target directories to be the same. The transformation task then writes to the original output directory of the compile task.

ClasspathByteBuddyPlugin:
Starting from version 1.10.15, ByteBuddy gradle plugin transformations require that plugin classes are given as class instances instead of a class name string. To be able to still use a plugin implementation that is not a buildscript dependency, this reimplements the previous logic by taking a delegate class name and class path as arguments and loading the plugin class from the provided classloader when the plugin is instantiated.

Manually verified that the shadowJar that is built includes the transformed classes.

@iNikem
Copy link
Contributor

iNikem commented Nov 11, 2020

@open-telemetry/java-instrumentation-approvers anybody wants to review? Or I will merge today-tomorrow.

transformation {
// Applying NoOp optimizes build by applying bytebuddy plugin to only compileJava task
tasks = ['compileJava', 'compileScala', 'compileKotlin']
plugin = 'io.opentelemetry.javaagent.tooling.muzzle.collector.MuzzleCodeGenerationPlugin$NoOp'
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this NoOp class if it's no longer used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it.

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

I don't really understand the gradle bytebuddy stuff, but I know we've been stuck on an old version of bytebuddy, so really appreciate this!

@iNikem iNikem merged commit 995e2ca into open-telemetry:master Nov 12, 2020
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.

None yet

4 participants