getsentry
Merge Hub & Scope in SDKs
This RFC is about merging the concepts of hubs & scopes in the SDK into a singular Scope concept.
Note that this RFC is specifically designed to be relevant to all SDKs. For more specific requirements/changes for e.g. the JavaScript SDK, there is a separate RFC here: https://github.com/getsentry/rfcs/pull/120. This way we can keep the discussion more focused on generally applicable things vs. more language/SDK specific things.
Updated this based on feedback in regard to scope<>client relationship.
Now, a scope holds a reference to a client, which can be set via scope.setClient(). The client is inherited by child scopes normally. scope.getClient() returns a Noop client if none has been set yet (=init has not been called yet).
Left some questions / comments. Generally I'm surprised to see such a focus on client and its APIs. In Java SDK these are rarely touched by clients and rather hidden away.
This is something that should not concern most users, but for those who need it (multiple clients...) we need to make sure stuff still works/is possible to do. But yes, we should def. focus on making the "regular" use case most convenient and abstracted away!
I read it and everything looks reasonable. How clients and the noop client are handled i have not yet fully figured out for python. But I guess I need to tinker with some code to fully get the picture and give meaningful feedback. Will try to make time for this this week.
I don't have much to add from the Mobile perspective because, on Mobile, we encourage our users to call the static APIs Sentry.captureException, etc., and hardly any users interact with the hub or client. I don't see a problem merging the scope and the hub for Mobile. Correct me if I missed something on other mobile platforms, @krystofwoldrich, @romtsn, @markushi, @brustolin, @stefanosiano.
Generally I'm surprised to see such a focus on client and its APIs. In Java SDK these are rarely touched by clients and rather hidden away.
I agree with @adinauer. I would actually love to make the client internal/private. I would prefer passing options to the scope, and that's all the flexibility you get.
On Cocoa, we don't support creating your own implementation of a hub, client, or transport, and nobody ever complained. Our only complaints so far are that some customers want to send the data to Sentry and their backend, but that has no impact on this RFC.
make the client internal/private
There are users who customize clients, e.g. to support multi DSN or send events somewhere else as well. While rare this is still a use case we probably shouldn't drop without good reason.
make the client internal/private
There are users who customize clients, e.g. to support multi DSN or send events somewhere else as well. While rare this is still a use case we probably shouldn't drop without good reason.
IMHO the goal should be to hide the client away as much as possible by default (which we already do anyhow generally, via init() - nobody will generally see a client reference in this scenario), but allow to configure it for "edge cases" (as @adinauer mentioned). But I also think that this can be decided in detail on a SDK-by-SDK basis, where it makes sense to adjust this, do it! (FWIW in JS there are some scenarios where you need to interact with a client manually)
I applied all the feedback and moved this out of draft - I think this is ready to "really" review!
From RN point of view I don't we can merge Hub and Scope. 99% time users can use Sentry.captureException static API, the same as in browser JS.
RN Uses only one Hub and extends browser JS implementation of Scope to synchronize with the native SDKs (iOS, Android).
Since RN heavily depends on browser JS SDKs, we will have to adopt it basically immediately when this changes in JS. Event if the native SDKs haven't made the change at the time, it should not affect the scope synchronizing.
From RN point of view I don't we can merge Hub and Scope. 99% time users can use
Sentry.captureExceptionstatic API, the same as in browser JS.RN Uses only one Hub and extends browser JS implementation of Scope to synchronize with the native SDKs (iOS, Android).
Since RN heavily depends on browser JS SDKs, we will have to adopt it basically immediately when this changes in JS. Event if the native SDKs haven't made the change at the time, it should not affect the scope synchronizing.
This change will def. happen in a major only, so there should be enough time etc. to sync this up with RN! The general Scope API should also remain more or less similar to right now, so IMHO it should be possible to continue doing more or less what you're doing right now in RN (*small tweaks will probably be necessary, but nothing we can't figure out!)
After talking to more mobile folks, we don't see a problem with moving forward with this, but we don't see a need to change that cause it doesn't cause any problems in our SDKs. We will most likely make that change in one of our next major versions, but we won't do a major version because of that.
After talking about this with @mitsuhiko , I updated this RFC to again include the (updated) concepts of a global scope, as well as an isolation scope.
The idea is that ALL SDKs should have the concepts. Although it may be less relevant for some - but in these cases, they will simply do nothing anyhow. But esp. for isomorphic envs like JS, the same APIs should exist for server & client.
Please re-read the RFC for the added sections. We had some talks before about the fact that this is not equally relevant for all SDKs (which is why I initially split this into two RFCs), but @mitsuhiko reiterated that we should do this in an aligned way.
I agree with @antonpirker's assessment that these changes seem reasonable at first glance for the Python SDK.
My one suggestion for the RFC would be to consider adding a visualization, similar to the one on the unified API docs page, to describe how these changes will affect the API at a high level.
🥔 for this RFC! Left some comments.
What I'd find especially helpful is some sort of mapping of the current concepts (e.g. creating a new hub) and/or common cases (e.g. wrapping a request processor) to the proposed concepts in this RFC (e.g., creating an isolation scope). You're already doing this in couple places in the RFC, but maybe it makes sense to sum up the most common cases in a table somewhere with a small explanation.
