From 1c7ded859b5ab3e0002e66717287cffb80ce4c70 Mon Sep 17 00:00:00 2001 From: ta264 Date: Wed, 13 Mar 2019 23:10:58 +0000 Subject: [PATCH] Fixed: More improvements to sentry logging (#669) * Only add the exception message for some types * Cleanse exception messages also * Don't put exception message into log It breaks the sentry grouping * Combine the two calculations of fingerprint --- src/Lidarr.Api.V1/Indexers/ReleaseModule.cs | 6 +- .../Instrumentation/Sentry/SentryCleanser.cs | 2 +- .../Instrumentation/Sentry/SentryTarget.cs | 71 +++++-------------- .../Download/Clients/Deluge/Deluge.cs | 2 +- .../DownloadStation/TorrentDownloadStation.cs | 4 +- .../DownloadStation/UsenetDownloadStation.cs | 4 +- .../Download/Clients/Hadouken/Hadouken.cs | 4 +- .../Clients/QBittorrent/QBittorrent.cs | 2 +- .../Download/Clients/Sabnzbd/Sabnzbd.cs | 2 +- .../Clients/Transmission/TransmissionBase.cs | 4 +- .../Download/Clients/uTorrent/UTorrent.cs | 2 +- .../Extras/Metadata/MetadataService.cs | 2 +- src/NzbDrone.Core/Music/AddArtistService.cs | 3 +- 13 files changed, 36 insertions(+), 72 deletions(-) diff --git a/src/Lidarr.Api.V1/Indexers/ReleaseModule.cs b/src/Lidarr.Api.V1/Indexers/ReleaseModule.cs index d7fbc9bf0..d7a170b10 100644 --- a/src/Lidarr.Api.V1/Indexers/ReleaseModule.cs +++ b/src/Lidarr.Api.V1/Indexers/ReleaseModule.cs @@ -69,7 +69,7 @@ namespace Lidarr.Api.V1.Indexers } catch (ReleaseDownloadException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Getting release from indexer failed"); throw new NzbDroneClientException(HttpStatusCode.Conflict, "Getting release from indexer failed"); } @@ -102,7 +102,7 @@ namespace Lidarr.Api.V1.Indexers } catch (Exception ex) { - _logger.Error(ex, "Album search failed: " + ex.Message); + _logger.Error(ex, "Album search failed"); } return new List(); @@ -119,7 +119,7 @@ namespace Lidarr.Api.V1.Indexers } catch (Exception ex) { - _logger.Error(ex, "Artist search failed: " + ex.Message); + _logger.Error(ex, "Artist search failed"); } return new List(); diff --git a/src/NzbDrone.Common/Instrumentation/Sentry/SentryCleanser.cs b/src/NzbDrone.Common/Instrumentation/Sentry/SentryCleanser.cs index d96958ec5..7d351a55a 100644 --- a/src/NzbDrone.Common/Instrumentation/Sentry/SentryCleanser.cs +++ b/src/NzbDrone.Common/Instrumentation/Sentry/SentryCleanser.cs @@ -1,6 +1,5 @@ using System; using System.Linq; -using NzbDrone.Common.EnvironmentInfo; using Sentry; using Sentry.Protocol; @@ -28,6 +27,7 @@ namespace NzbDrone.Common.Instrumentation.Sentry foreach (var exception in sentryEvent.SentryExceptions) { + exception.Value = CleanseLogMessage.Cleanse(exception.Value); foreach (var frame in exception.Stacktrace.Frames) { frame.FileName = ShortenPath(frame.FileName); diff --git a/src/NzbDrone.Common/Instrumentation/Sentry/SentryTarget.cs b/src/NzbDrone.Common/Instrumentation/Sentry/SentryTarget.cs index 54d2cc337..9384f45f4 100644 --- a/src/NzbDrone.Common/Instrumentation/Sentry/SentryTarget.cs +++ b/src/NzbDrone.Common/Instrumentation/Sentry/SentryTarget.cs @@ -9,10 +9,8 @@ using NLog; using NLog.Common; using NLog.Targets; using NzbDrone.Common.EnvironmentInfo; -using System.Globalization; using Sentry; using Sentry.Protocol; -using NzbDrone.Common.Extensions; namespace NzbDrone.Common.Instrumentation.Sentry { @@ -49,6 +47,13 @@ namespace NzbDrone.Common.Instrumentation.Sentry // Fix openflixr being stupid with permissions "openflixr" }; + + // exception types in this list will additionally have the exception message added to the + // sentry fingerprint. Make sure that this message doesn't vary by exception + // (e.g. containing a path or a url) so that the sentry grouping is sensible + private static readonly HashSet IncludeExceptionMessageTypes = new HashSet { + "SQLiteException" + }; private static readonly IDictionary LoggingLevelMap = new Dictionary { @@ -137,22 +142,25 @@ namespace NzbDrone.Common.Instrumentation.Sentry var fingerPrint = new List { - logEvent.Level.Ordinal.ToString(), - logEvent.LoggerName + logEvent.Level.ToString(), + logEvent.LoggerName, + logEvent.Message }; var ex = logEvent.Exception; if (ex != null) { - var exception = ex.GetType().Name; - + fingerPrint.Add(ex.GetType().FullName); + fingerPrint.Add(ex.TargetSite.ToString()); if (ex.InnerException != null) { - exception += ex.InnerException.GetType().Name; + fingerPrint.Add(ex.InnerException.GetType().FullName); + } + else if (IncludeExceptionMessageTypes.Contains(ex.GetType().Name)) + { + fingerPrint.Add(ex?.Message); } - - fingerPrint.Add(exception); } return fingerPrint; @@ -234,51 +242,8 @@ namespace NzbDrone.Common.Instrumentation.Sentry Message = logEvent.FormattedMessage, }; - var sentryFingerprint = new List { - logEvent.Level.ToString(), - logEvent.LoggerName, - logEvent.Message - }; - sentryEvent.SetExtras(extras); - - if (logEvent.Exception != null) - { - sentryFingerprint.Add(logEvent.Exception.GetType().FullName); - sentryFingerprint.Add(logEvent.Exception.TargetSite.ToString()); - - // only try to use the exeception message to fingerprint if there's no inner - // exception and the message is short, otherwise we're in danger of getting a - // stacktrace which will break the grouping - if (logEvent.Exception.InnerException == null) - { - string message = null; - - // bodge to try to get the exception message in English - // https://stackoverflow.com/questions/209133/exception-messages-in-english - // There may still be some localization but this is better than nothing. - var t = new Thread(() => { - message = logEvent.Exception?.Message; - }); - t.CurrentCulture = CultureInfo.InvariantCulture; - t.CurrentUICulture = CultureInfo.InvariantCulture; - t.Start(); - t.Join(); - - if (message.IsNotNullOrWhiteSpace() && message.Length < 200) - { - // Windows gives a trailing '.' for NullReferenceException but mono doesn't - sentryFingerprint.Add(message.TrimEnd('.')); - } - } - } - - if (logEvent.Properties.ContainsKey("Sentry")) - { - sentryFingerprint = ((string[])logEvent.Properties["Sentry"]).ToList(); - } - - sentryEvent.SetFingerprint(sentryFingerprint); + sentryEvent.SetFingerprint(fingerPrint); // this can't be in the constructor as at that point OsInfo won't have // populated these values yet diff --git a/src/NzbDrone.Core/Download/Clients/Deluge/Deluge.cs b/src/NzbDrone.Core/Download/Clients/Deluge/Deluge.cs index cba617f79..f825041cb 100644 --- a/src/NzbDrone.Core/Download/Clients/Deluge/Deluge.cs +++ b/src/NzbDrone.Core/Download/Clients/Deluge/Deluge.cs @@ -210,7 +210,7 @@ namespace NzbDrone.Core.Download.Clients.Deluge } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Password", "Authentication failed"); } catch (WebException ex) diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/TorrentDownloadStation.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/TorrentDownloadStation.cs index 53edffeaf..e028d718c 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/TorrentDownloadStation.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/TorrentDownloadStation.cs @@ -339,7 +339,7 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation } catch (DownloadClientAuthenticationException ex) // User could not have permission to access to downloadstation { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure(string.Empty, ex.Message); } catch (Exception ex) @@ -357,7 +357,7 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Username", "Authentication failure") { DetailedDescription = $"Please verify your username and password. Also verify if the host running Lidarr isn't blocked from accessing {Name} by WhiteList limitations in the {Name} configuration." diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/UsenetDownloadStation.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/UsenetDownloadStation.cs index f35a1c78d..dbe3889a4 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/UsenetDownloadStation.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/UsenetDownloadStation.cs @@ -238,7 +238,7 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation } catch (DownloadClientAuthenticationException ex) // User could not have permission to access to downloadstation { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure(string.Empty, ex.Message); } catch (Exception ex) @@ -256,7 +256,7 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Username", "Authentication failure") { DetailedDescription = $"Please verify your username and password. Also verify if the host running Lidarr isn't blocked from accessing {Name} by WhiteList limitations in the {Name} configuration." diff --git a/src/NzbDrone.Core/Download/Clients/Hadouken/Hadouken.cs b/src/NzbDrone.Core/Download/Clients/Hadouken/Hadouken.cs index d4bd188c9..60c30f61c 100644 --- a/src/NzbDrone.Core/Download/Clients/Hadouken/Hadouken.cs +++ b/src/NzbDrone.Core/Download/Clients/Hadouken/Hadouken.cs @@ -160,7 +160,7 @@ namespace NzbDrone.Core.Download.Clients.Hadouken } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex.Message, ex); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Password", "Authentication failed"); } @@ -176,7 +176,7 @@ namespace NzbDrone.Core.Download.Clients.Hadouken } catch (Exception ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to validate"); return new NzbDroneValidationFailure(String.Empty, "Failed to get the list of torrents: " + ex.Message); } diff --git a/src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrent.cs b/src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrent.cs index 82f6a328f..f9817f6b2 100644 --- a/src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrent.cs +++ b/src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrent.cs @@ -269,7 +269,7 @@ namespace NzbDrone.Core.Download.Clients.QBittorrent } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Username", "Authentication failure") { DetailedDescription = "Please verify your username and password." diff --git a/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs b/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs index 3d20eae70..77f07f558 100644 --- a/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs +++ b/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs @@ -384,7 +384,7 @@ namespace NzbDrone.Core.Download.Clients.Sabnzbd } catch (Exception ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new ValidationFailure("Host", "Unable to connect to SABnzbd"); } } diff --git a/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionBase.cs b/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionBase.cs index ba30ccf64..fb299f0b0 100644 --- a/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionBase.cs +++ b/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionBase.cs @@ -200,7 +200,7 @@ namespace NzbDrone.Core.Download.Clients.Transmission } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Username", "Authentication failure") { DetailedDescription = string.Format("Please verify your username and password. Also verify if the host running Lidarr isn't blocked from accessing {0} by WhiteList limitations in the {0} configuration.", Name) @@ -208,7 +208,7 @@ namespace NzbDrone.Core.Download.Clients.Transmission } catch (DownloadClientUnavailableException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to connect to transmission"); return new NzbDroneValidationFailure("Host", "Unable to connect") { DetailedDescription = "Please verify the hostname and port." diff --git a/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrent.cs b/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrent.cs index ea7fe3db6..041293ce3 100644 --- a/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrent.cs +++ b/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrent.cs @@ -241,7 +241,7 @@ namespace NzbDrone.Core.Download.Clients.UTorrent } catch (DownloadClientAuthenticationException ex) { - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Unable to authenticate"); return new NzbDroneValidationFailure("Username", "Authentication failure") { DetailedDescription = "Please verify your username and password." diff --git a/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs b/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs index 8caaa0bc1..8beccbce0 100644 --- a/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs +++ b/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs @@ -462,7 +462,7 @@ namespace NzbDrone.Core.Extras.Metadata } catch (Exception ex) { - _logger.Error(ex, "Couldn't download image {0} for {1}. {2}", image.Url, artist, ex.Message); + _logger.Error(ex, "Couldn't download image {0} for {1}", image.Url, artist); } } diff --git a/src/NzbDrone.Core/Music/AddArtistService.cs b/src/NzbDrone.Core/Music/AddArtistService.cs index 0c0ae7e74..ee58d7163 100644 --- a/src/NzbDrone.Core/Music/AddArtistService.cs +++ b/src/NzbDrone.Core/Music/AddArtistService.cs @@ -72,8 +72,7 @@ namespace NzbDrone.Core.Music catch (Exception ex) { // Catch Import Errors for now until we get things fixed up - _logger.Debug("Failed to import id: {0} - {1}", s.Metadata.Value.ForeignArtistId, s.Metadata.Value.Name); - _logger.Error(ex, ex.Message); + _logger.Error(ex, "Failed to import id: {0} - {1}", s.Metadata.Value.ForeignArtistId, s.Metadata.Value.Name); } }