-
-
Notifications
You must be signed in to change notification settings - Fork 461
POTEL 18 - Use correct SentryOptions for SentryClient constructor
#3490
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
POTEL 18 - Use correct SentryOptions for SentryClient constructor
#3490
Conversation
…pes-merge-2-add-scopes
…ainScopes to rootScopes
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- POTEL 18 - Use correct `SentryOptions` for `SentryClient` constructor ([#3490](https://github.com/getsentry/sentry-java/pull/3490))If none of the above apply, you can opt out of this check by adding |
Performance metrics 🚀
|
| final IScope rootIsolationScope = new Scope(options); | ||
|
|
||
| scopes.close(true); | ||
| globalScope.replaceOptions(options); |
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 diff looks like there was an auto-merge fail on a previous merge. It's just the options that are used for the SentryClient ctor that should be different.
| } | ||
|
|
||
| @Test | ||
| fun `current scope returns obj when scopes is active`() { |
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.
Are these tests now testing something different? Should they be additions instead of replacements?
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.
With Sentry.close() in test setup, InternalSentrySdk.getCurrentScope() doesn't work correctly afair, so I used the actual setup code. Tests should still test the same thing now.
I just retested with the old code and it seems to work so maybe this was just an interim thing. I do however think the new code should be closer to real world usage, so I'd keep it this way.
wdyt?
📜 Description
Use
SentryOptionsfromrootScopesforSentryClientconstructor inSentry.init.💡 Motivation and Context
If
replaceOptionsnoops, the newSentryOptionsaren't actually used which meansSentryClientand the rest of the SDK have differentSentryOptionsin use.💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps