diff --git a/CHANGELOG.md b/CHANGELOG.md index c09c4b7f6..9903df83a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Features +- Drastically improved performance of scope sync when targeting Android ([#2107](https://github.com/getsentry/sentry-unity/pull/2107)) - When running on Android, Windows or Linux, the SDK now links errors and events originating on different layers (managed, native errors) via `trace ID` ([#1997](https://github.com/getsentry/sentry-unity/pull/1997), [#2089](https://github.com/getsentry/sentry-unity/pull/2089)) - The SDK now reports the game's name as part of the app context ([2083](https://github.com/getsentry/sentry-unity/pull/2083)) - The SDK now reports the active scene's name as part of the `Unity Context` ([2084](https://github.com/getsentry/sentry-unity/pull/2084)) diff --git a/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs b/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs index 199de6498..55e51705f 100644 --- a/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs +++ b/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs @@ -9,96 +9,31 @@ namespace Sentry.Unity.Android; /// internal class AndroidJavaScopeObserver : ScopeObserver { - private readonly IJniExecutor _jniExecutor; + private ISentryJava _sentryJava; - public AndroidJavaScopeObserver(SentryOptions options, IJniExecutor jniExecutor) : base("Android", options) + public AndroidJavaScopeObserver(SentryOptions options, ISentryJava sentryJava) : base("Android", options) { - _jniExecutor = jniExecutor; + _sentryJava = sentryJava; } - private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); + public override void AddBreadcrumbImpl(Breadcrumb breadcrumb) => + _sentryJava.AddBreadcrumb(breadcrumb); - private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); + public override void SetExtraImpl(string key, string? value) => + _sentryJava.SetExtra(key, value); - public override void AddBreadcrumbImpl(Breadcrumb breadcrumb) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); - javaBreadcrumb.Set("message", breadcrumb.Message); - javaBreadcrumb.Set("type", breadcrumb.Type); - javaBreadcrumb.Set("category", breadcrumb.Category); - using var javaLevel = breadcrumb.Level.ToJavaSentryLevel(); - javaBreadcrumb.Set("level", javaLevel); - sentry.CallStatic("addBreadcrumb", javaBreadcrumb, null); - }); - } + public override void SetTagImpl(string key, string value) => + _sentryJava.SetTag(key, value); - public override void SetExtraImpl(string key, string? value) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("setExtra", key, value); - }); - } - public override void SetTagImpl(string key, string value) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("setTag", key, value); - }); - } + public override void UnsetTagImpl(string key) => + _sentryJava.UnsetTag(key); - public override void UnsetTagImpl(string key) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("removeTag", key); - }); - } + public override void SetUserImpl(SentryUser user) => + _sentryJava.SetUser(user); - public override void SetUserImpl(SentryUser user) - { - _jniExecutor.Run(() => - { - AndroidJavaObject? javaUser = null; - try - { - javaUser = new AndroidJavaObject("io.sentry.protocol.User"); - javaUser.Set("email", user.Email); - javaUser.Set("id", user.Id); - javaUser.Set("username", user.Username); - javaUser.Set("ipAddress", user.IpAddress); - using var sentry = GetSentryJava(); - sentry.CallStatic("setUser", javaUser); - } - finally - { - javaUser?.Dispose(); - } - }); - } + public override void UnsetUserImpl() => + _sentryJava.UnsetUser(); - public override void UnsetUserImpl() - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("setUser", null); - }); - } - - public override void SetTraceImpl(SentryId traceId, SpanId spanId) - { - _jniExecutor.Run(() => - { - using var sentry = GetInternalSentryJava(); - // We have to explicitly cast to `(Double?)` - sentry.CallStatic("setTrace", traceId.ToString(), spanId.ToString(), (Double?)null, (Double?)null); - }); - } + public override void SetTraceImpl(SentryId traceId, SpanId spanId) => + _sentryJava.SetTrace(traceId, spanId); } diff --git a/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs b/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs new file mode 100644 index 000000000..794ff8f59 --- /dev/null +++ b/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs @@ -0,0 +1,36 @@ +using System; +using Sentry.Extensibility; +using UnityEngine; + +namespace Sentry.Unity.Android; + +internal class AndroidOptionsConfiguration : AndroidJavaProxy +{ + private readonly Action _callback; + private readonly IDiagnosticLogger? _logger; + + public AndroidOptionsConfiguration(Action callback, IDiagnosticLogger? logger) + : base("io.sentry.Sentry$OptionsConfiguration") + { + _callback = callback; + _logger = logger; + } + + public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + { + try + { + if (methodName != "configure" || args.Length != 1) + { + throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); + } + + _callback(args[0]); + } + catch (Exception e) + { + _logger?.LogError(e, "Error invoking '{0}'.", methodName); + } + return null; + } +} diff --git a/src/Sentry.Unity.Android/IAndroidJNI.cs b/src/Sentry.Unity.Android/IAndroidJNI.cs new file mode 100644 index 000000000..e18e74478 --- /dev/null +++ b/src/Sentry.Unity.Android/IAndroidJNI.cs @@ -0,0 +1,18 @@ +using UnityEngine; + +namespace Sentry.Unity.Android; + +public interface IAndroidJNI +{ + public void AttachCurrentThread(); + public void DetachCurrentThread(); +} + +public class AndroidJNIAdapter : IAndroidJNI +{ + public static readonly AndroidJNIAdapter Instance = new(); + + public void AttachCurrentThread() => AndroidJNI.AttachCurrentThread(); + + public void DetachCurrentThread() => AndroidJNI.DetachCurrentThread(); +} diff --git a/src/Sentry.Unity.Android/IJniExecutor.cs b/src/Sentry.Unity.Android/IJniExecutor.cs deleted file mode 100644 index f465602cd..000000000 --- a/src/Sentry.Unity.Android/IJniExecutor.cs +++ /dev/null @@ -1,9 +0,0 @@ -using System; - -namespace Sentry.Unity.Android; - -internal interface IJniExecutor : IDisposable -{ - public TResult? Run(Func jniOperation, TimeSpan? timeout = null); - public void Run(Action jniOperation, TimeSpan? timeout = null); -} diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs deleted file mode 100644 index 079484b6d..000000000 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ /dev/null @@ -1,182 +0,0 @@ -using System; -using System.Threading; -using System.Threading.Tasks; -using Sentry.Extensibility; -using UnityEngine; - -namespace Sentry.Unity.Android; - -internal class JniExecutor : IJniExecutor -{ - // We're capping out at 16ms - 1 frame at 60 frames per second - private static readonly TimeSpan DefaultTimeout = TimeSpan.FromMilliseconds(16); - - private readonly CancellationTokenSource _shutdownSource; - private readonly AutoResetEvent _taskEvent; - private readonly IDiagnosticLogger? _logger; - - private Delegate _currentTask = null!; // The current task will always be set together with the task event - - private TaskCompletionSource? _taskCompletionSource; - - private readonly object _lock = new object(); - - private bool _isDisposed; - private Thread? _workerThread; - - public JniExecutor(IDiagnosticLogger? logger) - { - _logger = logger; - _taskEvent = new AutoResetEvent(false); - _shutdownSource = new CancellationTokenSource(); - - _workerThread = new Thread(DoWork) { IsBackground = true, Name = "SentryJniExecutorThread" }; - _workerThread.Start(); - } - - private void DoWork() - { - AndroidJNI.AttachCurrentThread(); - - var waitHandles = new[] { _taskEvent, _shutdownSource.Token.WaitHandle }; - - while (!_isDisposed) - { - var index = WaitHandle.WaitAny(waitHandles); - if (index > 0) - { - // We only care about the _taskEvent - break; - } - - try - { - // Matching the type of the `_currentTask` exactly. The result gets cast to the expected type - // when returning from the blocking call. - switch (_currentTask) - { - case Action action: - { - action.Invoke(); - _taskCompletionSource?.SetResult(null); - break; - } - case Func func1: - { - var result = func1.Invoke(); - _taskCompletionSource?.SetResult(result); - break; - } - case Func func2: - { - var result = func2.Invoke(); - _taskCompletionSource?.SetResult(result); - break; - } - case Func func3: - { - var result = func3.Invoke(); - _taskCompletionSource?.SetResult(result); - break; - } - default: - throw new NotImplementedException($"Task type '{_currentTask?.GetType()}' with value '{_currentTask}' is not implemented in the JniExecutor."); - } - } - catch (Exception e) - { - _logger?.LogError(e, "Error during JNI execution."); - _taskCompletionSource?.SetException(e); - } - } - - AndroidJNI.DetachCurrentThread(); - } - - public TResult? Run(Func jniOperation, TimeSpan? timeout = null) - { - lock (_lock) - { - timeout ??= DefaultTimeout; - using var timeoutCts = new CancellationTokenSource(timeout.Value); - _taskCompletionSource = new TaskCompletionSource(); - _currentTask = jniOperation; - _taskEvent.Set(); - - try - { - _taskCompletionSource.Task.Wait(timeoutCts.Token); - return (TResult?)_taskCompletionSource.Task.Result; - } - catch (OperationCanceledException) - { - _logger?.LogError("JNI execution timed out."); - return default; - } - catch (Exception e) - { - _logger?.LogError(e, "Error during JNI execution."); - return default; - } - finally - { - _currentTask = null!; - } - } - } - - public void Run(Action jniOperation, TimeSpan? timeout = null) - { - lock (_lock) - { - timeout ??= DefaultTimeout; - using var timeoutCts = new CancellationTokenSource(timeout.Value); - _taskCompletionSource = new TaskCompletionSource(); - _currentTask = jniOperation; - _taskEvent.Set(); - - try - { - _taskCompletionSource.Task.Wait(timeoutCts.Token); - } - catch (OperationCanceledException) - { - _logger?.LogError("JNI execution timed out."); - } - catch (Exception e) - { - _logger?.LogError(e, "Error during JNI execution."); - } - finally - { - _currentTask = null!; - } - } - } - - public void Dispose() - { - if (_isDisposed) - { - return; - } - - _isDisposed = true; - - _shutdownSource.Cancel(); - try - { - _workerThread?.Join(100); - } - catch (ThreadStateException) - { - _logger?.LogError("JNI Executor Worker thread was never started during disposal"); - } - catch (ThreadInterruptedException) - { - _logger?.LogError("JNI Executor Worker thread was interrupted during disposal"); - } - - _taskEvent.Dispose(); - } -} diff --git a/src/Sentry.Unity.Android/NativeContextWriter.cs b/src/Sentry.Unity.Android/NativeContextWriter.cs index d1db73e4e..573a8f2e6 100644 --- a/src/Sentry.Unity.Android/NativeContextWriter.cs +++ b/src/Sentry.Unity.Android/NativeContextWriter.cs @@ -4,12 +4,10 @@ namespace Sentry.Unity.Android; internal class NativeContextWriter : ContextWriter { - private readonly IJniExecutor _jniExecutor; private readonly ISentryJava _sentryJava; - public NativeContextWriter(IJniExecutor jniExecutor, ISentryJava sentryJava) + public NativeContextWriter(ISentryJava sentryJava) { - _jniExecutor = jniExecutor; _sentryJava = sentryJava; } @@ -53,7 +51,6 @@ protected override void WriteScope( // the "unity" context, but it doesn't seem so useful and the effort to do is larger because there's no // class for it in Java - not sure how we could add a generic context object in Java... _sentryJava.WriteScope( - _jniExecutor, GpuId, GpuName, GpuVendorName, diff --git a/src/Sentry.Unity.Android/ScopeCallback.cs b/src/Sentry.Unity.Android/ScopeCallback.cs new file mode 100644 index 000000000..78c2dae99 --- /dev/null +++ b/src/Sentry.Unity.Android/ScopeCallback.cs @@ -0,0 +1,38 @@ +// Implements the io.sentry.ScopeCallback interface. + +using System; +using UnityEngine; + +namespace Sentry.Unity.Android; + +internal class ScopeCallback : AndroidJavaProxy +{ + private readonly Action _callback; + + public ScopeCallback(Action callback) : base("io.sentry.ScopeCallback") + { + _callback = callback; + } + + // Note: defining the method should be enough with the default Invoke(), but in reality it doesn't work: + // No such proxy method: Sentry.Unity.Android.SentryJava+ScopeCallback.run(UnityEngine.AndroidJavaObject) + // public void run(AndroidJavaObject scope) => UnityEngine.Debug.Log("run() invoked"); + // Therefore, we're overriding the Invoke() instead: + public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + { + try + { + if (methodName != "run" || args.Length != 1) + { + throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); + } + _callback(args[0]); + } + catch (Exception e) + { + // Adding the Sentry logger tag ensures we don't send this error to Sentry. + Debug.unityLogger.Log(LogType.Error, UnityLogger.LogTag, $"Error in SentryJava.ScopeCallback: {e}"); + } + return null; + } +} diff --git a/src/Sentry.Unity.Android/SentryJava.cs b/src/Sentry.Unity.Android/SentryJava.cs index 679be6d71..55378909b 100644 --- a/src/Sentry.Unity.Android/SentryJava.cs +++ b/src/Sentry.Unity.Android/SentryJava.cs @@ -1,20 +1,17 @@ using System; -using System.Diagnostics; using Sentry.Extensibility; using UnityEngine; -using Debug = UnityEngine.Debug; namespace Sentry.Unity.Android; internal interface ISentryJava { - public bool IsEnabled(IJniExecutor jniExecutor, TimeSpan timeout); - public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout); - public string? GetInstallationId(IJniExecutor jniExecutor); - public bool? CrashedLastRun(IJniExecutor jniExecutor); - public void Close(IJniExecutor jniExecutor); + public bool? IsEnabled(); + public void Init(SentryUnityOptions options); + public string? GetInstallationId(); + public bool? CrashedLastRun(); + public void Close(); public void WriteScope( - IJniExecutor jniExecutor, int? GpuId, string? GpuName, string? GpuVendorName, @@ -31,6 +28,15 @@ public void WriteScope( bool? GpuMultiThreadedRendering, string? GpuGraphicsShaderLevel); public bool IsSentryJavaPresent(); + + // Methods for the ScopeObserver + public void AddBreadcrumb(Breadcrumb breadcrumb); + public void SetExtra(string key, string? value); + public void SetTag(string key, string? value); + public void UnsetTag(string key); + public void SetUser(SentryUser user); + public void UnsetUser(); + public void SetTrace(SentryId traceId, SpanId spanId); } /// @@ -43,20 +49,43 @@ public void WriteScope( /// internal class SentryJava : ISentryJava { - private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); + private readonly IAndroidJNI _androidJNI; + private IDiagnosticLogger? _logger; + private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); + protected virtual AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); - public bool IsEnabled(IJniExecutor jniExecutor, TimeSpan timeout) + public SentryJava(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null) { - return jniExecutor.Run(() => + _logger = logger; + _androidJNI ??= androidJNI ?? AndroidJNIAdapter.Instance; + } + + public bool? IsEnabled() + { + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); return sentry.CallStatic("isEnabled"); - }, timeout); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.IsEnabled' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + + return null; } - public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout) + public void Init(SentryUnityOptions options) { - jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = new AndroidJavaClass("io.sentry.android.core.SentryAndroid"); using var context = new AndroidJavaClass("com.unity3d.player.UnityPlayer") @@ -96,53 +125,42 @@ public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan androidOptions.Call("setAnrEnabled", false); androidOptions.Call("setEnableScopePersistence", false); }, options.DiagnosticLogger)); - }, timeout); - } - - internal class AndroidOptionsConfiguration : AndroidJavaProxy - { - private readonly Action _callback; - private readonly IDiagnosticLogger? _logger; - - public AndroidOptionsConfiguration(Action callback, IDiagnosticLogger? logger) - : base("io.sentry.Sentry$OptionsConfiguration") + } + catch (Exception e) { - _callback = callback; - _logger = logger; + _logger?.LogError(e, "Calling 'SentryJava.Init' failed."); } - - public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + finally { - try - { - if (methodName != "configure" || args.Length != 1) - { - throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); - } - - _callback(args[0]); - } - catch (Exception e) - { - _logger?.LogError(e, "Error invoking {0} ’.", methodName); - } - return null; + HandleJniThreadDetachment(); } } - public string? GetInstallationId(IJniExecutor jniExecutor) + public string? GetInstallationId() { - return jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); using var hub = sentry.CallStatic("getCurrentHub"); using var options = hub?.Call("getOptions"); return options?.Call("getDistinctId"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.GetInstallationId' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + + return null; } /// - /// Returns whether or not the last run resulted in a crash. + /// Returns whether the last run resulted in a crash. /// /// /// This value is returned by the Android SDK and reports for both ART and NDK. @@ -151,27 +169,48 @@ public AndroidOptionsConfiguration(Action callback, IDiagnost /// True if the last run terminated in a crash. No otherwise. /// If the SDK wasn't able to find this information, null is returned. /// - public bool? CrashedLastRun(IJniExecutor jniExecutor) + public bool? CrashedLastRun() { - return jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); using var jo = sentry.CallStatic("isCrashedLastRun"); return jo?.Call("booleanValue"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.CrashedLastRun' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + + return null; } - public void Close(IJniExecutor jniExecutor) + public void Close() { - jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); sentry.CallStatic("close"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.Close' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void WriteScope( - IJniExecutor jniExecutor, int? GpuId, string? GpuName, string? GpuVendorName, @@ -188,7 +227,9 @@ public void WriteScope( bool? GpuMultiThreadedRendering, string? GpuGraphicsShaderLevel) { - jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var gpu = new AndroidJavaObject("io.sentry.protocol.Gpu"); gpu.SetIfNotNull("name", GpuName); @@ -206,7 +247,15 @@ public void WriteScope( using var contexts = scope.Call("getContexts"); contexts.Call("setGpu", gpu); })); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.WriteScope' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public bool IsSentryJavaPresent() @@ -223,36 +272,150 @@ public bool IsSentryJavaPresent() return true; } - // Implements the io.sentry.ScopeCallback interface. - internal class ScopeCallback : AndroidJavaProxy + public void AddBreadcrumb(Breadcrumb breadcrumb) { - private readonly Action _callback; + HandleJniThreadAttachment(); - public ScopeCallback(Action callback) : base("io.sentry.ScopeCallback") + try + { + using var sentry = GetSentryJava(); + using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); + javaBreadcrumb.Set("message", breadcrumb.Message); + javaBreadcrumb.Set("type", breadcrumb.Type); + javaBreadcrumb.Set("category", breadcrumb.Category); + using var javaLevel = breadcrumb.Level.ToJavaSentryLevel(); + javaBreadcrumb.Set("level", javaLevel); + sentry.CallStatic("addBreadcrumb", javaBreadcrumb, null); + } + catch (Exception e) { - _callback = callback; + _logger?.LogError(e, "Calling 'SentryJava.AddBreadcrumb' failed."); } + finally + { + HandleJniThreadDetachment(); + } + } + + public void SetExtra(string key, string? value) + { + HandleJniThreadAttachment(); - // Note: defining the method should be enough with the default Invoke(), but in reality it doesn't work: - // No such proxy method: Sentry.Unity.Android.SentryJava+ScopeCallback.run(UnityEngine.AndroidJavaObject) - // public void run(AndroidJavaObject scope) => UnityEngine.Debug.Log("run() invoked"); - // Therefore, we're overriding the Invoke() instead: - public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + try { - try - { - if (methodName != "run" || args.Length != 1) - { - throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); - } - _callback(args[0]); - } - catch (Exception e) - { - // Adding the Sentry logger tag ensures we don't send this error to Sentry. - Debug.unityLogger.Log(LogType.Error, UnityLogger.LogTag, $"Error in SentryJava.ScopeCallback: {e}"); - } - return null; + using var sentry = GetSentryJava(); + sentry.CallStatic("setExtra", key, value); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetExtra' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + } + + public void SetTag(string key, string? value) + { + HandleJniThreadAttachment(); + + try + { + using var sentry = GetSentryJava(); + sentry.CallStatic("setTag", key, value); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetTag' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + } + + public void UnsetTag(string key) + { + HandleJniThreadAttachment(); + + try + { + using var sentry = GetSentryJava(); + sentry.CallStatic("removeTag", key); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.UnsetTag' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + } + + public void SetUser(SentryUser user) + { + HandleJniThreadAttachment(); + AndroidJavaObject? javaUser = null; + + try + { + javaUser = new AndroidJavaObject("io.sentry.protocol.User"); + javaUser.Set("email", user.Email); + javaUser.Set("id", user.Id); + javaUser.Set("username", user.Username); + javaUser.Set("ipAddress", user.IpAddress); + using var sentry = GetSentryJava(); + sentry.CallStatic("setUser", javaUser); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetUser' failed."); + } + finally + { + javaUser?.Dispose(); + HandleJniThreadDetachment(); + } + } + + public void UnsetUser() + { + HandleJniThreadAttachment(); + + try + { + using var sentry = GetSentryJava(); + sentry.CallStatic("setUser", null); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.UnsetUser' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + } + + public void SetTrace(SentryId traceId, SpanId spanId) + { + HandleJniThreadAttachment(); + + try + { + using var sentry = GetInternalSentryJava(); + // We have to explicitly cast to `(Double?)` + sentry.CallStatic("setTrace", traceId.ToString(), spanId.ToString(), (Double?)null, (Double?)null); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetTrace' failed."); + } + finally + { + HandleJniThreadDetachment(); } } @@ -266,6 +429,24 @@ public ScopeCallback(Action callback) : base("io.sentry.Scope SentryLevel.Warning => "WARNING", _ => "DEBUG" }; + + internal void HandleJniThreadAttachment(bool? isMainThread = null) + { + isMainThread ??= MainThreadData.IsMainThread(); + if (isMainThread is false) + { + _androidJNI.AttachCurrentThread(); + } + } + + internal void HandleJniThreadDetachment(bool? isMainThread = null) + { + isMainThread ??= MainThreadData.IsMainThread(); + if (isMainThread is false) + { + _androidJNI.DetachCurrentThread(); + } + } } internal static class AndroidJavaObjectExtension diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 1d2122939..76ee1f7ae 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -11,8 +11,9 @@ namespace Sentry.Unity.Android; /// public static class SentryNativeAndroid { - internal static IJniExecutor? JniExecutor; - internal static ISentryJava SentryJava = new SentryJava(); + // This is an internal static field that gets overwritten during testing. We cannot have it as optional + // parameter on `Configure` due SentryNativeAndroid being public + internal static ISentryJava? SentryJava; /// /// Configures the native Android support. @@ -20,6 +21,8 @@ public static class SentryNativeAndroid /// The Sentry Unity options to use. public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentryUnityInfo) { + MainThreadData.CollectData(); + options.DiagnosticLogger?.LogInfo("Attempting to configure native support via the Android SDK"); if (!options.AndroidNativeSupportEnabled) @@ -30,6 +33,8 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK is present."); + // If it's not been set (in a test) + SentryJava ??= new SentryJava(options.DiagnosticLogger); if (!SentryJava.IsSentryJavaPresent()) { options.DiagnosticLogger?.LogError("Android Native Support has been enabled but the " + @@ -39,11 +44,9 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry return; } - JniExecutor ??= new JniExecutor(options.DiagnosticLogger); - options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK has already been initialized"); - if (SentryJava.IsEnabled(JniExecutor, TimeSpan.FromMilliseconds(200))) + if (SentryJava.IsEnabled() is true) { options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized"); } @@ -51,12 +54,11 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry { options.DiagnosticLogger?.LogInfo("Initializing the Android SDK"); - // Local testing had Init at an average of about 25ms. - SentryJava.Init(JniExecutor, options, TimeSpan.FromMilliseconds(200)); + SentryJava.Init(options); options.DiagnosticLogger?.LogDebug("Validating Android SDK initialization"); - if (!SentryJava.IsEnabled(JniExecutor, TimeSpan.FromMilliseconds(200))) + if (SentryJava.IsEnabled() is not true) { options.DiagnosticLogger?.LogError("Failed to initialize Android Native Support"); return; @@ -65,14 +67,14 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Configuring scope sync"); - options.NativeContextWriter = new NativeContextWriter(JniExecutor, SentryJava); - options.ScopeObserver = new AndroidJavaScopeObserver(options, JniExecutor); + options.NativeContextWriter = new NativeContextWriter(SentryJava); + options.ScopeObserver = new AndroidJavaScopeObserver(options, SentryJava); options.EnableScopeSync = true; options.CrashedLastRun = () => { options.DiagnosticLogger?.LogDebug("Checking for 'CrashedLastRun'"); - var crashedLastRun = SentryJava.CrashedLastRun(JniExecutor); + var crashedLastRun = SentryJava.CrashedLastRun(); if (crashedLastRun is null) { // Could happen if the Android SDK wasn't initialized before the .NET layer. @@ -107,7 +109,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Fetching installation ID"); - options.DefaultUserId = SentryJava.GetInstallationId(JniExecutor); + options.DefaultUserId = SentryJava.GetInstallationId(); if (string.IsNullOrEmpty(options.DefaultUserId)) { // In case we can't get an installation ID we create one and sync that down to the native layer @@ -141,19 +143,19 @@ internal static void Close(SentryUnityOptions options, ISentryUnityInfo sentryUn { options.DiagnosticLogger?.LogInfo("Attempting to close the Android SDK"); - if (!sentryUnityInfo.IsNativeSupportEnabled(options, platform) || !SentryJava.IsSentryJavaPresent()) + if (!sentryUnityInfo.IsNativeSupportEnabled(options, platform)) { - options.DiagnosticLogger?.LogDebug("Android Native Support is not enable. Skipping closing the Android SDK"); + options.DiagnosticLogger?.LogDebug("Android Native Support is not enabled. Skipping closing the Android SDK"); return; } - // Sentry Native is initialized and closed by the Java SDK, no need to call into it directly - options.DiagnosticLogger?.LogDebug("Closing the Android SDK"); + if (SentryJava?.IsSentryJavaPresent() is not true) + { + options.DiagnosticLogger?.LogDebug("Failed to find Sentry Java. Skipping closing the Android SDK"); + return; + } - // This is an edge-case where the Android SDK has been enabled and setup during build-time but is being - // shut down at runtime. In this case Configure() has not been called and there is no JniExecutor yet - JniExecutor ??= new JniExecutor(options.DiagnosticLogger); - SentryJava?.Close(JniExecutor); - JniExecutor.Dispose(); + options.DiagnosticLogger?.LogDebug("Closing the Android SDK"); + SentryJava.Close(); } } diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs deleted file mode 100644 index 30e296e91..000000000 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ /dev/null @@ -1,114 +0,0 @@ -using System; -using System.Linq; -using System.Threading; -using NUnit.Framework; -using Sentry.Unity.Tests.SharedClasses; - -namespace Sentry.Unity.Android.Tests -{ - [TestFixture] - public class JniExecutorTests - { - private TestLogger _logger = null!; // Set during SetUp - private JniExecutor _sut = null!; // Set during SetUp - - [SetUp] - public void SetUp() - { - _logger = new TestLogger(); - _sut = new JniExecutor(_logger); - } - - [TearDown] - public void TearDown() - { - _sut.Dispose(); - } - - [Test] - public void Run_Action_ExecutesSuccessfully() - { - // Arrange - var executed = false; - var action = () => executed = true; - - // Act - _sut.Run(action); - - // Assert - Assert.That(executed, Is.True); - } - - [Test] - public void Run_FuncBool_ReturnsExpectedResult() - { - // Arrange - var func = () => true; - - // Act - var result = _sut.Run(func); - - // Assert - Assert.That(result, Is.True); - } - - [Test] - public void Run_FuncString_ReturnsExpectedResult() - { - // Arrange - const string? expected = "Hello"; - var func = () => expected; - - // Act - var result = _sut.Run(func); - - // Assert - Assert.AreEqual(expected, result); - } - - [Test] - public void Run_WithTimeout_LogsErrorOnTimeout() - { - // Arrange - var slowAction = () => Thread.Sleep(100); - var timeout = TimeSpan.FromMilliseconds(50); - - // Act - _sut.Run(slowAction, timeout); - - // Assert - Assert.IsTrue(_logger.Logs.Any(log => - log.logLevel == SentryLevel.Error && - log.message.Contains("JNI execution timed out."))); - } - - [Test] - public void Run_ThrowingOperation_LogsError() - { - // Arrange - var exception = new Exception("Test exception"); - Action throwingAction = () => throw exception; - - // Act - _sut.Run(throwingAction); - - // Assert - Assert.IsTrue(_logger.Logs.Any(log => - log.logLevel == SentryLevel.Error && - log.message.Contains("Error during JNI execution."))); - } - - [Test] - public void Run_Generic_ReturnsDefaultOnException() - { - // Arrange - Func throwingFunc = () => throw new Exception("Test exception"); - - // Act - var result = _sut.Run(throwingFunc); - - // Assert - Assert.That(result, Is.Null); - } - } -} diff --git a/test/Sentry.Unity.Android.Tests/SentryJavaTests.cs b/test/Sentry.Unity.Android.Tests/SentryJavaTests.cs new file mode 100644 index 000000000..efbf71bab --- /dev/null +++ b/test/Sentry.Unity.Android.Tests/SentryJavaTests.cs @@ -0,0 +1,53 @@ +using NUnit.Framework; +using Sentry.Unity.Tests.SharedClasses; + +namespace Sentry.Unity.Android.Tests; + +public class SentryJavaTests +{ + private TestLogger _logger = null!; + private TestAndroidJNI _androidJni = null!; + private SentryJava _sut = null!; + + [SetUp] + public void SetUp() + { + _logger = new TestLogger(); + _androidJni = new TestAndroidJNI(); + _sut = new SentryJava(_logger, _androidJni); + } + + [Test] + [TestCase(true, false, Description = "main thread = should attach")] + [TestCase(false, true, Description = "non main thread = should not attach")] + public void HandleJniThreadAttachment_AttachesIfMainThread(bool isMainThread, bool shouldAttach) + { + // Act + _sut.HandleJniThreadAttachment(isMainThread); + + // Assert + Assert.AreEqual(shouldAttach, _androidJni.AttachCalled); + } + + [Test] + [TestCase(true, false, Description = "main thread = should detach")] + [TestCase(false, true, Description = "non main thread = should not detach")] + public void HandleJniThreadDetachment_DetachesIfMainThread(bool isMainThread, bool shouldAttach) + { + // Act + _sut.HandleJniThreadDetachment(isMainThread); + + // Assert + Assert.AreEqual(shouldAttach, _androidJni.DetachCalled); + } + + internal class TestAndroidJNI : IAndroidJNI + { + public bool AttachCalled { get; private set; } + public bool DetachCalled { get; private set; } + + public void AttachCurrentThread() => AttachCalled = true; + + public void DetachCurrentThread() => DetachCalled = true; + } +} diff --git a/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs b/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs index f75c6f0b1..d52e90650 100644 --- a/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs +++ b/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs @@ -28,7 +28,6 @@ public void SetUp() _reinstallCalled = false; _sentryUnityInfo = new TestUnityInfo { IL2CPP = false }; - SentryNativeAndroid.JniExecutor ??= new JniExecutor(_logger); _testSentryJava = new TestSentryJava(); SentryNativeAndroid.SentryJava = _testSentryJava; diff --git a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs index be1139782..a5c44e1bb 100644 --- a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs +++ b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs @@ -1,4 +1,5 @@ using System; +using Sentry.Extensibility; namespace Sentry.Unity.Android.Tests; @@ -10,19 +11,17 @@ internal class TestSentryJava : ISentryJava public string? InstallationId { get; set; } public bool? IsCrashedLastRun { get; set; } - public bool IsEnabled(IJniExecutor jniExecutor, TimeSpan timeout) => Enabled; + public bool? IsEnabled() => Enabled; - public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout) { } + public void Init(SentryUnityOptions options) { } - public string? GetInstallationId(IJniExecutor jniExecutor) => InstallationId; + public string? GetInstallationId() => InstallationId; - public bool? CrashedLastRun(IJniExecutor jniExecutor) => IsCrashedLastRun; + public bool? CrashedLastRun() => IsCrashedLastRun; - public void Close(IJniExecutor jniExecutor) { } - public void SetTrace(IJniExecutor jniExecutor, string traceId, string spanId) { } + public void Close() { } public void WriteScope( - IJniExecutor jniExecutor, int? GpuId, string? GpuName, string? GpuVendorName, @@ -41,4 +40,16 @@ public void WriteScope( { } public bool IsSentryJavaPresent() => SentryPresent; + public void AddBreadcrumb(Breadcrumb breadcrumb) { } + + public void SetExtra(string key, string? value) { } + public void SetTag(string key, string? value) { } + + public void UnsetTag(string key) { } + + public void SetUser(SentryUser user) { } + + public void UnsetUser() { } + + public void SetTrace(SentryId traceId, SpanId spanId) { } }