From dc3c73f72300af57ad18b9671178c0bed2e1206d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:03:03 +0900 Subject: [PATCH 01/11] Enable sentry session tracking --- osu.Game/Utils/SentryLogger.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index d9c8199f75fd..7e0449cec8be 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -25,6 +25,8 @@ public SentryLogger(OsuGame game) var options = new SentryOptions { Dsn = "https://ad9f78529cef40ac874afb95a9aca04e@sentry.ppy.sh/2", + AutoSessionTracking = true, + IsEnvironmentUser = false, Release = game.Version }; From 09c21cde8ca15a02ddd29dba669cb200fa9919fb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:08:42 +0900 Subject: [PATCH 02/11] Add log level translation --- osu.Game/Utils/SentryLogger.cs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 7e0449cec8be..c39d718d6251 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -50,12 +50,37 @@ private void processLogEntry(LogEntry entry) if (lastException != null && lastException.Message == exception.Message && exception.StackTrace.StartsWith(lastException.StackTrace, StringComparison.Ordinal)) return; lastException = exception; - sentry.CaptureEvent(new SentryEvent(exception) { Message = entry.Message }, sentryScope); + sentry.CaptureEvent(new SentryEvent(exception) + { + Message = entry.Message, + Level = getSentryLevel(entry.Level), + }, sentryScope); } else sentryScope.AddBreadcrumb(DateTimeOffset.Now, entry.Message, entry.Target.ToString(), "navigation"); } + private SentryLevel? getSentryLevel(LogLevel entryLevel) + { + switch (entryLevel) + { + case LogLevel.Debug: + return SentryLevel.Debug; + + case LogLevel.Verbose: + return SentryLevel.Info; + + case LogLevel.Important: + return SentryLevel.Warning; + + case LogLevel.Error: + return SentryLevel.Error; + + default: + throw new ArgumentOutOfRangeException(nameof(entryLevel), entryLevel, null); + } + } + private bool shouldSubmitException(Exception exception) { switch (exception) From 64cc6ebddbb090edc7a39bb83415b68dc7f25ece Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:12:31 +0900 Subject: [PATCH 03/11] Add local user tracking to sentry reporting --- osu.Game/OsuGameBase.cs | 2 +- osu.Game/Utils/SentryLogger.cs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 324fcada89a5..3f0610e9aa66 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -125,7 +125,7 @@ public virtual string Version protected MusicController MusicController { get; private set; } - protected IAPIProvider API { get; set; } + protected internal IAPIProvider API { get; protected set; } protected Storage Storage { get; set; } diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index c39d718d6251..5cecc4d7764b 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -4,7 +4,10 @@ using System; using System.IO; using System.Net; +using JetBrains.Annotations; +using osu.Framework.Bindables; using osu.Framework.Logging; +using osu.Game.Online.API.Requests.Responses; using Sentry; namespace osu.Game.Utils @@ -18,6 +21,9 @@ public class SentryLogger : IDisposable private Scope sentryScope; private Exception lastException; + [UsedImplicitly] + private readonly IBindable localUser; + public SentryLogger(OsuGame game) { if (!game.IsDeployedBuild) return; @@ -34,6 +40,16 @@ public SentryLogger(OsuGame game) sentryScope = new Scope(options); Logger.NewEntry += processLogEntry; + + localUser = game.API.LocalUser.GetBoundCopy(); + localUser.BindValueChanged(user => + { + sentryScope.User = new User + { + Username = user.NewValue.Username, + Id = user.NewValue.Id.ToString(), + }; + }); } private void processLogEntry(LogEntry entry) @@ -50,6 +66,7 @@ private void processLogEntry(LogEntry entry) if (lastException != null && lastException.Message == exception.Message && exception.StackTrace.StartsWith(lastException.StackTrace, StringComparison.Ordinal)) return; lastException = exception; + sentry.CaptureEvent(new SentryEvent(exception) { Message = entry.Message, From a5b454edc72537daaaf428bca8b34bf8a9c926b7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:21:49 +0900 Subject: [PATCH 04/11] Remove unnecessary DI caching of `SentryLogger` --- osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs | 2 -- osu.Game/OsuGame.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs b/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs index 0f8337deb645..e4871f611e3e 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs @@ -23,7 +23,6 @@ using osu.Game.Scoring; using osu.Game.Screens.Menu; using osu.Game.Skinning; -using osu.Game.Utils; namespace osu.Game.Tests.Visual.Navigation { @@ -33,7 +32,6 @@ public class TestSceneOsuGame : OsuGameTestScene private IReadOnlyList requiredGameDependencies => new[] { typeof(OsuGame), - typeof(SentryLogger), typeof(OsuLogo), typeof(IdleTracker), typeof(OnScreenDisplay), diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 54c4231b06e2..a6c57998b062 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -258,8 +258,6 @@ private void load() { dependencies.CacheAs(this); - dependencies.Cache(SentryLogger); - dependencies.Cache(osuLogo = new OsuLogo { Alpha = 0 }); // bind config int to database RulesetInfo From 3338bffce3809d78d0f00e6b0b2bf332e62c8af7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:25:10 +0900 Subject: [PATCH 05/11] Attach user to sentry later in startup flow --- osu.Game/OsuGame.cs | 2 ++ osu.Game/OsuGameBase.cs | 2 +- osu.Game/Utils/SentryLogger.cs | 26 +++++++++++++++----------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index a6c57998b062..b8abef38a804 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -258,6 +258,8 @@ private void load() { dependencies.CacheAs(this); + SentryLogger.AttachUser(API.LocalUser); + dependencies.Cache(osuLogo = new OsuLogo { Alpha = 0 }); // bind config int to database RulesetInfo diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 3f0610e9aa66..324fcada89a5 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -125,7 +125,7 @@ public virtual string Version protected MusicController MusicController { get; private set; } - protected internal IAPIProvider API { get; protected set; } + protected IAPIProvider API { get; set; } protected Storage Storage { get; set; } diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 5cecc4d7764b..92f2902c0e27 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -1,10 +1,12 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; +using System.Diagnostics; using System.IO; using System.Net; -using JetBrains.Annotations; using osu.Framework.Bindables; using osu.Framework.Logging; using osu.Game.Online.API.Requests.Responses; @@ -19,10 +21,9 @@ public class SentryLogger : IDisposable { private SentryClient sentry; private Scope sentryScope; - private Exception lastException; + private Exception? lastException; - [UsedImplicitly] - private readonly IBindable localUser; + private IBindable? localUser; public SentryLogger(OsuGame game) { @@ -40,16 +41,21 @@ public SentryLogger(OsuGame game) sentryScope = new Scope(options); Logger.NewEntry += processLogEntry; + } + + public void AttachUser(IBindable user) + { + Debug.Assert(localUser == null); - localUser = game.API.LocalUser.GetBoundCopy(); - localUser.BindValueChanged(user => + localUser = user.GetBoundCopy(); + localUser.BindValueChanged(u => { sentryScope.User = new User { - Username = user.NewValue.Username, - Id = user.NewValue.Id.ToString(), + Username = u.NewValue.Username, + Id = u.NewValue.Id.ToString(), }; - }); + }, true); } private void processLogEntry(LogEntry entry) @@ -137,8 +143,6 @@ public void Dispose() protected virtual void Dispose(bool isDisposing) { Logger.NewEntry -= processLogEntry; - sentry = null; - sentryScope = null; } #endregion From 9734d778f4bda85e2f38cd03e326c09cb3c40ac7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:44:54 +0900 Subject: [PATCH 06/11] Update sentry SDK usage in line with more recent specifications --- osu.Game/Utils/SentryLogger.cs | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 92f2902c0e27..02bcdd281a3c 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -19,26 +19,24 @@ namespace osu.Game.Utils /// public class SentryLogger : IDisposable { - private SentryClient sentry; - private Scope sentryScope; private Exception? lastException; private IBindable? localUser; + private readonly IDisposable? sentrySession; + public SentryLogger(OsuGame game) { - if (!game.IsDeployedBuild) return; - - var options = new SentryOptions + sentrySession = SentrySdk.Init(options => { - Dsn = "https://ad9f78529cef40ac874afb95a9aca04e@sentry.ppy.sh/2", - AutoSessionTracking = true, - IsEnvironmentUser = false, - Release = game.Version - }; + // Not setting the dsn will completely disable sentry. + if (game.IsDeployedBuild) + options.Dsn = "https://ad9f78529cef40ac874afb95a9aca04e@sentry.ppy.sh/2"; - sentry = new SentryClient(options); - sentryScope = new Scope(options); + options.AutoSessionTracking = true; + options.IsEnvironmentUser = false; + options.Release = game.Version; + }); Logger.NewEntry += processLogEntry; } @@ -50,11 +48,11 @@ public void AttachUser(IBindable user) localUser = user.GetBoundCopy(); localUser.BindValueChanged(u => { - sentryScope.User = new User + SentrySdk.ConfigureScope(scope => scope.User = new User { Username = u.NewValue.Username, Id = u.NewValue.Id.ToString(), - }; + }); }, true); } @@ -73,14 +71,14 @@ private void processLogEntry(LogEntry entry) lastException = exception; - sentry.CaptureEvent(new SentryEvent(exception) + SentrySdk.CaptureEvent(new SentryEvent(exception) { Message = entry.Message, Level = getSentryLevel(entry.Level), - }, sentryScope); + }); } else - sentryScope.AddBreadcrumb(DateTimeOffset.Now, entry.Message, entry.Target.ToString(), "navigation"); + SentrySdk.AddBreadcrumb(entry.Message, entry.Target.ToString(), "navigation"); } private SentryLevel? getSentryLevel(LogLevel entryLevel) @@ -143,6 +141,7 @@ public void Dispose() protected virtual void Dispose(bool isDisposing) { Logger.NewEntry -= processLogEntry; + sentrySession?.Dispose(); } #endregion From 99e6d56508799132912238edf898df000f33033a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 14:45:55 +0900 Subject: [PATCH 07/11] Add finalizer to sentry logger for safety --- osu.Game/Utils/SentryLogger.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 02bcdd281a3c..c6429b6b2c0d 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -41,6 +41,8 @@ public SentryLogger(OsuGame game) Logger.NewEntry += processLogEntry; } + ~SentryLogger() => Dispose(false); + public void AttachUser(IBindable user) { Debug.Assert(localUser == null); From c6112b3ae78becb6980053ae047258c103a4db8e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 15:07:02 +0900 Subject: [PATCH 08/11] Add unhandled exception marking --- osu.Game/Utils/SentryLogger.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index c6429b6b2c0d..728d6ab9be1c 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -11,6 +11,7 @@ using osu.Framework.Logging; using osu.Game.Online.API.Requests.Responses; using Sentry; +using Sentry.Protocol; namespace osu.Game.Utils { @@ -73,6 +74,13 @@ private void processLogEntry(LogEntry entry) lastException = exception; + // framework does some weird exception redirection which means sentry does not see unhandled exceptions using its automatic methods. + // but all unhandled exceptions still arrive via this pathway. we just need to mark them as unhandled for tagging purposes. + // easiest solution is to check the message matches what the framework logs this as. + // see https://github.com/ppy/osu-framework/blob/f932f8df053f0011d755c95ad9a2ed61b94d136b/osu.Framework/Platform/GameHost.cs#L336 + bool wasHandled = entry.Message != @"An unhandled exception has occurred."; + exception.Data[Mechanism.HandledKey] = wasHandled; + SentrySdk.CaptureEvent(new SentryEvent(exception) { Message = entry.Message, From 363643a16d64777ffd0daf681507ec0e53d003c3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 15:08:49 +0900 Subject: [PATCH 09/11] Remove sentry logger debounce This is probably going to result in a high quantity of exceptions, but I think this is fine. We can add rules as we go to not log certain exception types. --- osu.Game/Utils/SentryLogger.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 728d6ab9be1c..170d8e7cb0ff 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -20,8 +20,6 @@ namespace osu.Game.Utils /// public class SentryLogger : IDisposable { - private Exception? lastException; - private IBindable? localUser; private readonly IDisposable? sentrySession; @@ -69,11 +67,6 @@ private void processLogEntry(LogEntry entry) { if (!shouldSubmitException(exception)) return; - // since we let unhandled exceptions go ignored at times, we want to ensure they don't get submitted on subsequent reports. - if (lastException != null && lastException.Message == exception.Message && exception.StackTrace.StartsWith(lastException.StackTrace, StringComparison.Ordinal)) return; - - lastException = exception; - // framework does some weird exception redirection which means sentry does not see unhandled exceptions using its automatic methods. // but all unhandled exceptions still arrive via this pathway. we just need to mark them as unhandled for tagging purposes. // easiest solution is to check the message matches what the framework logs this as. From 216c68e6d018246a37870fa2ada898f7def2a080 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 15:19:43 +0900 Subject: [PATCH 10/11] Add unobserved exception hinting --- osu.Game/Utils/SentryLogger.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 170d8e7cb0ff..96affd85a7fb 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -71,8 +71,22 @@ private void processLogEntry(LogEntry entry) // but all unhandled exceptions still arrive via this pathway. we just need to mark them as unhandled for tagging purposes. // easiest solution is to check the message matches what the framework logs this as. // see https://github.com/ppy/osu-framework/blob/f932f8df053f0011d755c95ad9a2ed61b94d136b/osu.Framework/Platform/GameHost.cs#L336 - bool wasHandled = entry.Message != @"An unhandled exception has occurred."; - exception.Data[Mechanism.HandledKey] = wasHandled; + bool wasUnhandled = entry.Message == @"An unhandled error has occurred."; + bool wasUnobserved = entry.Message == @"An unobserved error has occurred."; + + if (wasUnobserved) + { + // see https://github.com/getsentry/sentry-dotnet/blob/c6a660b1affc894441c63df2695a995701671744/src/Sentry/Integrations/TaskUnobservedTaskExceptionIntegration.cs#L39 + exception.Data[Mechanism.MechanismKey] = @"UnobservedTaskException"; + } + + if (wasUnhandled) + { + // see https://github.com/getsentry/sentry-dotnet/blob/main/src/Sentry/Integrations/AppDomainUnhandledExceptionIntegration.cs#L38-L39 + exception.Data[Mechanism.MechanismKey] = @"AppDomain.UnhandledException"; + } + + exception.Data[Mechanism.HandledKey] = !wasUnhandled; SentrySdk.CaptureEvent(new SentryEvent(exception) { From 6a49eb68759d89e4d001866a7ae4fcbb7addc8d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 10 May 2022 16:14:04 +0900 Subject: [PATCH 11/11] Add breadcrumb level mappings --- osu.Game/Utils/SentryLogger.cs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index 96affd85a7fb..ad4bcf62745d 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -95,10 +95,31 @@ private void processLogEntry(LogEntry entry) }); } else - SentrySdk.AddBreadcrumb(entry.Message, entry.Target.ToString(), "navigation"); + SentrySdk.AddBreadcrumb(entry.Message, entry.Target.ToString(), "navigation", level: getBreadcrumbLevel(entry.Level)); } - private SentryLevel? getSentryLevel(LogLevel entryLevel) + private BreadcrumbLevel getBreadcrumbLevel(LogLevel entryLevel) + { + switch (entryLevel) + { + case LogLevel.Debug: + return BreadcrumbLevel.Debug; + + case LogLevel.Verbose: + return BreadcrumbLevel.Info; + + case LogLevel.Important: + return BreadcrumbLevel.Warning; + + case LogLevel.Error: + return BreadcrumbLevel.Error; + + default: + throw new ArgumentOutOfRangeException(nameof(entryLevel), entryLevel, null); + } + } + + private SentryLevel getSentryLevel(LogLevel entryLevel) { switch (entryLevel) {