-
-
Notifications
You must be signed in to change notification settings - Fork 55
feat: Add OptionConfiguration
to replace Runtime/BuildTime
#1888
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
Changes from all commits
130b8dd
56dcc01
b0016b8
445a388
610c042
3d4e8f9
c296280
65cea10
65fca3d
3c7e7b1
08ed47d
2a08a6f
78d4a98
0cc186a
af5e3ec
c440582
ebfd6fa
54d956f
3405fa1
d8a455f
888b8a8
e434228
822adf8
0d6f704
915d207
42648e5
84371a0
2162e24
7cbfc65
efa3c50
beadc82
20a046d
a3f535a
53108ee
6693c98
b67c98c
45852f8
cf80744
cdbe3db
8bc698d
ad9674d
bc34b5f
cc3d268
5b31dc6
2875076
99bcae9
729b7e5
5337ea8
32eb242
2ee361c
1ebf9bc
dbf1dbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
+24 −4 | CHANGELOG.md | |
+5 −0 | CMakeLists.txt | |
+21 −0 | cmake/utils.cmake | |
+1 −1 | external/crashpad | |
+39 −1 | include/sentry.h | |
+1 −1 | ndk/gradle.properties | |
+1 −0 | scripts/bump-version.sh | |
+22 −0 | sentry.rc.in | |
+40 −33 | src/backends/sentry_backend_inproc.c | |
+4 −0 | src/sentry_core.c | |
+18 −0 | src/sentry_options.c | |
+1 −0 | src/sentry_options.h | |
+10 −2 | src/sentry_scope.c | |
+1 −1 | src/sentry_tracing.c | |
+2 −2 | tests/assertions.py | |
+10 −0 | tests/cmake.py | |
+64 −0 | tests/fixtures/dotnet_signal/Program.cs | |
+19 −0 | tests/fixtures/dotnet_signal/crash.c | |
+8 −0 | tests/fixtures/dotnet_signal/test_dotnet.csproj | |
+1 −0 | tests/requirements.txt | |
+119 −0 | tests/test_dotnet_signals.py | |
+3 −2 | tests/test_integration_http.py | |
+2 −0 | tests/unit/CMakeLists.txt | |
+4 −4 | tests/unit/test_tracing.c | |
+19 −0 | tests/win_utils.py |
This file was deleted.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,25 +13,25 @@ MonoBehaviour: | |
m_Name: SentryOptions | ||
m_EditorClassIdentifier: | ||
<Enabled>k__BackingField: 1 | ||
<Dsn>k__BackingField: https://[email protected]/4504604988538880 | ||
<Dsn>k__BackingField: https://[email protected].us.sentry.io/4504604988538880 | ||
<CaptureInEditor>k__BackingField: 1 | ||
<EnableLogDebouncing>k__BackingField: 0 | ||
<DebounceTimeLog>k__BackingField: 1000 | ||
<DebounceTimeWarning>k__BackingField: 1000 | ||
<DebounceTimeError>k__BackingField: 1000 | ||
<TracesSampleRate>k__BackingField: 1 | ||
<TracesSampleRate>k__BackingField: 0 | ||
<AutoStartupTraces>k__BackingField: 1 | ||
<AutoSceneLoadTraces>k__BackingField: 1 | ||
<AutoAwakeTraces>k__BackingField: 1 | ||
<AutoAwakeTraces>k__BackingField: 0 | ||
<AutoSessionTracking>k__BackingField: 1 | ||
<AutoSessionTrackingInterval>k__BackingField: 30000 | ||
<ReleaseOverride>k__BackingField: | ||
<EnvironmentOverride>k__BackingField: | ||
<AttachStacktrace>k__BackingField: 0 | ||
<AttachScreenshot>k__BackingField: 1 | ||
<ScreenshotQuality>k__BackingField: 2 | ||
<AttachScreenshot>k__BackingField: 0 | ||
<ScreenshotQuality>k__BackingField: 1 | ||
<ScreenshotCompression>k__BackingField: 75 | ||
<AttachViewHierarchy>k__BackingField: 1 | ||
<AttachViewHierarchy>k__BackingField: 0 | ||
<MaxViewHierarchyRootObjects>k__BackingField: 100 | ||
<MaxViewHierarchyObjectChildCount>k__BackingField: 20 | ||
<MaxViewHierarchyDepth>k__BackingField: 10 | ||
|
@@ -46,25 +46,30 @@ MonoBehaviour: | |
<IsEnvironmentUser>k__BackingField: 0 | ||
<EnableOfflineCaching>k__BackingField: 1 | ||
<MaxCacheItems>k__BackingField: 30 | ||
<InitCacheFlushTimeout>k__BackingField: 2000 | ||
<InitCacheFlushTimeout>k__BackingField: 0 | ||
<SampleRate>k__BackingField: 1 | ||
<ShutdownTimeout>k__BackingField: 2000 | ||
<MaxQueueItems>k__BackingField: 30 | ||
<AnrDetectionEnabled>k__BackingField: 1 | ||
<AnrTimeout>k__BackingField: 5000 | ||
<CaptureFailedRequests>k__BackingField: 1 | ||
<FailedRequestStatusCodes>k__BackingField: f401000057020000 | ||
<FilterBadGatewayExceptions>k__BackingField: 1 | ||
<FilterWebExceptions>k__BackingField: 1 | ||
<FilterSocketExceptions>k__BackingField: 1 | ||
<IosNativeSupportEnabled>k__BackingField: 1 | ||
<AndroidNativeSupportEnabled>k__BackingField: 1 | ||
<NdkIntegrationEnabled>k__BackingField: 1 | ||
<NdkScopeSyncEnabled>k__BackingField: 1 | ||
<PostGenerateGradleProjectCallbackOrder>k__BackingField: 1 | ||
<WindowsNativeSupportEnabled>k__BackingField: 1 | ||
<MacosNativeSupportEnabled>k__BackingField: 1 | ||
<LinuxNativeSupportEnabled>k__BackingField: 1 | ||
<Il2CppLineNumberSupportEnabled>k__BackingField: 1 | ||
<RuntimeOptionsConfiguration>k__BackingField: {fileID: 11400000, guid: f9a47e8442a604414a84d5ad6da6aa43, | ||
type: 2} | ||
<BuildTimeOptionsConfiguration>k__BackingField: {fileID: 11400000, guid: 1c61999bd2a61408393cfe218d42287e, | ||
<RuntimeOptionsConfiguration>k__BackingField: {fileID: 0} | ||
<BuildTimeOptionsConfiguration>k__BackingField: {fileID: 0} | ||
<OptionsConfiguration>k__BackingField: {fileID: 11400000, guid: cea63afb7c75f429799422326f926abe, | ||
type: 2} | ||
<Debug>k__BackingField: 1 | ||
<DebugOnlyInEditor>k__BackingField: 0 | ||
<DiagnosticLevel>k__BackingField: 0 | ||
<DebugOnlyInEditor>k__BackingField: 1 | ||
<DiagnosticLevel>k__BackingField: 2 |
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
using Sentry.Unity; | ||
using UnityEngine; | ||
using UnityEngine.SceneManagement; | ||
|
||
public class SentryOptionConfiguration : SentryOptionsConfiguration | ||
{ | ||
public override void Configure(SentryUnityOptions options) | ||
{ | ||
// Here you can programmatically modify the Sentry option properties used for the SDK's initialization | ||
|
||
#if UNITY_ANDROID || UNITY_IOS | ||
// NOTE! | ||
// On Android and iOS, ALL options configured here will be "baked" into the exported project | ||
// during the build process. | ||
// Changes to the options at runtime will not affect the native SDKs (Java, C/C++, Objective-C) | ||
// and only apply to the C# layer. | ||
|
||
/* | ||
* Sentry Unity SDK - Hybrid Architecture | ||
* ====================================== | ||
* | ||
* Build Time Runtime | ||
* ┌─────────────────────────┐ ┌─────────────────────────┐ | ||
* │ Unity Editor │ │ Game Startup │ | ||
* └──────────┬──────────────┘ └───────────┬─────────────┘ | ||
* │ │ | ||
* ▼ ▼ | ||
* ┌────────────────────────────────────────────────────────────┐ | ||
* │ Options Configuration │ | ||
* │ (This Method) │ | ||
* └─────────────────────────────┬──────────────────────────────┘ | ||
* │ | ||
* ┌───────────────────────────────────┐ | ||
* │ Options used for Init │ | ||
* ▼ ▼ | ||
* ┌──────────────────────────┐ ┌──────────────────────┐ | ||
* │ Native SDK │ │ Unity C# SDK │ | ||
* │ Android & iOS │ │ Initialization │ | ||
* │ ┌────────────────────┐ │ └──────────────────────┘ | ||
* │ │ Options "Baked in" │ │ | ||
* │ └────────────────────┘ │ | ||
* │ The configure call made │ | ||
* │ for this part ran on │ | ||
* │ your build-machine │ | ||
* └──────────────────────────┘ | ||
* │ | ||
* ▼ | ||
* ┌──────────────────────────┐ | ||
* │ Native SDK │ | ||
* │ Android & iOS │ | ||
* └──────────────────────────┘ | ||
*/ | ||
|
||
// Works as expected and will enable all debug logging | ||
// options.Debug = true; | ||
|
||
// Will NOT work as expected. | ||
// This will run twice. | ||
// 1. Once during the build, being baked into the native SDKs | ||
// 2. And a second time every time when the game starts | ||
// options.Release = ComputeVersion(); | ||
#endif | ||
|
||
Debug.Log("OptionConfigure started."); | ||
|
||
// Making sure the SDK is not already initialized during tests | ||
var sceneName = SceneManager.GetActiveScene().name; | ||
if (sceneName != null && sceneName.Contains("TestScene")) | ||
{ | ||
Debug.Log("Disabling the SDK while running tests."); | ||
options.Enabled = false; | ||
} | ||
|
||
// BeforeSend is only relevant at runtime. It wouldn't hurt to be set at build time, just wouldn't do anything. | ||
options.SetBeforeSend((sentryEvent, _) => | ||
{ | ||
if (sentryEvent.Tags.ContainsKey("SomeTag")) | ||
{ | ||
// Don't send events with a tag SomeTag to Sentry | ||
return null; | ||
} | ||
|
||
return sentryEvent; | ||
}); | ||
|
||
Debug.Log("OptionConfigure finished."); | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Should we just make the C# layer initialize the native layer by default?
And make this "Native inits first" thing be opt-in? It seems still hard for folks to understand what's going on, even with the comments below. (I don't expect a beginner to get it)
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.
It would be simpler but we would lose on the ability to catch native exceptions happening at startup. Is that worth it?
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 can see a point for: If you've got plugins that might crash the app before the Unity game starts you can opt-in the native first init and still have full coverage.
If you don't then are those native crashes/exceptions be even actionable for you?
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.
Wouldn't that mean we still need to keep the buildtime native configuration generator (which I understand Bruno was suggesting we could remove)?
Even the information that there are startup crashes is valuable IMO.
Overall, I wouldn't change this - it does provide value (startup crashes) even though it does make things a little more complicated IF users need to adapt these settings programmatically
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've summarized my ideas in #1907
Maybe you can take a look? The idea is to have the auto-init be opt-in. Reduce friction and confusion for those that don't need it but still provide the feature for those that do.