Update azure-core-tracing-opentelemetry version and fix sync suppression #10350
Conversation
| @@ -24,7 +24,7 @@ sourceSets { | |||
| dependencies { | |||
| compileOnly(project(":instrumentation:azure-core:azure-core-1.36:library-instrumentation-shaded", configuration = "shadow")) | |||
|
|
|||
| library("com.azure:azure-core:1.36.0") | |||
| library("com.azure:azure-core:1.45.1") | |||
There was a problem hiding this comment.
this is not really necessary and just keeps azure-core in sync with azure-core-tracing-opentelemetry. azure-core is not shaded and is compatible with old version.
So 1.36 is still the min valid version for this code, but should I also update the project name since I'm changing the current lib version?
There was a problem hiding this comment.
I'd keep 1.36.0 here, tests are already run against latest version of library dependencies
There was a problem hiding this comment.
This version here is only really used for compiling, the version used for tests is in testAzure task. We are using testAzure instead of test because the shaded azure-core-tracing-opentelemetry dependency ended in test classpath due to which it was hard to verify whether these classes are found from the agent as they should be of from the test classpath.
Looking at the failures I think we need to introduce azure-core-1.40 module, the new azure-core-tracing-opentelemetry isn't compatible with azure-core-1.36. I'll push adding azure-core-1.40 to this PR, hope you don't mind.
There was a problem hiding this comment.
Thanks a lot @laurit !
the new azure-core-tracing-opentelemetry isn't compatible with azure-core-1.36
it is not but only on the manual configuration path where it never affects the agent.
I was wondering if it's possible to suppress muzzle warning and ignore this specific mismatch.
But I'm going to add another module for azure-core 1.40 or 1.41 in a couple of weeks and we can revisit this.
So thanks a lot for the fix and feel free to merge if you're happy with it.
There was a problem hiding this comment.
it looks like muzzle scans all methods in all helper classes
when the helper classes are implemented in this repo we can just remove unused methods (like the OpenTelemetryTracingOptions constructor that is causing issues but isn't needed by the agent instrumentation)
maybe we could "remove" unnecessary methods when we shade the helper classes, but not sure how to do that easily, so splitting out a new module makes sense to me
There was a problem hiding this comment.
I remove the azure-core-1.40 module and instead replaced the OpenTelemetryTracingOptions that caused muzzle failures with a stub. Please review and decide which approach you like better.
There was a problem hiding this comment.
that looks nice to me, thx, will give @lmolkova the final word 👍
There was a problem hiding this comment.
ah nice trick @laurit! Looks great! Give me 30 mins to pull, build, and quickly test.
There was a problem hiding this comment.
tried it and everything looks great. Thanks a lot!
…ion (#10350) Co-authored-by: Lauri Tulmin <[email protected]>
…ion (open-telemetry#10350) Co-authored-by: Lauri Tulmin <[email protected]>
This PR updates
azure-core-tracing-opentelemetryto version1.0.0-beta.42- the latest (from beta.32).Versions in between don't contain breaking changes that affect OpenTelemetry, but contains some bug fixes including Azure/azure-sdk-for-java#33836
In addition to version update, this PR fixes HTTP span double-collection for synchronous HTTP requests.
Tested manually.
/cc @trask