fix: switch gix TLS backend from native-tls to rustls#1302
fix: switch gix TLS backend from native-tls to rustls#1302jmacd wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
Replace blocking-http-transport-reqwest-native-tls with blocking-http-transport-reqwest-rust-tls in weaver_common to avoid TLS conflicts for downstream consumers that use reqwest with rustls. Remove the now-unnecessary openssl workspace dependency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lquerel
left a comment
There was a problem hiding this comment.
LGTM.
I'd like to provide a bit more context for the other Weaver maintainers.
The OTel Dataflow Engine (aka otel-arrow) has been using Weaver for some time to generate synthetic traffic compatible with a semantic convention registry. This is conceptually similar to weaver registry emit, but implemented as a receiver in the collector. We rely heavily on this traffic_gen receiver across nearly all our tests, including some performance benchmarks.
In addition, we have a requirement to support pluggable crypto libraries in order to handle deployments that require FIPS-compliant cryptography.
The change requested by Joshua fits within this context. From my perspective, this does not introduce any issue for Weaver, but I'd prefer to let other maintainers (@jsuereth @jerbly @lmolkova ) weigh in as well, since I'm somewhat both judge and stakeholder in this specific case.
|
We should do a dummy cargo dist run to check this builds correctly on all platforms. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1302 +/- ##
=======================================
- Coverage 81.2% 81.2% -0.1%
=======================================
Files 111 111
Lines 9371 9371
=======================================
- Hits 7617 7614 -3
- Misses 1754 1757 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@jmacd We had issues at one point with our docker image and this, where the image builds, but fails to run - Just want to confirm this was tests on this patch. |
Replace blocking-http-transport-reqwest-native-tls with blocking-http-transport-reqwest-rust-tls in weaver_common to avoid TLS conflicts for downstream consumers that use reqwest with rustls.
Remove the now-unnecessary openssl workspace dependency.
This helps with an otel-arrow PR attempting the weaver update translated into broken tests because of TLS mismatches open-telemetry/otel-arrow#2368.