Skip to content

fix: make HybridCache::disable_tracing disable tracing - #14

Closed
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-make-hybridcache-disable-tracing-disable-traci-e50e76
Closed

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-make-hybridcache-disable-tracing-disable-traci-e50e76

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 11, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Bug

HybridCache::disable_tracing() stored true into the internal AtomicBool (Inner::tracing) instead of false — an apparent copy-paste from enable_tracing() directly above it, introduced in PR foyer-rs#574 and preserved through subsequent refactors. The flag is read without inversion (root_span! creates a real Span::root when true, else Span::noop(); is_tracing_enabled() returns the raw bool), so disable_tracing() actually enabled tracing and is_tracing_enabled() reported true right after.

  • The flag defaults to false, so only callers of disable_tracing() are affected.
  • With fastrace/enable additionally on and a reporter installed, traced insert/get/… operations after disable_tracing() still export spans.
  • The disable_tracing path had zero test coverage anywhere in the repo, which is why the typo persisted.

Fix

In foyer/src/hybrid/cache.rs, changed disable_tracing() to store(false, …). Minimal and idiomatic: the flag's documented semantics are true = tracing on, so disabling must store false, mirroring enable_tracing()'s symmetric store(true).

Testing

Two regression tests added to mod tests (both #[cfg(feature = "tracing")]):

  • test_disable_tracing_flag_level — asserts the public is_tracing_enabled() contract (default-off → enable → on → disable → off → repeatable toggling). Meaningful with just --features tracing.
  • test_disable_tracing_end_to_end_emits_no_spans — installs an in-process fastrace reporter, traces an insert with tracing on (positive control), then calls disable_tracing() and asserts no new insert spans are exported, then re-enables and asserts new spans appear. Self-calibrates: without fastrace/enable (the tracing feature deliberately doesn't enable it) Span::root collapses to Span::noop(), so the test exits as a no-op rather than failing on an inert reporter pipeline.

Checks run:

  • New unit tests pass under --features tracing (flag-level) and --features 'tracing,fastrace/enable' (end-to-end, span path genuinely exercised).
  • Full hybrid::cache::tests module: 13/13 pass with tracing,fastrace/enable; 11/11 pass with tracing off (new tests correctly absent). The fuzzy integration test (test_utils) passes (1/1).
  • Routine checks all green: cargo fmt --all -- --check, cargo clippy -p foyer --features tracing --all-targets -- -D warnings, and cargo check -p foyer (default and --features tracing). The only clippy diagnostic is a pre-existing workspace unknown_lints note unrelated to this change.

Non-versioned verification:

  • Negative control: temporarily reverting disable_tracing() to store(true, …) makes both new tests fail — the flag-level test with "disable_tracing() must report tracing disabled", and the end-to-end test with exported insert span records whose names include foyer::hybrid::cache::insert and the foyer::memory::*::insert children (the exact symptom reported). Restoring the fix makes both pass.
  • Example smoke test: cargo run --features 'tracing,otel' --example tail_based_tracing builds and runs to completion (exit 0) — enable_tracing() + insert + get succeed without panic. Caveat: no live OTLP collector was running on the hard-coded endpoint 127.0.0.1:4317, so actual span delivery to a collector was not verified; the program returns Ok(()) regardless since export is non-fatal. The span-suppression contract itself is fully covered by the in-process reporter test above, which needs no external collector.

Automatic Fixes PRs can be configured here.

@detail-app

detail-app Bot commented Oct 12, 2026

Copy link
Copy Markdown
Author

Closing this pull request because it has had no human activity in 30 days (since 2026-09-11) and was opened 30 days ago. Detail closes its pull requests once they are more than 30 days old and have had no human activity for 14 days.

If this change is still useful, reopen the pull request and Detail won't close it again. To keep a Detail pull request open, add the detail-keep-open label to it.

@detail-app detail-app Bot closed this Oct 12, 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.

0 participants