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

@@ -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<ReleaseResource>();
@@ -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<ReleaseResource>();

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);

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
 {
@@ -50,6 +48,13 @@ namespace NzbDrone.Common.Instrumentation.Sentry
             "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<string> IncludeExceptionMessageTypes = new HashSet<string> {
+            "SQLiteException"
+        };
+        
         private static readonly IDictionary<LogLevel, SentryLevel> LoggingLevelMap = new Dictionary<LogLevel, SentryLevel>
         {
             {LogLevel.Debug, SentryLevel.Debug},
@@ -137,22 +142,25 @@ namespace NzbDrone.Common.Instrumentation.Sentry
 
             var fingerPrint = new List<string>
             {
-                logEvent.Level.Ordinal.ToString(),
+                logEvent.Level.ToString(),
-                logEvent.LoggerName
+                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<string> {
-                        logEvent.Level.ToString(),
-                        logEvent.LoggerName,
-                        logEvent.Message
-                };
-                
                 sentryEvent.SetExtras(extras);
-
+                sentryEvent.SetFingerprint(fingerPrint);
-                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);
 
                 // this can't be in the constructor as at that point OsInfo won't have
                 // populated these values yet

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)

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."

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."

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);
             }
 

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."

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");
             }
         }

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."

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."

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);
             }
         }
 

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, "Failed to import id: {0} - {1}", s.Metadata.Value.ForeignArtistId, s.Metadata.Value.Name);
-                    _logger.Error(ex, ex.Message);
                 }
                 
             }