fix: Disable tracing by default for crates that cause stack overflows on exit#31
fix: Disable tracing by default for crates that cause stack overflows on exit#31RasmusRendal wants to merge 1 commit into
Conversation
| .add_directive("hyper=off".parse().expect("Could not parse constant directive")) | ||
| .add_directive("tonic=off".parse().expect("Could not parse constant directive")) | ||
| .add_directive("h2=off".parse().expect("Could not parse constant directive")) | ||
| .add_directive("reqwest=off".parse().expect("Could not parse constant directive")); |
There was a problem hiding this comment.
Do we need to hardcode this or can we just turn them off by default, still allowing library consumers to override the values when needed? I can name numerous occasions in the past when debug logging of hyper/h2/reqwest at least came in useful during troubleshooting.
Additionally, I am not sure how I feel about allowing expect in non-test code in a library crate, particularly a public one, even if we realistically don't expect anyone external to Famedly to use it. I am probably being nitpicky here though, would welcome thoughts from others.
There was a problem hiding this comment.
I would also prefer to just set the default to off and be able to override it with the config. We already have a way to set the level of each dependency. We could just change the default there
Lines 127 to 138 in 09b352a
There was a problem hiding this comment.
Alrighty, I changed it to be the default. I like this much better myself as well.
The expects are also gone, so nothing to discuss there.
bff586a to
3d19cca
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
+ Coverage 74.18% 74.75% +0.56%
==========================================
Files 5 5
Lines 275 301 +26
==========================================
+ Hits 204 225 +21
- Misses 71 76 +5
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
|
3d19cca to
454d5e2
Compare
… default Due to open-telemetry/opentelemetry-rust#2877, we have to turn off tracing for a couple of crates, to avoid a stack overflow when exiting a binary using opentelemetry-rust and an OTLP endpoint. I don't love this fix personally, but the alternative seems worse.
454d5e2 to
beb9b39
Compare
Due to open-telemetry/opentelemetry-rust#2877, we
have to turn off tracing for a couple of crates, to avoid a stack overflow
when exiting a binary using opentelemetry-rust and an OTLP endpoint.
Given that this fix hasn't been implemented in rust-telemetry before now, other teams must have some other fix to this problem. I'd be curious to hear what it is.