-
Notifications
You must be signed in to change notification settings - Fork 0
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
NH-20004: NH Java agent: upgrade Otel dependency to the latest version #74
Conversation
# Conflicts: # custom/src/main/java/com/appoptics/opentelemetry/extensions/AppOpticsTracerProviderConfigurer.java
# Conflicts: # instrumentation/servlet/servlet-3.0/javaagent/src/main/java/com/appoptics/opentelemetry/instrumentation/servlet/v3_0/Servlet3Advice.java # instrumentation/servlet/servlet-5.0/javaagent/src/main/java/com/appoptics/opentelemetry/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - uncomment checkstyle config - fix compile error
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - bump version to 1.22.0 - refactor deprecated API usage
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - remove reference to SL4J logger due to issue with class loading - exclude resource from shadow relocation - exclude `io/opentelemetry/javaagent/shaded/instrumentation/api/**` from angent loader path as this is available in the bootstrap path.
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - problem *solved* (mostly :) - use standard project structure - use build configuration similar demo [distro](https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/examples/distro)
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - remove unnecessary deps
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - It turns out that we have to use the builder to create a working tracer. Though using `GlobalOpenTelemetry` is still viable, it is actually discouraged and requires nasty workaround. With the new release the tooling that made using `GlobalOpenTelemetry` work were removed(`ApplicationTracer`).
ec445be
to
c617bf6
Compare
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - configure maven local publication and add script
...c/main/java/com/appoptics/opentelemetry/extensions/AppOpticsInboundMetricsSpanProcessor.java
Show resolved
Hide resolved
custom/src/main/java/com/appoptics/opentelemetry/extensions/initialize/Initializer.java
Outdated
Show resolved
Hide resolved
...ppoptics/opentelemetry/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java
Outdated
Show resolved
Hide resolved
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.
LGTM
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004) - remove shadow tasks from agent libs build.gradle - exclude bootstrap classes from agent class loader path
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.
Wow, thank you for sorting through the rat's nest of classloading and upstream changes and putting our distro, finally, in a much better place @cleverchuk! (guess what, OTel 1.23.0 is on the way ;))
I didn't do so much a code review as a set of manual tests, checking some basic functionality like:
- trace spans, metrics, profiling spans - OK
- recent exporter and __Init message changes - OK
- your fix to response header - OK
One representative trace in staging being https://my.na-01.st-ssp.solarwinds.com/136477216875174912/traces/2EDFF3AEBCCF3EB2245CCA697712C099/4FE52F36BDD41034/details/breakdown?duration=3600, which has errors and code profiling.
This PR addresses this and this ticket.
To accomplish this, the project was refactored to follow similar configuration as is found here and ensuring that any dependency on OTEL is added using
compileOnly
configuration. This ensures that bootstrap class loader and the agent class loader don't end up loading the same exact class.The
buildSrc
directory was removed because the new build configuration uses the Gradle instrumentation plugin making it unnecessary. Also added maven local repository installation to facilitate local testing