Conversation
When reviewing #2206, I spotted 2 things that could be improved: - When closing the SDK, we shouldn't need to enqueue the last client report sending to background worker again. This makes the behaviour of `close` more difficult to predict. - Since the session flusher can still enqueue a job to background worker when flushing, we should postpone background worker's shutdown instead.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2207 +/- ##
=======================================
Coverage 97.36% 97.36%
=======================================
Files 101 101
Lines 3798 3799 +1
=======================================
+ Hits 3698 3699 +1
Misses 100 100
|
Yeah. I don't see a reason to send it async at that stage while waiting to shutdown the worker right away. |
|
|
||
| if configuration&.include_local_variables | ||
| exception_locals_tp.disable | ||
| if client = get_current_client |
There was a problem hiding this comment.
@sl0thentr0py Another "concern" I have is that we currently only focus on the main hub's top client, but not other clients. Ideally we should do that for all clients of the main hub, as well as other hubs' (which we don't track AFAIK) clients?
I think the former is probably fine as a hub is unlikely to have more than 1 client. But I do recall some customers tried doing multi-hub setup in the past (also rare tho).
I know both are fairly rare. I just want to raise it so we keep that in mind.
There was a problem hiding this comment.
yea both valid concerns but as a heads up, sometime next year we'll move to removing the Hub and only having a Scope (which will be closer to opentelemetry's context).
rfc here - getsentry/rfcs#122
There was a problem hiding this comment.
Thank you for the context! I just gave it a first pass and got confused around different types of scopes too, as raised by you and many other comments.
But yeah if I understand it correctly, there will only be 1 client in the future and no hubs, which means this won't be a concern anymore.
When reviewing #2206, I spotted 2 things that could be improved:
closemore difficult to predict.#skip-changelog