Unity Sentry SDK programmatic setup #130

semuserable · 2021-04-23T17:13:15Z

Initial attempt of programmatic setup, there are a lot to cover.

Current:

Only one SentryUnity can exist, all subsequent inits will dispose old one and replace with a new one.

Example from SentryInitialization.Init:

// options from Sentry Window, DSN = https://94677106febe46b88b9b9ae5efd18a00@o447951.ingest.sentry.io/5439417
SentryUnity.Init(options);

Debug.LogError("Error!"); // sent to https://94677106febe46b88b9b9ae5efd18a00@o447951.ingest.sentry.io/5439417

// previous disposed, new one created
SentryUnity.Init(new UnitySentryOptions
{
    Enabled = true,
    Dsn = "YOUR_DSN"
});

Debug.LogError("Error next!"); // sent to 'YOUR_DSN'

All of the above are checked on il2cpp builds.

Tests rewritten. Now uses SentryUnity

Things to consider:

link.xml is not implemented for this approach yet
Unity options should be renamed to be consistent with Sentry dotnet SDKs
Should notify a user (more gracefully then now) that a previous SentryUnity is disposed (removed/destroyed) and a new one is created. My thoughts: retrieve call site (stack? frame? CallMember?) of the Init methods and store it somewhere, then check and compare when new Init happens -> notify user.
More use cases?

Let's discuss further steps.

src/Sentry.Unity/SentryUnity.cs

src/Sentry.Unity/UnitySentryOptions.cs

src/test/Sentry.Unity.Tests/PlayModeTests.cs

src/Sentry.Unity/SentryInitialization.cs

src/Sentry.Unity/Extensions/SentryOptionsExtensions.cs

src/Sentry.Unity/Integrations/UnityApplicationLoggingIntegration.cs

src/Sentry.Unity/Integrations/UnityBeforeSceneLoadIntegration.cs

semuserable · 2021-04-29T07:27:37Z

What do you think about this updated window?

Main points

Disable Programmatic Initialization
Compression level UI based on SentryOptions - System.IO.Compression.CompressionLevel

cc @bruno-garcia @bitsandfoxes

semuserable · 2021-04-29T15:27:24Z

Checked InternalsVisibleTo on Sentry.Unity

( AssemblyInfo.cs(18, 12): [CS1726] Friend assembly reference 'Sentry.Unity' is invalid. Strong-name signed assemblies must specify a public key in their InternalsVisibleTo declarations.)

As Sentry is signed, we must also sign Sentry.Unity if we want to reuse internals.

cc @bruno-garcia

…dow new params

semuserable · 2021-04-30T15:45:16Z

Green, yay!

semuserable · 2021-04-30T17:34:36Z

Changes:

Sentry window updated
- Disable Programmatic Initialization opt-in programmatic init
- Disable Auto Compress Payload now a flag, CompressionLevel reused from Sentry dotnet SDK, if not ticked -> NoCompression is selected
Sentry integrations use Hub instance, not static calls
Infra for more testability (IAppDomain, TestHub), new tests
Tests reorganized

CHANGELOG.md

bruno-garcia · 2021-05-01T23:08:45Z

src/Sentry.Unity.Editor/SentryWindow.cs

@@ -105,10 +107,16 @@ public void OnLostFocus()
        // ReSharper disable once UnusedMember.Local
        private void OnGUI()
        {
+            GUILayout.Label(new GUIContent(GUIContent.none), EditorStyles.boldLabel);
+            Options.DisableProgrammaticInitialization = EditorGUILayout.BeginToggleGroup(
+                new GUIContent("Disable Programmatic Initialization", "Disable manual Sentry setup. Rely on SentryOptions config."),


This feels backwards though.

Ideally the experience is defined via SentryWindow. In order to opt-out from the "auto init", we can have a checkbox:

"Initialize SDK programatically'. If the user selects that, the whole SentryWindow should be disabled. Or best: we hide everything.

Another option it to just rely on the Enable flag that' already there. If the user Disables it, it means SentryWindow won't be defining the options to initialize the SDK. The user is always free to call SentryUnity.Init.

I think the best approach is ideal. What are your thoughts? @semuserable @bitsandfoxes

Yeah, I tried to make it like you described, that's why it feels backwards for now. I encountered some problems with the way Unity provides API for this enable/disable functionality. Probably need to take another shot at this.

Another option it to just rely on the Enable flag that' already there. If the user Disables it, it means SentryWindow won't be defining the options to initialize the SDK. The user is always free to call SentryUnity.Init.

I think this is the way to go. Any reasons not to take this approach?

bruno-garcia · 2021-05-01T23:10:18Z

src/Sentry.Unity.Editor/SentryWindow.cs

@@ -138,6 +142,15 @@ private void OnGUI()
                                                       "If not set, auto detects such as 'development', 'production' or 'editor'."),
                Options.Environment);

