Skip to content

tunnel:Resolve parents in multi-threaded Sender environments#75

Open
rawler wants to merge 1 commit into
slowli:mainfrom
rawler:sender-parent-connection
Open

tunnel:Resolve parents in multi-threaded Sender environments#75
rawler wants to merge 1 commit into
slowli:mainfrom
rawler:sender-parent-connection

Conversation

@rawler
Copy link
Copy Markdown

@rawler rawler commented May 30, 2025

Summary

TracingEventSender did not track per-thread span context, so spans and events using implicit (contextual) parent lookup were serialized without parent IDs. This made it impossible for receivers to reconstruct correct parent-child relationships when multiple threads emit concurrently.

  • Add a per-thread SpanStack (via thread_local) to TracingEventSender
  • Fall back to the thread-local current span when no explicit parent is set
  • Feature-gate stack module and thread_local dep behind sender

@rawler rawler force-pushed the sender-parent-connection branch 2 times, most recently from dad661d to 15d42f6 Compare May 30, 2025 21:09
@rawler rawler force-pushed the sender-parent-connection branch from 15d42f6 to 7fd4f08 Compare March 18, 2026 23:10
TracingEventSender, being a tracing Subscriber, did not track which spans
were entered per thread. When spans or events used implicit (contextual)
parent lookup, the serialized events carried no parent ID, making it
impossible for the receiver to reconstruct correct parent-child
relationships — especially when multiple threads emit concurrently.

TracingEventSender now maintains a per-thread span stack (via
thread_local) and falls back to it when an event or span has no explicit
parent, ensuring serialized events always carry accurate parent IDs.
@rawler rawler force-pushed the sender-parent-connection branch from 7fd4f08 to e506ac1 Compare March 19, 2026 19:18
@rawler rawler marked this pull request as ready for review March 19, 2026 19:18
Copy link
Copy Markdown
Owner

@slowli slowli left a comment

Choose a reason for hiding this comment

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

I'd strongly prefer for all multithreaded sender functionality to be opt-in and have zero overhead if not opted in, so that the general approach continues working if a sender is placed in a no-std / single-threaded environment. AFAIU, it could be possible to move much of the SpanStack logic to the receiver side, which would satisfy this approach. The main thing this would require is attaching (serializable) thread IDs to spans. Unfortunately, the corresponding functionality from the standard library is unstable, but there are workarounds (e.g., using a thread_local! var + a static atomic as in this comment). For the no-std case, thread ID could be skipped (e.g., set to None).

Comment thread tunnel/src/stack.rs
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is largely copied from the tracing-subscriber crate, isn't it? Please provide attribution in a comment.

},
);

assert_eq!(&*event_errors.lock().unwrap(), &Vec::<String>::new());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It'd be easier to compare with an empty slice; does it not work?

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.

2 participants