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

Extract agent shadow configuration to conventions script. #3256

Merged
merged 8 commits into from
Jun 12, 2021

Conversation

anuraaga
Copy link
Contributor

We have been desparately in need of making this common logic for a long time. And for fixing the -all jar, I am expecting yet another project that contains the bundle of exporters with the shadow configuration. So finally did it.

I am using this relatively new pattern of conventions plugins

https://docs.gradle.org/current/samples/sample_convention_plugins.html

Eventually we can migrate all the gradle/*.gradle files to conventions to allow using the plugins block everywhere.

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.

We have been desparately in need of making this common logic for a long time

💯

@@ -48,6 +49,14 @@
* plugin.
*/
public class ReferenceCollector {

// ReferenceCollector's classloader has a parent including the Gradle classpath, such as buildSrc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out we were previously loading SPI helper resources from buildSrc before our own classpath. Adding shadow to buildSrc, for use in the convention plugin, broke log4j instrumentation since shadow (probably incorrectly, it seems to be the log4j default) bundles in a log4j service file.

implementation("org.ow2.asm:asm:7.0-beta")
implementation("org.ow2.asm:asm-tree:7.0-beta")
implementation("org.apache.httpcomponents:httpclient:4.5.10")
// When updating, also update dependencyManagement/dependencyManagement.gradle.kts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/cc @iNikem I bundled in an update to the byte buddy gradle plugin into this PR (it didn't fix the problem I was seeing but was my first try :P)

@trask trask merged commit 5f49dc0 into open-telemetry:main Jun 12, 2021
robododge pushed a commit to robododge/opentelemetry-java-instrumentation that referenced this pull request Jun 17, 2021
…etry#3256)

* Extract agent shadow configuration to conventions script.

* Remove redundant plugin version declaration

* Resource loader doesn't load from buildSrc

* Comments about byte buddy version

* Fix ReferenceCollectorTest

Co-authored-by: Trask Stalnaker <[email protected]>
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.

4 participants