Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition between root span modification and trace sampling #44

Open
dgoffredo opened this issue Jul 12, 2023 · 0 comments
Open

Race condition between root span modification and trace sampling #44

dgoffredo opened this issue Jul 12, 2023 · 0 comments

Comments

@dgoffredo
Copy link
Contributor

dgoffredo commented Jul 12, 2023

If thread A modifies a trace's root span while another thread B injects one of the root's descendant spans, the root span's SpanData might be concurrently read from (by B) and written to (by A).

I discovered this when thinking about proposed extensions to trace sampling (where spans other than the root span could be considered).

A simple fix would be to replace std::mutex TraceSegment::mutex_ with a std::shared_mutex, and acquire a std::shared_lock on the mutex any time sampling-relevant span data is modified.

Before we commit to that, though, let's revisit the threading guarantees of the library overall. For example, none of nginx, envoy, nor the HTTP example server concurrently access TraceSegment. I hesitate to forbid it outright, but as of now it would make no difference.

Do we want to support multiple threads banging on different spans that are part of the same trace segment? I can imagine fork/join workflows that do this.

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

No branches or pull requests

1 participant