-
Notifications
You must be signed in to change notification settings - Fork 1k
Instrumentation extension plugin #15861
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
base: main
Are you sure you want to change the base?
Conversation
|
This is exactly what I was looking for, do you think we should also include the following ?
As an added benefit this also provides us a future hook to help with maintenance of extensions, for example if we need to change something in the packaging for indy. |
|
|
||
| dependencies { | ||
| // Bootstrap dependencies: Required classes for instrumentation that run in the bootstrap classloader | ||
| add("muzzleBootstrap", "io.opentelemetry.instrumentation:opentelemetry-instrumentation-api") |
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.
perhaps it would be better if the existing plugins could automatically add these instead of introducing a new plugin?
For the shadow plugin, is your idea that we configure it, or just package it? it's also already included transitively via the muzzle check plugin. for the bom dependencies, it gets a little complicated, but I do have a POC mostly working. Im not sure how the default behavior should be. I implemented the change that @laurit suggested, moving the dependency resolution into the muzzle plugins themselves, and then I tried implementing some logic so that we can handle cases where a user wants to override the BOM. The tricky part was getting the plugin to know its own version at runtime. I found a way around this by generating a properties file at build time and embedding it in the plugin JAR. I' not sure if this is a good approach or not. In my POC, you can set the helper plugin plugins {
id("io.opentelemetry.instrumentation.javaagent-extension") version "2.24.0-alpha"
} and then and then the user can override it if they want dependencies {
implementation(platform("io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha:2.22.0-alpha"))
}
So i guess we should decide if we want to support this, or just leave the two muzzle plugins with the new logic to auto resolve their dependencies? |
I am definitely not a gradle expert here. If the user does not have to do any configuration, then I think we can configure it for them to avoid copy/paste, if there are still things they need to customize then it could maybe simpler to just leave it as part of the example. |
72c9462 to
fde1b1a
Compare
fde1b1a to
80fe2f2
Compare
@SylvainJuge raised a question in slack about whether we could eliminate the need for declaring the muzzle dependencies when using the muzzle plugins, and in general just make it easier to get started with extensions.
This change publishes a new gradle plugin
io.opentelemetry.instrumentation.javaagent-extensionthat packages up the 2 muzzle plugins and their associated dependencies.In our example, it can replace these lines, and then remove the need for these lines entirely. I will followup with updating the example code once this has been published