-
-
Notifications
You must be signed in to change notification settings - Fork 51
Add native local scope for Windows/Linux #928
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
Conversation
|
…y-unreal into feat/local-scope-desktop
Awesome Stuff! (Non helpful but hopefully encouraging comment) |
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.
looking promising, almost better than i had expected without the getters! 👍
plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentrySubsystem.cpp
Show resolved
Hide resolved
plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentrySubsystem.cpp
Show resolved
Hide resolved
plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentryScope.cpp
Outdated
Show resolved
Hide resolved
This PR updates the Unreal Engine SDK docs to reflect recent changes that were made in getsentry/sentry-unreal#928. Specifically, `ConfigureScope` function was removed so now users can modify scope either via dedicated methods of `SentrySubsystem` class (for global scope) or by using a corresponding callback parameter that is available for all `capture` methods to add data for one specific event (local scope).
# Conflicts: # sample/Content/UI/W_SentryDemo.uasset
# Conflicts: # modules/sentry-native
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.
I'm a bit nervous about the API changes but the sentry-native related changes LGTM 👍
CHANGELOG.md
Outdated
- `Environment` and `Dist` get/set functions were removed from the `Scope` class and now these properties have to be set in plugin settings instead. | ||
- `ConfigureScope` function was removed from `SentrySubsystem` class. |
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 probably ok without deprecating first because it hasn't reached 1.0.0?
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.
Yes, given the scope of the changes in this PR it seems like an appropriate time to remove these APIs ahead of the major version bump.
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.
Summarizing the reason with a few words and perhaps linking to https://develop.sentry.dev/sdk/miscellaneous/hub_and_scope_refactoring/ could make the breaking change easier to accept for those who'll be affected 😉
This PR updates the Unreal Engine SDK docs to reflect recent changes that were made in getsentry/sentry-unreal#928. Specifically, `ConfigureScope` function was removed so now users can modify scope either via dedicated methods of `SentrySubsystem` class (for global scope) or by using a corresponding callback parameter that is available for all `capture` methods to add data for one specific event (local scope).
This PR updates the Unreal Engine SDK docs to reflect recent changes that were made in getsentry/sentry-unreal#928. Specifically, `ConfigureScope` function was removed so now users can modify scope either via dedicated methods of `SentrySubsystem` class (for global scope) or by using a corresponding callback parameter that is available for all `capture` methods to add data for one specific event (local scope).
Key Changes: - Updated some C++ examples to use `FSentryVariant` type instead of string (getsentry/sentry-unreal#971) - Added notice about using local scopes on Windows/Linux (getsentry/sentry-unreal#928) - Fixed supported platforms for screenshot feature
This PR adds Unreal Engine SDK migration guide for the upcoming `1.0.0` plugin release. Related to: - getsentry/sentry-unreal#928 - getsentry/sentry-unreal#971 - getsentry/sentry-unreal#1051 - getsentry/sentry-unreal#1018 - getsentry/sentry-unreal#886 - getsentry/sentry-unreal#745 - getsentry/sentry-unreal#436 - getsentry/sentry-unreal#589 - getsentry/sentry-unreal#1057 - getsentry/sentry-unreal#669 --------- Co-authored-by: Bruno Garcia <[email protected]>
This PR adopts the local scope implementation changes for
sentry-native
discussed in getsentry/sentry-native#1226.Instead of applying scope data to each outgoing event via the
before_send
handler, the SDK now uses a write-onlysentry_scope_t
instance which is then passed to the newsentry_capture_event_with_scope
function. Also, with this approach global scope can be modified directly via native's API (tags, level, etc.) which addresses the issue with missing context values for fast-fail crashes.Breaking changes:
Dist
andEnvironment
properties are now set during the SDK initialization via options, no more corresponding set/get functions inScope
class.ConfigureScope
function and it's a no-op on Win/Linux anyway it was removed fromUSentrySubsystem
.Note: adding breadcrumbs via
sentry_add_breadcrumb
to global scope unblocks further work on getsentry/sentry-native#1166 as custom handling for the corresponding event in Unreal plugin is no longer required.Docs update - getsentry/sentry-docs#13872