Re-initialization of Sentry causes events in other threads to not get sent.

[nrxus]

Environment

How do you use Sentry?
Sentry SaaS (sentry.io)

Which SDK and version?
Rust 0.23

Steps to Reproduce

1. Initialize sentry: sentry::init
2. Send events in different threads (the one where it was created and others). This works.
3. Drop the sentry guard and then re-create sentry (again through sentry::init).
4. Try to send more events in the other threads (not the thread where it was created).

Expected Result

Events should all reach sentry regardless of what thread it is sent from.

Actual Result

Only events from the thread with the main hub (the thread that re-created sentry) are successful. Turning on debug on the client options show that indeed it does not even try to send the events across (no logs the response).

Technical Details

When sentry is initialized, a client is bounded to the main Hub. As other threads try to send events across, the client is shared across each thread's local Hub by cloning the reference to the main Hub's client.

When the sentry guard is dropped, the client's transport is removed making the client no longer enabled. Since all the Hub's share the same disabled client, all of the Hub's become inactive.

When sentry is then re-initialized, a new client is bounded to the main Hub. However, the Hubs in the other threads are not aware of this new client, and these Hubs are not re-created as Hubs are thread local static variables that only get created once. These non-main Hubs are still using the disabled Client thus making the Hubs inactive. This means that any events that try to get sent in these threads do not get sent.

Real world implications

In our use case, we need to re-create Sentry because we want the ability to change different sentry configurations on the fly. (i.e., sample rate). We are currently getting around it by checking if a client is disabled for a thread's Hub and if so binding the main Hub's client to it instead before any event gets sent. This is fine but pretty ugly and the root cause took us a long time to figure out so it is definitely surprising behavior. We can try to submit a PR that does something similar during Hub::with_active, in the Sentry code but we are not sure if there may be a better solution that someone with more in-depth knowledge of the architecture of this codebase could see.

[flub]

Hi, interesting case and thanks for the detailed description. Your solution is practical, but as you say I'm not sure if this is reasonable behaviour in the runtime model of the SDK. The Sentry SDKs tries to stay the on all platforms and we'd have to be sure we do not diverge in a strange and conflicting way of the common model. I've pinged this question to the SDK folks however who'll discuss the this at their next meeting and get back with any conclusions they make. Until then I'm reluctant to say we should do one thing or the other for now.

[Swatinem]

I think this basically comes down to PR #97 which is also stalled, so I’m afraid there is no good solution for this ATM.

[nrxus]

I took a cursory look at the Go SDK and it looks like they don't clone the hub per goroutine. So the idea of having a hub per thread that all share the same client seems to be an implementation detail of the Rust SDK. With that in mind, it looks like in Go if the sentry is re-initialized then sentry continues to work on any goroutine, which is not the behavior in the Rust SDK.

Given that, even apart from #97 , which is a PR for for changing the client on-the-fly, re-initialization of the client should not break the sentry client in other threads. I can make a PR for a "simple" way to handle this (using weak arc references and checking if they are still valid) but maybe there is a more elegant solution that I am not seeing as I am not super familiar with this codebase yet.

[Swatinem]

Note that goroutines are fundamentally different from OS level threads.
What you could always do is to just use the Hub explicitly and pass that down to whereever you need. But that might be inconvenient sometimes, thats why the hub is also a thread-local.

[smeubank]

I think this basically comes down to PR #97 which is also stalled, so I’m afraid there is no good solution for this ATM.

as stated here, and by the discussion in the linked PR, this is not a direction we want to go at the moment. If someone disagrees, we can reopen the issue, and we are open to suggestions!