feat: add thread-local logging support for parallel test isolation#485
feat: add thread-local logging support for parallel test isolation#485xdustinface merged 1 commit intov0.42-devfrom
thread-local logging support for parallel test isolation#485Conversation
Adds a `thread_local` flag to `LoggingConfig` that uses `tracing::subscriber::set_default` (thread-local) instead of `try_init` (global). This allows multiple independent loggers in the same process, primarily for parallel tests where each test needs its own log subscriber. The `LoggingGuard` now holds an optional `DefaultGuard` to keep the thread-local subscriber alive. Additionally, the FFI client's tokio runtime builder propagates the caller's tracing dispatcher to worker threads via `on_thread_start`, so spawned async tasks capture logs through the correct subscriber.
📝 WalkthroughWalkthroughThe changes introduce thread-local logging support to enable proper log isolation for async tasks spawned on worker threads. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dash-spv/src/logging.rs (1)
149-156: Add regression tests for the newthread_localbranch.Please add tests that exercise
init_loggingwiththread_local: true(including guard drop/restoration behavior) to prevent subtle regressions in scoped subscriber handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dash-spv/src/logging.rs` around lines 149 - 156, Add regression tests that cover the new thread_local branch in init_logging: write tests that call init_logging with config.thread_local = true, verify that set_default(subscriber) is used (observe return of default_guard), and exercise guard drop/restoration behavior by dropping the guard and ensuring the previous global subscriber is restored or logging behavior reverts accordingly; specifically target the init_logging function and the thread_local handling around set_default and default_guard to assert correct scoped subscriber behavior and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dash-spv-ffi/src/client.rs`:
- Around line 11-13: Replace the forget(guard) pattern with a thread-local
storage for the dispatcher guard: create a thread_local! static (e.g.
DISPATCHER_GUARD: std::cell::RefCell<Option<tracing::dispatcher::DefaultGuard>>)
and, after calling set_default(...), store the returned guard into
DISPATCHER_GUARD.with(|c| *c.borrow_mut() = Some(guard)); remove the use of
std::mem::forget and keep get_default/set_default imports; this preserves the
DefaultGuard for the lifetime of the thread so its Drop will restore the
previous dispatcher when the thread exits.
---
Nitpick comments:
In `@dash-spv/src/logging.rs`:
- Around line 149-156: Add regression tests that cover the new thread_local
branch in init_logging: write tests that call init_logging with
config.thread_local = true, verify that set_default(subscriber) is used (observe
return of default_guard), and exercise guard drop/restoration behavior by
dropping the guard and ensuring the previous global subscriber is restored or
logging behavior reverts accordingly; specifically target the init_logging
function and the thread_local handling around set_default and default_guard to
assert correct scoped subscriber behavior and prevent regressions.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dash-spv-ffi/src/client.rsdash-spv-ffi/src/utils.rsdash-spv/src/logging.rsdash-spv/src/main.rs
This is to allow independent logging in integration tests. Adds a
thread_localflag toLoggingConfigthat usestracing::subscriber::set_default(thread-local) instead oftry_init(global). This allows multiple independent loggers in the same process, primarily for parallel tests where each test needs its own log subscriber. TheLoggingGuardnow holds an optionalDefaultGuardto keep the thread-local subscriber alive. Additionally, the FFI client's tokio runtime builder propagates the caller's tracing dispatcher to worker threads viaon_thread_start, so spawned async tasks capture logs through the correct subscriber.Summary by CodeRabbit
Release Notes