Logging fixes and improvements (#4019)

* Sort usings

* Prevent GetLogger from being called before NLog init

* Align Monitoring default log level with other instances

* Simplify and unify logging configuration

* Pass LoggingSettings to CommandRunner

* Align more logging settings

* Expose LoggingSettings through Settings

src/ServiceControl.AcceptanceTests.RavenDB/StartupModeTests.cs

@@ -6,7 +6,6 @@
     using Microsoft.Extensions.Hosting;
     using NServiceBus;
     using NUnit.Framework;
-    using Particular.ServiceControl;
     using Particular.ServiceControl.Hosting;
     using Persistence;
     using Persistence.RavenDB;
@@ -63,7 +62,7 @@ protected override EndpointConfiguration CreateEndpointConfiguration(Settings se
             {
                 var configuration = base.CreateEndpointConfiguration(settings);
 
-                //HINT: we want to exclude this assembly to prevent loading features that are part of the acceptance testing framework  
+                //HINT: we want to exclude this assembly to prevent loading features that are part of the acceptance testing framework
                 var thisAssembly = new[] { typeof(StartupModeTests).Assembly.GetName().Name };
 
                 configuration.AssemblyScanner().ExcludeAssemblies(thisAssembly);

src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs

@@ -45,7 +45,11 @@ public ServiceControlComponentRunner(ITransportIntegration transportToUse, Accep
 
         async Task InitializeServiceControl(ScenarioContext context)
         {
-            var settings = new Settings(instanceName, transportToUse.TypeName, persistenceToUse.PersistenceType, forwardErrorMessages: false, errorRetentionPeriod: TimeSpan.FromDays(10))
+            var logPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+            Directory.CreateDirectory(logPath);
+            var loggingSettings = new LoggingSettings(defaultLevel: LogLevel.Debug, logPath: logPath);
+
+            var settings = new Settings(instanceName, transportToUse.TypeName, persistenceToUse.PersistenceType, loggingSettings, forwardErrorMessages: false, errorRetentionPeriod: TimeSpan.FromDays(10))
             {
                 AllowMessageEditing = true,
                 ForwardErrorMessages = false,
@@ -95,7 +99,7 @@ async Task InitializeServiceControl(ScenarioContext context)
             using (new DiagnosticTimer($"Creating infrastructure for {instanceName}"))
             {
                 var setupCommand = new SetupCommand();
-                await setupCommand.Execute(new HostArguments(Array.Empty<string>()), settings);
+                await setupCommand.Execute(new HostArguments([]), settings);
             }
 
             var configuration = new EndpointConfiguration(instanceName);
@@ -105,16 +109,12 @@ async Task InitializeServiceControl(ScenarioContext context)
 
             using (new DiagnosticTimer($"Starting ServiceControl {instanceName}"))
             {
-                var logPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
-                Directory.CreateDirectory(logPath);
-
-                var loggingSettings = new LoggingSettings(settings.ServiceName, defaultLevel: LogLevel.Debug, logPath: logPath);
                 var hostBuilder = WebApplication.CreateBuilder(new WebApplicationOptions
                 {
                     // Force the DI container to run the dependency resolution check to verify all dependencies can be resolved
                     EnvironmentName = Environments.Development
                 });
-                hostBuilder.AddServiceControl(settings, configuration, loggingSettings);
+                hostBuilder.AddServiceControl(settings, configuration);
                 hostBuilder.AddServiceControlApi();
 
                 hostBuilder.AddServiceControlTesting(settings);

src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs

@@ -38,7 +38,12 @@ public class ServiceControlComponentRunner(
 
         async Task InitializeServiceControl(ScenarioContext context)
         {
-            settings = new Settings(instanceName, transportToUse.TypeName, persistenceToUse.PersistenceType)
+            var logPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+            Directory.CreateDirectory(logPath);
+
+            var loggingSettings = new LoggingSettings(defaultLevel: NLog.LogLevel.Debug, logPath: logPath);
+
+            settings = new Settings(instanceName, transportToUse.TypeName, persistenceToUse.PersistenceType, loggingSettings)
             {
                 TransportConnectionString = transportToUse.ConnectionString,
                 MaximumConcurrencyLevel = 2,
@@ -91,7 +96,7 @@ async Task InitializeServiceControl(ScenarioContext context)
             using (new DiagnosticTimer($"Creating infrastructure for {instanceName}"))
             {
                 var setupCommand = new SetupCommand();
-                await setupCommand.Execute(new HostArguments(Array.Empty<string>()), settings);
+                await setupCommand.Execute(new HostArguments([]), settings);
             }
 
             var configuration = new EndpointConfiguration(instanceName);
@@ -101,10 +106,6 @@ async Task InitializeServiceControl(ScenarioContext context)
 
             using (new DiagnosticTimer($"Starting ServiceControl {instanceName}"))
             {
-                var logPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
-                Directory.CreateDirectory(logPath);
-
-                var loggingSettings = new LoggingSettings(settings.ServiceName, defaultLevel: NLog.LogLevel.Debug, logPath: logPath);
                 var hostBuilder = WebApplication.CreateBuilder(new WebApplicationOptions
                 {
                     // Force the DI container to run the dependency resolution check to verify all dependencies can be resolved
@@ -121,7 +122,7 @@ async Task InitializeServiceControl(ScenarioContext context)
                     };
                     context.Logs.Enqueue(logitem);
                     return criticalErrorContext.Stop(cancellationToken);
-                }, settings, configuration, loggingSettings);
+                }, settings, configuration);
 
                 hostBuilder.AddServiceControlAuditApi();
 

src/ServiceControl.Audit.UnitTests/API/APIApprovals.cs

@@ -30,10 +30,7 @@ public void RootPathValue()
 
             var settings = CreateTestSettings();
 
-            var controller = new RootController(
-                new LoggingSettings("testEndpoint"),
-                settings
-            )
+            var controller = new RootController(settings)
             {
                 ControllerContext = controllerContext,
                 Url = new UrlHelper(actionContext)

src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt

@@ -1,4 +1,11 @@
 {
+  "LoggingSettings": {
+    "LogLevel": {
+      "Name": "Info",
+      "Ordinal": 2
+    },
+    "LogPath": "C:\\Logs"
+  },
   "MessageFilter": null,
   "ValidateConfiguration": true,
   "SkipQueueCreation": false,

src/ServiceControl.Audit.UnitTests/Infrastructure/When_instance_is_setup.cs

@@ -4,7 +4,6 @@
     using System.Collections.Generic;
     using System.Threading;
     using System.Threading.Tasks;
-    using Audit.Infrastructure;
     using Audit.Infrastructure.Hosting;
     using Audit.Infrastructure.Hosting.Commands;
     using Audit.Infrastructure.Settings;
@@ -28,7 +27,7 @@ public async Task Should_provision_queues()
             };
 
             var setupCommand = new SetupCommand();
-            await setupCommand.Execute(new HostArguments(Array.Empty<string>()), settings);
+            await setupCommand.Execute(new HostArguments([]), settings);
 
             CollectionAssert.AreEquivalent(new[]
             {

src/ServiceControl.Audit/Auditing/AuditIngestion.cs

@@ -27,7 +27,6 @@ class AuditIngestion : IHostedService
             TransportSettings transportSettings,
             Metrics metrics,
             IFailedAuditStorage failedImportsStorage,
-            LoggingSettings loggingSettings,
             AuditIngestionCustomCheck.State ingestionState,
             AuditIngestor auditIngestor,
             IAuditIngestionUnitOfWorkFactory unitOfWorkFactory,
@@ -53,7 +52,7 @@ class AuditIngestion : IHostedService
                 FullMode = BoundedChannelFullMode.Wait
             });
 
-            errorHandlingPolicy = new AuditIngestionFaultPolicy(failedImportsStorage, loggingSettings, OnCriticalError);
+            errorHandlingPolicy = new AuditIngestionFaultPolicy(failedImportsStorage, settings.LoggingSettings, OnCriticalError);
 
             watchdog = new Watchdog("audit message ingestion", EnsureStarted, EnsureStopped, ingestionState.ReportError, ingestionState.Clear, settings.TimeToRestartAuditIngestionAfterFailure, logger);
 

src/ServiceControl.Audit/HostApplicationBuilderExtensions.cs

@@ -2,7 +2,6 @@ namespace ServiceControl.Audit;
 
 using System;
 using System.Diagnostics;
-using System.Net;
 using System.Threading;
 using System.Threading.Tasks;
 using Auditing;
@@ -27,14 +26,13 @@ static class HostApplicationBuilderExtensions
     public static void AddServiceControlAudit(this IHostApplicationBuilder builder,
         Func<ICriticalErrorContext, CancellationToken, Task> onCriticalError,
         Settings settings,
-        EndpointConfiguration configuration,
-        LoggingSettings loggingSettings)
+        EndpointConfiguration configuration)
     {
         var persistenceConfiguration =
             PersistenceConfigurationFactory.LoadPersistenceConfiguration(settings.PersistenceType);
         var persistenceSettings = persistenceConfiguration.BuildPersistenceSettings(settings);
 
-        RecordStartup(settings, loggingSettings, configuration, persistenceConfiguration);
+        RecordStartup(settings, configuration, persistenceConfiguration);
 
         if (!string.IsNullOrWhiteSpace(settings.LicenseFileText))
         {
@@ -46,15 +44,14 @@ static class HostApplicationBuilderExtensions
 
         builder.Logging.ClearProviders();
         builder.Logging.AddNLog();
-        builder.Logging.SetMinimumLevel(loggingSettings.ToHostLogLevel());
+        builder.Logging.SetMinimumLevel(settings.LoggingSettings.ToHostLogLevel());
 
         var services = builder.Services;
 
         services.Configure<HostOptions>(options => options.ShutdownTimeout = TimeSpan.FromSeconds(30));
         services.AddSingleton(transportSettings);
         services.AddSingleton(transportCustomization);
 
-        services.AddSingleton(loggingSettings);
         services.AddSingleton(settings);
         services.AddSingleton<EndpointInstanceMonitoring>();
         services.AddSingleton<AuditIngestor>();
@@ -76,7 +73,7 @@ static class HostApplicationBuilderExtensions
 
         services.AddPersistence(persistenceSettings, persistenceConfiguration);
 
-        NServiceBusFactory.Configure(settings, transportCustomization, transportSettings, loggingSettings, onCriticalError, configuration);
+        NServiceBusFactory.Configure(settings, transportCustomization, transportSettings, onCriticalError, configuration);
         builder.UseNServiceBus(configuration);
 
         // Configure after the NServiceBus hosted service to ensure NServiceBus is already started
@@ -99,7 +96,7 @@ static TransportSettings MapSettings(Settings settings)
         return transportSettings;
     }
 
-    static void RecordStartup(Settings settings, LoggingSettings loggingSettings, EndpointConfiguration endpointConfiguration, IPersistenceConfiguration persistenceConfiguration)
+    static void RecordStartup(Settings settings, EndpointConfiguration endpointConfiguration, IPersistenceConfiguration persistenceConfiguration)
     {
         var version = FileVersionInfo.GetVersionInfo(typeof(HostApplicationBuilderExtensions).Assembly.Location).ProductVersion;
 
@@ -108,7 +105,7 @@ static void RecordStartup(Settings settings, LoggingSettings loggingSettings, En
 ServiceControl Audit Version:       {version}
 Audit Retention Period:             {settings.AuditRetentionPeriod}
 Forwarding Audit Messages:          {settings.ForwardAuditMessages}
-ServiceControl Logging Level:       {loggingSettings.LoggingLevel}
+ServiceControl Logging Level:       {settings.LoggingSettings.LogLevel}
 Transport Customization:            {settings.TransportType},
 Persistence Customization:          {settings.PersistenceType},
 Persistence:                        {persistenceConfiguration.Name}
@@ -118,8 +115,7 @@ static void RecordStartup(Settings settings, LoggingSettings loggingSettings, En
         logger.Info(startupMessage);
         endpointConfiguration.GetSettings().AddStartupDiagnosticsSection("Startup", new
         {
-            Settings = settings,
-            LoggingSettings = loggingSettings
+            Settings = settings
         });
     }
 }

src/ServiceControl.Audit/Infrastructure/Hosting/Commands/AbstractCommand.cs

@@ -1,9 +1,10 @@
 namespace ServiceControl.Audit.Infrastructure.Hosting.Commands
 {
     using System.Threading.Tasks;
+    using ServiceControl.Audit.Infrastructure.Settings;
 
     abstract class AbstractCommand
     {
-        public abstract Task Execute(HostArguments args, Settings.Settings settings);
+        public abstract Task Execute(HostArguments args, Settings settings);
     }
 }

src/ServiceControl.Audit/Infrastructure/Hosting/Commands/CommandRunner.cs

@@ -3,10 +3,11 @@
     using System;
     using System.Collections.Generic;
     using System.Threading.Tasks;
+    using ServiceControl.Audit.Infrastructure.Settings;
 
     class CommandRunner(List<Type> commands)
     {
-        public async Task Execute(HostArguments args, Settings.Settings settings)
+        public async Task Execute(HostArguments args, Settings settings)
         {
             foreach (var commandType in commands)
             {

src/ServiceControl.Audit/Infrastructure/Hosting/Commands/ImportFailedAuditsCommand.cs

@@ -6,7 +6,6 @@
     using Auditing;
     using Microsoft.Extensions.DependencyInjection;
     using Microsoft.Extensions.Hosting;
-    using NLog;
     using NServiceBus;
     using Settings;
 
@@ -17,7 +16,6 @@ public override async Task Execute(HostArguments args, Settings settings)
             settings.IngestAuditMessages = false;
 
             var endpointConfiguration = new EndpointConfiguration(settings.ServiceName);
-            var loggingSettings = new LoggingSettings(settings.ServiceName, LogLevel.Info);
 
             using var tokenSource = new CancellationTokenSource();
 
@@ -26,7 +24,7 @@ public override async Task Execute(HostArguments args, Settings settings)
             {
                 tokenSource.Cancel();
                 return Task.CompletedTask;
-            }, settings, endpointConfiguration, loggingSettings);
+            }, settings, endpointConfiguration);
 
             using var app = hostBuilder.Build();
             await app.StartAsync(tokenSource.Token);

src/ServiceControl.Audit/Infrastructure/Hosting/Commands/MaintenanceModeCommand.cs

@@ -5,10 +5,11 @@
     using Microsoft.Extensions.Hosting;
     using Microsoft.Extensions.Hosting.WindowsServices;
     using Persistence;
+    using ServiceControl.Audit.Infrastructure.Settings;
 
     class MaintenanceModeCommand : AbstractCommand
     {
-        public override async Task Execute(HostArguments args, Settings.Settings settings)
+        public override async Task Execute(HostArguments args, Settings settings)
         {
             var persistenceConfiguration = PersistenceConfigurationFactory.LoadPersistenceConfiguration(settings.PersistenceType);
             var persistenceSettings = persistenceConfiguration.BuildPersistenceSettings(settings);

src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs

@@ -14,14 +14,12 @@ public override async Task Execute(HostArguments args, Settings settings)
             var assemblyScanner = endpointConfiguration.AssemblyScanner();
             assemblyScanner.ExcludeAssemblies("ServiceControl.Plugin");
 
-            var loggingSettings = new LoggingSettings(settings.ServiceName);
-
             var hostBuilder = WebApplication.CreateBuilder();
             hostBuilder.AddServiceControlAudit((_, __) =>
             {
                 //Do nothing. The transports in NSB 8 are designed to handle broker outages. Audit ingestion will be paused when broker is unavailable.
                 return Task.CompletedTask;
-            }, settings, endpointConfiguration, loggingSettings);
+            }, settings, endpointConfiguration);
             hostBuilder.AddServiceControlAuditApi();
 
             var app = hostBuilder.Build();