-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
DRAFT: Introduce global & request scopes for JavaScript SDK #120
Conversation
Sentry.init(); | ||
|
||
const scope = Sentry.getScope(); // <-- the initial scope, that other scopes will inherit from | ||
const globalScope = Sentry.getGlobalScope(); // <-- the global scope that will be applied to all events | ||
|
||
scope !== globalScope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Sentry.init
create both the scope
and globalScope
and set the same client on both of them or only the globalScope
is tied to the created client and scope
has NoopClient
?
Do we always have these two scopes? What would be the difference for envs like RN, where we have currently only one Hub and Scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of an implementation detail, I think I would implement it in a way that the global scope is created when it is first fetched. E.g. something like this:
function getGlobalScope() {
const client = getClient();
const globalScope = getGlobalScopeForClient(client); // this could either live on the client, or e.g. in a weakmap
if (globalScope) {
return globalScope;
}
return createGlobalScopeForClient(client);
}
Does that make sense to you?
I think it may also be reasonable to not have this in RN if we only have a single hub/scope. (Just to clarify, does that mean in RN we do not have withScope()
or Scope.clone()
? )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RN has withScope()
or Scope.clone()
but it's basically not used. One exception is cloning the scope for specific event -> captureException(error, withScope)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From #122 I understand that scopes hold reference to clients, 1:1.
Will that be the same with the global scope or it will be available to all created clients?
Superseded by #122 |
WORK IN PROGRESS
This is an RFC to rework hubs & scopes for the JavaScript SDK. Subject to change and very much WIP!
Rendered RFC
Note: We decided to split this up into two RFCs. One for the general Hub/Scope/Client changes, and this one will be more specifically tailored to JS and it's specific needs (which may or may not apply to other SDKs/languages as well, and may or may not be used by SDKs going forward).