+            GUILayout.Label(new GUIContent(GUIContent.none), EditorStyles.boldLabel);
+            Options.DisableAutoCompression = EditorGUILayout.BeginToggleGroup(
+                new GUIContent("Disable Auto Compress Payload", "Disable auto Sentry setup. Rely on SentryOptions config."),


This is also a bit confusing IMHO.

Can't we have a "fake" value called Auto and have that selected? And if a user changes to one of the other values we'll just store that instead?

My idea was to use Sentry CompressionLevel as is for this window.

I can create another enum with the same structure as CompressionLevel, call it UnityCompressionLevel and do the proper mapping to CompressionLevel.

Do you prefer the second approach (with UnityCompressionLevel enum)?

I think it's simpler to have auto in the UI and not bother creating a new enum

Something like that

?

src/Sentry.Unity/Extensions/JsonExtensions.cs

src/Sentry.Unity/Extensions/SentryOptionsExtensions.cs

src/Sentry.Unity/SentryInitialization.cs

src/test/Sentry.Unity.Tests/Stubs/TestHub.cs

src/test/Sentry.Unity.Tests/UnityApplicationLoggingIntegrationTests.cs

src/test/Sentry.Unity.Tests/UnityEventExceptionProcessorTests.cs

bruno-garcia

Great improvements. Thanks

src/Sentry.Unity/Extensions/SentryOptionsExtensions.cs

src/Sentry.Unity/Integrations/UnityApplicationLoggingIntegration.cs

bruno-garcia · 2021-05-07T18:45:33Z

src/Sentry.Unity.Editor/SentryWindow.cs

@@ -105,10 +107,16 @@ public void OnLostFocus()
        // ReSharper disable once UnusedMember.Local
        private void OnGUI()
        {
+            GUILayout.Label(new GUIContent(GUIContent.none), EditorStyles.boldLabel);
+            Options.DisableProgrammaticInitialization = EditorGUILayout.BeginToggleGroup(
+                new GUIContent("Disable Programmatic Initialization", "Disable manual Sentry setup. Rely on SentryOptions config."),


Another option it to just rely on the Enable flag that' already there. If the user Disables it, it means SentryWindow won't be defining the options to initialize the SDK. The user is always free to call SentryUnity.Init.

I think this is the way to go. Any reasons not to take this approach?

src/Sentry.Unity/Integrations/UnityApplicationLoggingIntegration.cs

bruno-garcia · 2021-05-07T19:12:22Z

src/Sentry.Unity/Integrations/UnityBeforeSceneLoadIntegration.cs

+        public UnityBeforeSceneLoadIntegration(IApplication? appDomain = null)
+            => _application = appDomain ?? ApplicationAdapter.Instance;
+
+        public void Register(IHub hub, SentryOptions options)


This is called by Sentry when the SDK is initialized.

THe integrations are supposed to call into Sentry whenever something happens. Like when the game changes scenes, we could add a new breadcrumb. And even change a tag. That would be an integration

src/Sentry.Unity/SentryInitialization.cs

bruno-garcia · 2021-05-07T21:09:54Z

We should discuss this on a call and wrap it up together

# Conflicts: # src/Sentry.Unity/SentryInitialization.cs

semuserable · 2021-05-11T17:18:10Z

Updated with the latest (+ main merged).

cc @bruno-garcia let's review it together on a call and close it.

semuserable added 4 commits April 23, 2021 19:25

Minor refactor

daa3157

SentryUnity extracted and reused; config extensions

5c667d9

Tests aligned

3101270

minor renaming

9f2cd4a

semuserable requested review from bruno-garcia and bitsandfoxes April 23, 2021 17:14

bruno-garcia reviewed Apr 23, 2021

View reviewed changes

semuserable added 5 commits April 26, 2021 21:25

Refactor; more Sentry compliant

1d1acfd

UnityBeforeSceneLoadIntegration & ref

13a53dd

Integrations moved into separate files

3c95d4d

SentryUnity.Init returns void; tests aligned

fe05c33

Unity integrations moved into SentryUnity.Init

f1bf08e

bruno-garcia reviewed Apr 29, 2021

View reviewed changes

semuserable added 2 commits April 30, 2021 12:02

Stubs for IHub and IAppDomain; unit tests for integrations; SentryWin…

287580e

…dow new params

Merge branch 'main' into feature/sentry-unity-init

6298231

semuserable marked this pull request as ready for review April 30, 2021 09:16

semuserable added 2 commits April 30, 2021 17:58

changelog

6d38a29

typo

ae10a29

semuserable added 3 commits April 30, 2021 18:46

garbage

caa129a

unity test package updated

691bfe7

EventCapture removed

fefc269

bruno-garcia reviewed May 2, 2021

View reviewed changes

Review changes

b01d022

bruno-garcia reviewed May 5, 2021

View reviewed changes

semuserable mentioned this pull request May 6, 2021

Move Unity specific tests from Sentry SDK #145

Closed

Review changes

5469903

semuserable added 3 commits May 7, 2021 17:18

ToEventTagType

6d04f22

SentryUnity #if DEVELOPMENT_BUILD removed

0eacd81

UnitySentryOptions constructor introduced

3bf0dd4

bruno-garcia reviewed May 7, 2021

View reviewed changes

semuserable added 2 commits May 11, 2021 18:39

SentryWindow changes; FlushAsync for hub

b39357d

Merge branch 'main' into feature/sentry-unity-init

bb69620

# Conflicts: # src/Sentry.Unity/SentryInitialization.cs

bruno-garcia merged commit 2962a98 into main May 11, 2021

bruno-garcia deleted the feature/sentry-unity-init branch May 11, 2021 17:42

semuserable mentioned this pull request May 13, 2021

Application Close event should only flush the queue #141

Closed

semuserable mentioned this pull request May 27, 2021

Add a clear path to opt-out of the SentryWindow approach to manual initialization #66

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unity Sentry SDK programmatic setup #130

Unity Sentry SDK programmatic setup #130

semuserable commented Apr 23, 2021 •

edited

Loading

semuserable commented Apr 29, 2021

semuserable commented Apr 29, 2021 •

edited

Loading

semuserable commented Apr 30, 2021

semuserable commented Apr 30, 2021

bruno-garcia May 1, 2021

semuserable May 5, 2021

bruno-garcia May 7, 2021

bruno-garcia May 1, 2021

semuserable May 5, 2021

bruno-garcia May 6, 2021

semuserable May 11, 2021

bruno-garcia left a comment

bruno-garcia May 7, 2021

bruno-garcia May 7, 2021

bruno-garcia commented May 7, 2021

semuserable commented May 11, 2021 •

edited

Loading

Unity Sentry SDK programmatic setup #130

Unity Sentry SDK programmatic setup #130

Conversation

semuserable commented Apr 23, 2021 • edited Loading

semuserable commented Apr 29, 2021

semuserable commented Apr 29, 2021 • edited Loading

semuserable commented Apr 30, 2021

semuserable commented Apr 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bruno-garcia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bruno-garcia commented May 7, 2021

semuserable commented May 11, 2021 • edited Loading

semuserable commented Apr 23, 2021 •

edited

Loading

semuserable commented Apr 29, 2021 •

edited

Loading

semuserable commented May 11, 2021 •

edited

Loading