Skip to content

fix: Disable tracing by default for crates that cause stack overflows on exit#31

Draft
RasmusRendal wants to merge 1 commit into
mainfrom
RasmusRendal/ignore_so_crates
Draft

fix: Disable tracing by default for crates that cause stack overflows on exit#31
RasmusRendal wants to merge 1 commit into
mainfrom
RasmusRendal/ignore_so_crates

Conversation

@RasmusRendal

Copy link
Copy Markdown
Contributor

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.

Comment thread src/lib.rs Outdated
.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"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

impl ProviderConfig {
/// Builds a trace filter
pub(crate) fn get_filter(&self, crate_name: &'static str) -> String {
format!(
"{},{}{}={}",
self.general_level,
build_dependencies_level_string(&self.dependencies_levels),
crate_name,
self.level
)
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@RasmusRendal RasmusRendal force-pushed the RasmusRendal/ignore_so_crates branch from bff586a to 3d19cca Compare June 17, 2026 09:18
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.75%. Comparing base (ec0ec43) to head (beb9b39).
✅ All tests successful. No failed tests found.

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     
Files with missing lines Coverage Δ
src/config.rs 77.94% <100.00%> (+11.27%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec0ec43...beb9b39. Read the comment docs.

@RasmusRendal RasmusRendal force-pushed the RasmusRendal/ignore_so_crates branch from 3d19cca to 454d5e2 Compare June 17, 2026 09:58
… 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.
@RasmusRendal RasmusRendal force-pushed the RasmusRendal/ignore_so_crates branch from 454d5e2 to beb9b39 Compare June 17, 2026 09:58
@RasmusRendal RasmusRendal changed the title fix: Disable tracing for crates that cause stack overflows on exit fix: Disable tracing by default for crates that cause stack overflows on exit Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants