-
Notifications
You must be signed in to change notification settings - Fork 879
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
Split up build logic to plugins that can eventually be published and … #3474
Split up build logic to plugins that can eventually be published and … #3474
Conversation
…this-repo-specific stuff.
} | ||
|
||
dependencies { | ||
add("muzzleBootstrap", "io.opentelemetry.javaagent:opentelemetry-javaagent-bootstrap") |
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.
As @mateuszrzeszutek said, instrumentationMuzzle
seemed to be equivalent to runtimeClasspath
, which is equivalent to just having a normal dependency like this one
`java-library` | ||
} | ||
|
||
afterEvaluate { |
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.
Compared to previous version, this publishable one removes the (javaagent, library, testing) folder scheme handling from this file (moved to otel.java-conventions) and adds this application of the BOM
dependencies { | ||
// Integration tests may need to define custom instrumentation modules so we include the standard | ||
// instrumentation infrastructure for testing too. | ||
compileOnly("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.
Compared to the previous version, these have been changed from project dependencies (which are subbed back in by otel.java-conventions)
archiveFileName.set("agent-testing.jar") | ||
} | ||
|
||
val agentForTesting by configurations.creating { |
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.
Instead of directly referencing task, I made this configuration and resolve via it to allow resolving the external dependency (or the project dependency that's subbed in in otel.java-conventions)
// OpenTelemetry SDK is not needed for compilation | ||
exclude(group = "io.opentelemetry", module = "opentelemetry-sdk") | ||
exclude(group = "io.opentelemetry", module = "opentelemetry-sdk-metrics") | ||
} | ||
compileOnly("net.bytebuddy:byte-buddy") |
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.
Hoping this won't need to be added for external dependencies (wonder if we even need it internally I didn't try deleting it yet)
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.
If this (and slf4j-api) are needed, we should probably just make them api
dependencies on javaagent-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.
This one should be api
dependency on javaagent-extension-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.
Cool we already had it
// OpenTelemetry SDK is not needed for compilation | ||
exclude(group = "io.opentelemetry", module = "opentelemetry-sdk") | ||
exclude(group = "io.opentelemetry", module = "opentelemetry-sdk-metrics") | ||
} | ||
compileOnly("net.bytebuddy:byte-buddy") |
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.
This one should be api
dependency on javaagent-extension-api
…nstrumentation into extract-externalizable-plugins
withType<Test>().configureEach { | ||
// TODO run tests both with and without experimental span attributes | ||
jvmArgs("-Dotel.instrumentation.apache-camel.experimental-span-attributes=true") | ||
jvmArgs("-Dotel.instrumentation.aws-sdk.experimental-span-attributes=true") | ||
} | ||
|
||
named<Javadoc>("javadoc") { |
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.
This was ported over from the Groovy before
But was originally incorrect because dependencies is on project
, having it under javadoc
didn't do anything, but apparently it could still work in some cases based on the evaluation order of the project based on other configuration. This PR seems to have triggered a subtle change in order that didn't evaluate javadoc
in time
…nstrumentation into extract-externalizable-plugins
…this-repo-specific stuff.
This creates several plugins prefixed
io.opentelemetry.instrumentation
(our maven coordinate). It means we can then separate them into a separate project, apply the gradle plugin publish plugin, and publish them to Gradle plugin portal as part of our release build. Of course, that would first require testing that a distro can actually be built with just those plugins which I'm hoping @iNikem will help with as part of #2778We still have
otel.
plugins to use these publishable plugins as well as add our repository's specific opinions (errorprone, etc)