Feat: Automatic user IDs on native crashes & .NET events #728

vaind · 2022-05-04T14:56:06Z

closes #606

android -> dotnet
iOS-> dotnet
macOS -> dotnet
Unity -> dotnet -> Windows
Unity -> dotnet -> Linux
Unity -> dotnet on WebGL. Notes:
- the ID seems to persist across tabs & browser sessions, likely a cookie or local storage based, so pretty useful and in line with other platforms
- opening in a different browser or a private tab gives a new ID, which is also OK

bitsandfoxes

So in summary:

For Android, Cocoa (iOS and macOS) we fetch the ID from the native SDKs (which is great because they initialize first)
For Native (Windows & Linux) we set the ID from the C# layer.

This is great!

CHANGELOG.md

bitsandfoxes · 2022-05-13T14:57:07Z

src/Sentry.Unity/Sentry.Unity.csproj

@@ -27,4 +27,9 @@
      Overwrite="true"/>
  </Target>

+  <ItemGroup>
+    <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">


TIL that this exists. @bruno-garcia would that help with some of the issues we had with some of the .NET SDK internals we did not want to make public like inside of SentryOptions for SentryUnityOptions?

No, this is the same as adding [assembly: InternalsVisibleToAttribute(..) in C#. Just another way.

bitsandfoxes · 2022-05-13T15:00:40Z

src/Sentry.Unity/SentryUnityOptions.cs

+                _defaultUserId = value;
+                if (_defaultUserId is null)
+                {
+                    DiagnosticLogger?.LogWarning("Couldn't set the default user ID - the value is NULL.");
+                }
+                else
+                {
+                    DiagnosticLogger?.LogDebug("Setting '{0}' as the default user ID.", _defaultUserId);
+                }


Suggested change

_defaultUserId = value;

if (_defaultUserId is null)

{

DiagnosticLogger?.LogWarning("Couldn't set the default user ID - the value is NULL.");

}

else

{

DiagnosticLogger?.LogDebug("Setting '{0}' as the default user ID.", _defaultUserId);

}

if (value is null)

{

DiagnosticLogger?.LogWarning("Couldn't set the default user ID - the value is NULL.");

}

else

{

_defaultUserId = value;

DiagnosticLogger?.LogDebug("Setting '{0}' as the default user ID.", _defaultUserId);

}

Should this not set the id if it logs that it can't?

Is the goal to reset to null as it'll be needed later to "unset"?

Should this not set the id if it logs that it can't?

this is a setter - doesn't feel right to ignore the value being passed (i.e. not set)

bruno-garcia · 2022-05-13T17:31:58Z

src/Sentry.Unity/Sentry.Unity.csproj

@@ -27,4 +27,9 @@
      Overwrite="true"/>
  </Target>

+  <ItemGroup>
+    <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">


No, this is the same as adding [assembly: InternalsVisibleToAttribute(..) in C#. Just another way.

bruno-garcia · 2022-05-13T17:34:42Z

src/Sentry.Unity/SentryUnityOptions.cs

+                _defaultUserId = value;
+                if (_defaultUserId is null)
+                {
+                    DiagnosticLogger?.LogWarning("Couldn't set the default user ID - the value is NULL.");
+                }
+                else
+                {
+                    DiagnosticLogger?.LogDebug("Setting '{0}' as the default user ID.", _defaultUserId);
+                }


Is the goal to reset to null as it'll be needed later to "unset"?

bruno-garcia · 2022-05-13T17:36:26Z

src/Sentry.Unity/WebGL/SentryWebGL.cs

@@ -36,6 +37,9 @@ public static void Configure(SentryUnityOptions options)
                options.DiagnosticLogger?.LogWarning("Attaching screenshots on WebGL is disabled - " +
                    "it currently produces blank screenshots mid-frame.");
            }
+
+            // Use AnalyticsSessionInfo.userId as the default UserID in native & dotnet
+            options.DefaultUserId = AnalyticsSessionInfo.userId;


If this is an ID that can be used to link the user across apps/services, we shouldn't be using it here.
We purposely on some GUIDs we create so that we can have some consistency in detecting "number ofusers impacted" but the goal is never to do actual user tracking. For that reason we've dropped "Android ID" also, for example

JS will default to the connection IP Address, and there's no native so we might be better off without anything here

we don't have sentry-js (yet)

therefore setting this here only affects c# events

I'm setting it here because that's what feeled right - each ~~native~~ platform integration defines the source for the DefaultUserId value

If this is an ID that can be used to link the user across apps/services, we shouldn't be using it here.

I really don't think so. It's an installation ID.

https://docs.unity3d.com/ScriptReference/Analytics.AnalyticsSessionInfo-userId.html
A random GUID uniquely identifying sessions played on the same instance of your game or app.
Unity generates the userId GUID immediately after installation. The userId value is only regenerated if the user re-installs the game.

vaind · 2022-05-16T07:18:33Z

since this is already approved I'm merging to avoid conflicts in #747 - if we need to make changes based on those unresolved conversations, let's do them in the other PR.

vaind force-pushed the feat/user-id-sync branch 2 times, most recently from e98a20f to 0be2cce Compare May 11, 2022 08:41

vaind changed the title ~~Feat: synchronize user IDs between native & dotnet~~ Feat: Automatic user IDs on native crashes & .NET events May 11, 2022

vaind marked this pull request as ready for review May 11, 2022 12:17

vaind requested review from bruno-garcia and bitsandfoxes as code owners May 11, 2022 12:17

vaind force-pushed the feat/user-id-sync branch 3 times, most recently from c9ad8ca to b6424d4 Compare May 12, 2022 08:41

bitsandfoxes approved these changes May 13, 2022

View reviewed changes

bruno-garcia reviewed May 13, 2022

View reviewed changes

vaind added 12 commits May 16, 2022 09:20

feat: allow setting default user ID from native integrations

8b92992

feat: Synchronize user-ID from android to dotnet

0f52319

chore: update comments

76d85da

feat: Synchronize user-ID from Unity to windows native & dotnet

534b4a5

feat: Synchronize user-ID from Unity to dotnet on WebGL

2d8f251

test: SmokeTester checks for user ID in dotnet events

37d796d

feat: Synchronize user-ID from native to dotnet on iOS/macOS

e43f2c4

chore: improve RunUnity script log output

8c7700d

chore: update changelog

2f64583

chore: improve WebGL test script

25687d1

refactor: DefaultUserId setting logging

8226f42

chore: fix changelog

afd9a7b

vaind force-pushed the feat/user-id-sync branch from 884aae4 to afd9a7b Compare May 16, 2022 07:26

fix: iOS playmode tests

8d7a4c4

vaind merged commit 1db2fa4 into main May 16, 2022

vaind deleted the feat/user-id-sync branch May 16, 2022 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Automatic user IDs on native crashes & .NET events #728

Feat: Automatic user IDs on native crashes & .NET events #728

vaind commented May 4, 2022 •

edited

Loading

bitsandfoxes left a comment •

edited

Loading

bitsandfoxes May 13, 2022

bruno-garcia May 13, 2022

bitsandfoxes May 13, 2022

bruno-garcia May 13, 2022

vaind May 13, 2022

bruno-garcia May 13, 2022

bruno-garcia May 13, 2022

bruno-garcia May 13, 2022

bruno-garcia May 13, 2022

vaind May 13, 2022 •

edited

Loading

vaind commented May 16, 2022

Feat: Automatic user IDs on native crashes & .NET events #728

Feat: Automatic user IDs on native crashes & .NET events #728

Conversation

vaind commented May 4, 2022 • edited Loading

bitsandfoxes left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaind May 13, 2022 • edited Loading

Choose a reason for hiding this comment

vaind commented May 16, 2022

vaind commented May 4, 2022 •

edited

Loading

bitsandfoxes left a comment •

edited

Loading

vaind May 13, 2022 •

edited

Loading