From 06b68772bd3e7672a936984dacd39ce34ff11270 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Sun, 22 Sep 2024 12:52:06 -0500 Subject: [PATCH] refactor: Configurator design pattern for log configuration Primary motivation for this change is to create separation of concerns for log sink configuration. The LoggerFactory was becoming too large and had too many responsibilities. Later on this will help facilitate extension of log functionality while respecting the Open Closed Principle. --- Directory.Packages.props | 3 +- Recyclarr.sln | 6 ++ src/Recyclarr.Cli/CompositionRoot.cs | 14 ++-- .../Logging/ConsoleLogSinkConfigurator.cs | 23 ++++++ .../Logging/FileLogSinkConfigurator.cs | 40 +++++++++++ src/Recyclarr.Cli/Logging/LoggerFactory.cs | 72 ------------------- src/Recyclarr.Cli/Recyclarr.Cli.csproj | 1 + src/Recyclarr.Logging/ExceptionExtensions.cs | 23 ++++++ .../FlurlExceptionSanitizingEnricher.cs | 3 +- src/Recyclarr.Logging/ILogConfigurator.cs | 8 +++ src/Recyclarr.Logging/LogProperty.cs | 6 ++ src/Recyclarr.Logging/LogTemplates.cs | 16 +++++ src/Recyclarr.Logging/LoggerFactory.cs | 22 ++++++ src/Recyclarr.Logging/LoggingAutofacModule.cs | 18 +++++ .../Recyclarr.Logging.csproj | 10 +++ src/Recyclarr.Logging/Sanitize.cs | 40 +++++++++++ 16 files changed, 224 insertions(+), 81 deletions(-) create mode 100644 src/Recyclarr.Cli/Logging/ConsoleLogSinkConfigurator.cs create mode 100644 src/Recyclarr.Cli/Logging/FileLogSinkConfigurator.cs delete mode 100644 src/Recyclarr.Cli/Logging/LoggerFactory.cs create mode 100644 src/Recyclarr.Logging/ExceptionExtensions.cs rename src/{Recyclarr.Cli/Logging => Recyclarr.Logging}/FlurlExceptionSanitizingEnricher.cs (91%) create mode 100644 src/Recyclarr.Logging/ILogConfigurator.cs create mode 100644 src/Recyclarr.Logging/LogProperty.cs create mode 100644 src/Recyclarr.Logging/LogTemplates.cs create mode 100644 src/Recyclarr.Logging/LoggerFactory.cs create mode 100644 src/Recyclarr.Logging/LoggingAutofacModule.cs create mode 100644 src/Recyclarr.Logging/Recyclarr.Logging.csproj create mode 100644 src/Recyclarr.Logging/Sanitize.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 03b4ab18..3ea57025 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,6 +14,7 @@ + @@ -77,4 +78,4 @@ - + \ No newline at end of file diff --git a/Recyclarr.sln b/Recyclarr.sln index 8d3c5b26..e00e2970 100644 --- a/Recyclarr.sln +++ b/Recyclarr.sln @@ -61,6 +61,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Recyclarr.Cache", "src\Recy EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Recyclarr.Cli.TestLibrary", "tests\Recyclarr.Cli.TestLibrary\Recyclarr.Cli.TestLibrary.csproj", "{7B730BDB-1519-4847-BA23-998AB3750E31}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Recyclarr.Logging", "src\Recyclarr.Logging\Recyclarr.Logging.csproj", "{0CE6D5F2-A7DC-4C03-B367-ECF8D06FC51E}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -155,6 +157,10 @@ Global {7B730BDB-1519-4847-BA23-998AB3750E31}.Debug|Any CPU.Build.0 = Debug|Any CPU {7B730BDB-1519-4847-BA23-998AB3750E31}.Release|Any CPU.ActiveCfg = Release|Any CPU {7B730BDB-1519-4847-BA23-998AB3750E31}.Release|Any CPU.Build.0 = Release|Any CPU + {0CE6D5F2-A7DC-4C03-B367-ECF8D06FC51E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {0CE6D5F2-A7DC-4C03-B367-ECF8D06FC51E}.Debug|Any CPU.Build.0 = Debug|Any CPU + {0CE6D5F2-A7DC-4C03-B367-ECF8D06FC51E}.Release|Any CPU.ActiveCfg = Release|Any CPU + {0CE6D5F2-A7DC-4C03-B367-ECF8D06FC51E}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/src/Recyclarr.Cli/CompositionRoot.cs b/src/Recyclarr.Cli/CompositionRoot.cs index f395b08c..96c35f7e 100644 --- a/src/Recyclarr.Cli/CompositionRoot.cs +++ b/src/Recyclarr.Cli/CompositionRoot.cs @@ -21,6 +21,7 @@ using Recyclarr.Compatibility; using Recyclarr.Config; using Recyclarr.Http; using Recyclarr.Json; +using Recyclarr.Logging; using Recyclarr.Platform; using Recyclarr.Repo; using Recyclarr.ServarrApi; @@ -28,7 +29,6 @@ using Recyclarr.Settings; using Recyclarr.TrashGuide; using Recyclarr.VersionControl; using Recyclarr.Yaml; -using Serilog.Core; using Spectre.Console.Cli; namespace Recyclarr.Cli; @@ -42,7 +42,7 @@ public static class CompositionRoot // Needed for Autofac.Extras.Ordering builder.RegisterSource(); - RegisterLogger(builder); + RegisterLogger(builder, thisAssembly); builder.RegisterModule(); builder.RegisterModule(); @@ -89,12 +89,14 @@ public static class CompositionRoot .OrderByRegistration(); } - private static void RegisterLogger(ContainerBuilder builder) + private static void RegisterLogger(ContainerBuilder builder, Assembly thisAssembly) { + builder.RegisterAssemblyTypes(thisAssembly) + .AssignableTo() + .As(); + + builder.RegisterModule(); builder.RegisterType(); - builder.RegisterType().SingleInstance(); - builder.RegisterType(); - builder.Register(c => c.Resolve().Create()).As().SingleInstance(); } private static void CliRegistrations(ContainerBuilder builder) diff --git a/src/Recyclarr.Cli/Logging/ConsoleLogSinkConfigurator.cs b/src/Recyclarr.Cli/Logging/ConsoleLogSinkConfigurator.cs new file mode 100644 index 00000000..d451d4b9 --- /dev/null +++ b/src/Recyclarr.Cli/Logging/ConsoleLogSinkConfigurator.cs @@ -0,0 +1,23 @@ +using Recyclarr.Logging; +using Recyclarr.Platform; +using Serilog.Core; +using Serilog.Templates; +using Serilog.Templates.Themes; + +namespace Recyclarr.Cli.Logging; + +internal class ConsoleLogSinkConfigurator(LoggingLevelSwitch levelSwitch, IEnvironment env) : ILogConfigurator +{ + public void Configure(LoggerConfiguration config) + { + config.WriteTo.Console(BuildExpressionTemplate(), levelSwitch: levelSwitch); + } + + private ExpressionTemplate BuildExpressionTemplate() + { + var template = "[{@l:u3}] " + LogTemplates.Base; + + var raw = !string.IsNullOrEmpty(env.GetEnvironmentVariable("NO_COLOR")); + return new ExpressionTemplate(template, theme: raw ? null : TemplateTheme.Code); + } +} diff --git a/src/Recyclarr.Cli/Logging/FileLogSinkConfigurator.cs b/src/Recyclarr.Cli/Logging/FileLogSinkConfigurator.cs new file mode 100644 index 00000000..c741228f --- /dev/null +++ b/src/Recyclarr.Cli/Logging/FileLogSinkConfigurator.cs @@ -0,0 +1,40 @@ +using System.IO.Abstractions; +using Recyclarr.Logging; +using Recyclarr.Platform; +using Serilog.Events; +using Serilog.Templates; + +namespace Recyclarr.Cli.Logging; + +internal class FileLogSinkConfigurator(IAppPaths paths) : ILogConfigurator +{ + public void Configure(LoggerConfiguration config) + { + var logFilePrefix = $"recyclarr_{DateTime.Now:yyyy-MM-dd_HH-mm-ss}"; + var logDir = paths.LogDirectory; + var template = BuildExpressionTemplate(); + + config + .WriteTo.Logger(c => c + .MinimumLevel.Debug() + .WriteTo.File(template, LogFilePath("debug"))) + .WriteTo.Logger(c => c + .Filter.ByIncludingOnly(e => e.Level == LogEventLevel.Verbose) + .WriteTo.File(template, LogFilePath("verbose"))); + + return; + + string LogFilePath(string type) + { + return logDir.File($"{logFilePrefix}.{type}.log").FullName; + } + } + + private static ExpressionTemplate BuildExpressionTemplate() + { + var template = "[{@t:HH:mm:ss} {@l:u3}] " + LogTemplates.Base + + "{Inspect(@x).StackTrace}"; + + return new ExpressionTemplate(template); + } +} diff --git a/src/Recyclarr.Cli/Logging/LoggerFactory.cs b/src/Recyclarr.Cli/Logging/LoggerFactory.cs deleted file mode 100644 index c42e94f2..00000000 --- a/src/Recyclarr.Cli/Logging/LoggerFactory.cs +++ /dev/null @@ -1,72 +0,0 @@ -using System.IO.Abstractions; -using Recyclarr.Platform; -using Serilog.Core; -using Serilog.Events; -using Serilog.Templates; -using Serilog.Templates.Themes; - -namespace Recyclarr.Cli.Logging; - -public class LoggerFactory( - IAppPaths paths, - LoggingLevelSwitch levelSwitch, - IEnvironment env, - IEnumerable sinks) -{ - private static string GetBaseTemplateString() - { - var scope = LogProperty.Scope; - - return - $"{{#if {scope} is not null}}{{{scope}}}: {{#end}}" + - "{@m}"; - } - - private ExpressionTemplate GetConsoleTemplate() - { - var template = "[{@l:u3}] " + GetBaseTemplateString() + - "{#if SanitizedExceptionMessage is not null}: {SanitizedExceptionMessage}{#end}\n"; - - var raw = !string.IsNullOrEmpty(env.GetEnvironmentVariable("NO_COLOR")); - return new ExpressionTemplate(template, theme: raw ? null : TemplateTheme.Code); - } - - private static ExpressionTemplate GetFileTemplate() - { - var template = "[{@t:HH:mm:ss} {@l:u3}] " + GetBaseTemplateString() + - "{#if SanitizedExceptionMessage is not null}: {SanitizedExceptionMessage}{#end}\n" + - "{Inspect(@x).StackTrace}"; - - return new ExpressionTemplate(template); - } - - public ILogger Create() - { - var logFilePrefix = $"recyclarr_{DateTime.Now:yyyy-MM-dd_HH-mm-ss}"; - var logDir = paths.LogDirectory; - - var config = new LoggerConfiguration() - .MinimumLevel.Is(LogEventLevel.Verbose) - .Enrich.FromLogContext() - .Enrich.With() - .WriteTo.Console(GetConsoleTemplate(), levelSwitch: levelSwitch) - .WriteTo.Logger(c => c - .MinimumLevel.Debug() - .WriteTo.File(GetFileTemplate(), LogFilePath("debug"))) - .WriteTo.Logger(c => c - .Filter.ByIncludingOnly(e => e.Level == LogEventLevel.Verbose) - .WriteTo.File(GetFileTemplate(), LogFilePath("verbose"))); - - foreach (var sink in sinks) - { - config.WriteTo.Sink(sink, levelSwitch: levelSwitch); - } - - return config.CreateLogger(); - - string LogFilePath(string type) - { - return logDir.File($"{logFilePrefix}.{type}.log").FullName; - } - } -} diff --git a/src/Recyclarr.Cli/Recyclarr.Cli.csproj b/src/Recyclarr.Cli/Recyclarr.Cli.csproj index c6159c82..f92d3b83 100644 --- a/src/Recyclarr.Cli/Recyclarr.Cli.csproj +++ b/src/Recyclarr.Cli/Recyclarr.Cli.csproj @@ -30,6 +30,7 @@ + diff --git a/src/Recyclarr.Logging/ExceptionExtensions.cs b/src/Recyclarr.Logging/ExceptionExtensions.cs new file mode 100644 index 00000000..2e9d94ad --- /dev/null +++ b/src/Recyclarr.Logging/ExceptionExtensions.cs @@ -0,0 +1,23 @@ +using Recyclarr.Common.Extensions; + +namespace Recyclarr.Logging; + +public static class ExceptionExtensions +{ + public static string FullMessage(this Exception ex) + { + if (ex is AggregateException aex) + { + return aex.InnerExceptions.Aggregate("[ ", (total, next) => $"{total}[{next.FullMessage()}] ") + "]"; + } + + var msg = ex.Message.Replace(", see inner exception.", "").Trim(); + var innerMsg = ex.InnerException?.FullMessage(); + if (innerMsg != null && !innerMsg.ContainsIgnoreCase(msg) && !msg.ContainsIgnoreCase(innerMsg)) + { + msg = $"{msg} [ {innerMsg} ]"; + } + + return msg; + } +} diff --git a/src/Recyclarr.Cli/Logging/FlurlExceptionSanitizingEnricher.cs b/src/Recyclarr.Logging/FlurlExceptionSanitizingEnricher.cs similarity index 91% rename from src/Recyclarr.Cli/Logging/FlurlExceptionSanitizingEnricher.cs rename to src/Recyclarr.Logging/FlurlExceptionSanitizingEnricher.cs index 79966350..4e8cc44e 100644 --- a/src/Recyclarr.Cli/Logging/FlurlExceptionSanitizingEnricher.cs +++ b/src/Recyclarr.Logging/FlurlExceptionSanitizingEnricher.cs @@ -1,8 +1,7 @@ -using Recyclarr.Http; using Serilog.Core; using Serilog.Events; -namespace Recyclarr.Cli.Logging; +namespace Recyclarr.Logging; internal class FlurlExceptionSanitizingEnricher : ILogEventEnricher { diff --git a/src/Recyclarr.Logging/ILogConfigurator.cs b/src/Recyclarr.Logging/ILogConfigurator.cs new file mode 100644 index 00000000..703cef54 --- /dev/null +++ b/src/Recyclarr.Logging/ILogConfigurator.cs @@ -0,0 +1,8 @@ +using Serilog; + +namespace Recyclarr.Logging; + +public interface ILogConfigurator +{ + void Configure(LoggerConfiguration config); +} diff --git a/src/Recyclarr.Logging/LogProperty.cs b/src/Recyclarr.Logging/LogProperty.cs new file mode 100644 index 00000000..e0363e55 --- /dev/null +++ b/src/Recyclarr.Logging/LogProperty.cs @@ -0,0 +1,6 @@ +namespace Recyclarr.Logging; + +public static class LogProperty +{ + public static string Scope => nameof(Scope); +} diff --git a/src/Recyclarr.Logging/LogTemplates.cs b/src/Recyclarr.Logging/LogTemplates.cs new file mode 100644 index 00000000..057a9d21 --- /dev/null +++ b/src/Recyclarr.Logging/LogTemplates.cs @@ -0,0 +1,16 @@ +namespace Recyclarr.Logging; + +public static class LogTemplates +{ + public static string Base { get; } = GetBaseTemplateString(); + + private static string GetBaseTemplateString() + { + var scope = LogProperty.Scope; + + return + $"{{#if {scope} is not null}}{{{scope}}}: {{#end}}" + + "{@m}" + + "{#if SanitizedExceptionMessage is not null}: {SanitizedExceptionMessage}{#end}\n"; + } +} diff --git a/src/Recyclarr.Logging/LoggerFactory.cs b/src/Recyclarr.Logging/LoggerFactory.cs new file mode 100644 index 00000000..16f94fef --- /dev/null +++ b/src/Recyclarr.Logging/LoggerFactory.cs @@ -0,0 +1,22 @@ +using Serilog; +using Serilog.Events; + +namespace Recyclarr.Logging; + +public class LoggerFactory(IEnumerable configurators) +{ + public ILogger Create() + { + var config = new LoggerConfiguration() + .MinimumLevel.Is(LogEventLevel.Verbose) + .Enrich.FromLogContext() + .Enrich.With(); + + foreach (var configurator in configurators) + { + configurator.Configure(config); + } + + return config.CreateLogger(); + } +} diff --git a/src/Recyclarr.Logging/LoggingAutofacModule.cs b/src/Recyclarr.Logging/LoggingAutofacModule.cs new file mode 100644 index 00000000..45db2b4d --- /dev/null +++ b/src/Recyclarr.Logging/LoggingAutofacModule.cs @@ -0,0 +1,18 @@ +using Autofac; +using Serilog; +using Serilog.Core; +using Module = Autofac.Module; + +namespace Recyclarr.Logging; + +public class LoggingAutofacModule : Module +{ + protected override void Load(ContainerBuilder builder) + { + base.Load(builder); + + builder.RegisterType().SingleInstance(); + builder.RegisterType(); + builder.Register(c => c.Resolve().Create()).As().SingleInstance(); + } +} diff --git a/src/Recyclarr.Logging/Recyclarr.Logging.csproj b/src/Recyclarr.Logging/Recyclarr.Logging.csproj new file mode 100644 index 00000000..894b2a89 --- /dev/null +++ b/src/Recyclarr.Logging/Recyclarr.Logging.csproj @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/src/Recyclarr.Logging/Sanitize.cs b/src/Recyclarr.Logging/Sanitize.cs new file mode 100644 index 00000000..91a8d760 --- /dev/null +++ b/src/Recyclarr.Logging/Sanitize.cs @@ -0,0 +1,40 @@ +using System.Text.RegularExpressions; +using Flurl; + +namespace Recyclarr.Logging; + +public static partial class Sanitize +{ + public static string Message(string message) + { + // Replace full URLs + var result = UrlRegex().Replace(message, SanitizeMatch); + + // There are sometimes parenthetical parts of the message that contain the host but are not + // detected as true URLs. Just strip those out completely. + return HostRegex().Replace(result, ""); + } + + public static string ExceptionMessage(Exception exception) + { + return Message(exception.FullMessage()); + } + + public static Url Url(Url url) + { + // Replace hostname for user privacy + url.Host = "REDACTED"; + return url; + } + + private static string SanitizeMatch(Match match) + { + return Url(match.Value).ToString() ?? match.Value; + } + + [GeneratedRegex(@"\([-a-zA-Z0-9@:%._+~#=]{1,256}(?::[0-9]+)?\)")] + private static partial Regex HostRegex(); + + [GeneratedRegex(@"https?://(www\.)?[-a-zA-Z0-9@:%._+~#=]{1,256}(:[0-9]+)?\b([-a-zA-Z0-9()@:%_+.~#?&/=]*)")] + private static partial Regex UrlRegex(); +}