-
Notifications
You must be signed in to change notification settings - Fork 860
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 to Sdk 0.16.0 #2221
Update to Sdk 0.16.0 #2221
Conversation
Hmm - maybe unversioned snapshot isn't a good idea after all. AFAICT, it seems to trigger some flakiness when muzzling, this error message doesn't seem valid (I can click through to the links it says are missing)
|
…nstrumentation into sdk-0.16.0-SNAPSHOT
Updated to target released 0.16.0 |
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.
❤️ for keeping us up-to-date over the past several months of API and SDK changes
...a-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambda/v1_0/LambdaUtils.java
Show resolved
Hide resolved
...y/io/opentelemetry/instrumentation/awslambda/v1_0/TracingRequestApiGatewayWrapperTest.groovy
Show resolved
Hide resolved
relocate "io.opentelemetry.extension.kotlin", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.kotlin" | ||
relocate "io.opentelemetry.extension.trace.propagation", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.trace.propagation" |
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.
do we still need this for b3, jaeger, and ottrace?
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.
We don't directly use any in our instrumentation so I think it's ok not to shade them (they're only in the agent classloader, never injected into the user). Maybe it's confusing though let me know if we should just shade for the heck of it.
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.
omg it's so confusing 😭, but let's not shade for the heck of it, that just masks the confusion of what's going on, I'll try to write a better comment for these relocation blocks tomorrow, let's merge this(!)
No more breaking changes so using unversioned SNAPSHOT should be OK.