Skip to content

Commit

Permalink
fix(iroh-blobs): preserve tracing subscriber in the LocalPool (#2735)
Browse files Browse the repository at this point in the history
## Description

This preserves the tracing subscriber that is installed in the current
thread when the LocalPool is created.  It then installs it into every
thread managed by the pool, ensuring that tracing output from the pool
is preserved.

## Breaking Changes

Why is this even pub?  But fine:

`iroh_blobs::util::local_pool::LocalPool` will now install the tracing
subscriber of the thread creating the pool, into each thread managed
by the pool.  Practically this should not break any code already
managing their tracing subscribers either manually inside tasks or by
setting a global subscriber before creating the pool.  But if you
really liked the behaviour of swallowing the logs on doing this it's a
breaking change.

## Notes & open questions

Maybe this should be configurable, though since this is just an
internal tool for us I think we're fine just always having this
behaviour.

Fixes #2577 
Replaces #2589

## Change checklist

- [x] Self-review.
- [x] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [x] Tests if relevant.
- [x] All breaking changes documented.
  • Loading branch information
flub authored Sep 17, 2024
1 parent 285101e commit 5dd8bd3
Showing 1 changed file with 31 additions and 0 deletions.
31 changes: 31 additions & 0 deletions iroh-blobs/src/util/local_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ enum Message {
/// and then wait for all threads to finish executing their loops before
/// returning. This means that all currently executing tasks will be allowed to
/// run to completion.
///
/// The pool will install the [`tracing::Subscriber`] which was set on the current thread of
/// where it was created as the default subscriber in all spawned threads.
#[derive(Debug)]
pub struct LocalPool {
threads: Vec<std::thread::JoinHandle<()>>,
Expand Down Expand Up @@ -165,9 +168,11 @@ impl LocalPool {
shutdown_sem: Arc<Semaphore>,
handle: tokio::runtime::Handle,
) -> std::io::Result<std::thread::JoinHandle<()>> {
let tracing_dispatcher = tracing::dispatcher::get_default(|dispatcher| dispatcher.clone());
std::thread::Builder::new()
.name(thread_name)
.spawn(move || {
let _tracing_guard = tracing::dispatcher::set_default(&tracing_dispatcher);
let mut s = JoinSet::new();
let mut last_panic = None;
let mut handle_join = |res: Option<std::result::Result<(), JoinError>>| -> bool {
Expand Down Expand Up @@ -536,6 +541,8 @@ impl CancellationToken {
mod tests {
use std::{sync::atomic::AtomicU64, time::Duration};

use tracing::info;

use super::*;

/// A struct that simulates a long running drop operation
Expand Down Expand Up @@ -577,6 +584,30 @@ mod tests {
x.forget();
}

#[tokio::test]
async fn test_tracing() {
// This test wants to make sure that logging inside the pool propagates to the
// tracing subscriber that was set for the current thread at the time the pool was
// created.
//
// Look, there should be a custom tracing subscriber here that allows us to inspect
// the messages sent to it so we can verify it received all the messages. But have
// you ever tried to implement a tracing subscriber? In the mean time this test will
// just always pass, to really see the test run it with:
//
// cargo nextest run -p iroh-blobs local_pool::tests::test_tracing --success-output final
//
// and eyeball the output. yolo
let _guard = iroh_test::logging::setup();
info!("hello from the test");
let pool = LocalPool::single();
pool.spawn(|| async move {
info!("hello from the pool");
})
.await
.unwrap();
}

#[tokio::test]
async fn test_drop() {
let _ = tracing_subscriber::fmt::try_init();
Expand Down

0 comments on commit 5dd8bd3

Please sign in to comment.