From 14212861876235b6658fcd1b04b6041dd304a0dd Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Mon, 26 Oct 2026 11:09:28 +0000 Subject: [PATCH 1/2] add structured logging in --- Directory.Packages.props | 42 +++--- .../CapturingLogger.cs | 46 ++++--- .../DomainEventHandlerTests.cs | 3 +- .../DomainServiceHelperTests.cs | 6 +- .../FileProcessorDomainServiceTests.cs | 6 +- ...ileProfileDirectoryRecoveryServiceTests.cs | 10 +- .../Common/FileLineProcessingDiagnostics.cs | 127 ++++++++++++++++-- .../Services/FileProcessorDomainService.cs | 20 ++- .../FileProfileDirectoryRecoveryService.cs | 15 +-- .../FileProfileDirectorySynchronizer.cs | 4 +- .../FileProfileAggregate.cs | 2 +- .../Common/GenericSteps.cs | 5 +- FileProcessor/Common/Extensions.cs | 4 +- FileProcessor/Handlers/DomainEventHandlers.cs | 10 +- FileProcessor/Handlers/FileHandlers.cs | 6 +- FileProcessor/Program.cs | 5 +- .../FileProfileDirectoryRefreshService.cs | 4 +- FileProcessor/nlog.config | 32 ++--- 18 files changed, 219 insertions(+), 128 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 396b601..b598d65 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -4,8 +4,8 @@ - - + + @@ -13,18 +13,18 @@ - + - - - - - - - - - + + + + + + + + + @@ -39,17 +39,17 @@ - - - - - - - - + + + + + + + + - + diff --git a/FileProcessor.BusinessLogic.Tests/CapturingLogger.cs b/FileProcessor.BusinessLogic.Tests/CapturingLogger.cs index 4d6ac68..b227c45 100644 --- a/FileProcessor.BusinessLogic.Tests/CapturingLogger.cs +++ b/FileProcessor.BusinessLogic.Tests/CapturingLogger.cs @@ -1,32 +1,38 @@ using System; using System.Collections.Generic; -using Microsoft.Extensions.Logging; +using System.Linq; +using Shared.Logger; namespace FileProcessor.BusinessLogic.Tests; -internal sealed class CapturingLogger : ILogger +internal sealed class CapturingLogger : Shared.Logger.ILogger { - public List Messages { get; } = new(); - - public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + public bool IsInitialised { get; set; } = true; - public bool IsEnabled(LogLevel logLevel) => true; + public List Messages { get; } = new(); - public void Log(LogLevel logLevel, - EventId eventId, - TState state, - Exception exception, - Func formatter) - { - this.Messages.Add(formatter(state, exception)); - } + public void LogCritical(string message) => this.Capture(message); + public void LogCritical(Exception exception) => this.Capture(exception.ToString()); + public void LogCritical(string message, params object[] args) => this.Capture(message, args); + public void LogCritical(Exception exception, string message, params object[] args) => this.Capture(message, args, exception.ToString()); + public void LogCritical(string message, Exception exception) => this.Capture(message, null, exception.ToString()); + public void LogDebug(string message) => this.Capture(message); + public void LogDebug(string message, params object[] args) => this.Capture(message, args); + public void LogError(Exception exception) => this.Capture(exception.ToString()); + public void LogError(string message, params object[] args) => this.Capture(message, args); + public void LogError(Exception exception, string message, params object[] args) => this.Capture(message, args, exception.ToString()); + public void LogError(string message) => this.Capture(message); + public void LogError(string message, Exception exception) => this.Capture(message, null, exception.ToString()); + public void LogInformation(string message) => this.Capture(message); + public void LogInformation(string message, params object[] args) => this.Capture(message, args); + public void LogTrace(string message) => this.Capture(message); + public void LogTrace(string message, params object[] args) => this.Capture(message, args); + public void LogWarning(string message) => this.Capture(message); + public void LogWarning(string message, params object[] args) => this.Capture(message, args); - private sealed class NullScope : IDisposable + private void Capture(string message, object[] args = null, string exception = null) { - public static NullScope Instance { get; } = new(); - - public void Dispose() - { - } + String renderedArguments = args is { Length: > 0 } ? String.Join(" ", args) : String.Empty; + this.Messages.Add(String.Join(" ", new[] { message, renderedArguments, exception }.Where(value => !String.IsNullOrEmpty(value)))); } } diff --git a/FileProcessor.BusinessLogic.Tests/DomainEventHandlerTests.cs b/FileProcessor.BusinessLogic.Tests/DomainEventHandlerTests.cs index be674c6..f8ff21a 100644 --- a/FileProcessor.BusinessLogic.Tests/DomainEventHandlerTests.cs +++ b/FileProcessor.BusinessLogic.Tests/DomainEventHandlerTests.cs @@ -1,3 +1,4 @@ +using Shared.Logger; using System; using System.Collections.Generic; using System.Linq; @@ -23,7 +24,7 @@ public class DomainEventHandlerTests { public DomainEventHandlerTests() { - Shared.Logger.Logger.Initialise(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + Shared.Logger.Logger.Initialise(new NullLogger()); } [Fact] diff --git a/FileProcessor.BusinessLogic.Tests/DomainServiceHelperTests.cs b/FileProcessor.BusinessLogic.Tests/DomainServiceHelperTests.cs index 23c49bb..ce518ec 100644 --- a/FileProcessor.BusinessLogic.Tests/DomainServiceHelperTests.cs +++ b/FileProcessor.BusinessLogic.Tests/DomainServiceHelperTests.cs @@ -1,8 +1,6 @@ using System; -using FileProcessor.BusinessLogic.Services; -using Microsoft.Extensions.Logging.Abstractions; - using Shared.EventStore.Helpers; +using Shared.Logger; using Shouldly; using SimpleResults; using Xunit; @@ -12,7 +10,7 @@ namespace FileProcessor.BusinessLogic.Tests; public class DomainServiceHelperTests { public DomainServiceHelperTests() { - Shared.Logger.Logger.Initialise(NullLogger.Instance); + Shared.Logger.Logger.Initialise(new NullLogger()); } [Fact] diff --git a/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs b/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs index 4bde44b..5535ee5 100644 --- a/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs +++ b/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs @@ -51,13 +51,13 @@ public class FileProcessorDomainServiceTests private FileProcessorDomainService FileProcessorDomainService; private MockFileSystem FileSystem; - private CapturingLogger DiagnosticLogger; + private CapturingLogger DiagnosticLogger; public FileProcessorDomainServiceTests() { IConfigurationRoot configurationRoot = new ConfigurationBuilder().AddInMemoryCollection(TestData.DefaultAppSettings).Build(); ConfigurationReader.Initialise(configurationRoot); - Logger.Initialise(NullLogger.Instance); + //Logger.Initialise(NullLogger.Instance); this.FileProcessorManager = new IFileProcessorManagerImposter(); this.FileProcessorManager.GetFileProfileVersion(Arg.Any(), @@ -73,7 +73,7 @@ public FileProcessorDomainServiceTests() this.FileFormatHandler.ValidateFile(Arg.Any(), Arg.Any()).Returns( new FileValidationResult(Guid.Empty, true, Array.Empty())); this.FileSystem = new MockFileSystem(); - this.DiagnosticLogger = new CapturingLogger(); + this.DiagnosticLogger = new CapturingLogger(); Func fileFormatHandlerResolver = (format) => { diff --git a/FileProcessor.BusinessLogic.Tests/FileProfileDirectoryRecoveryServiceTests.cs b/FileProcessor.BusinessLogic.Tests/FileProfileDirectoryRecoveryServiceTests.cs index c4f0d48..780fea4 100644 --- a/FileProcessor.BusinessLogic.Tests/FileProfileDirectoryRecoveryServiceTests.cs +++ b/FileProcessor.BusinessLogic.Tests/FileProfileDirectoryRecoveryServiceTests.cs @@ -9,12 +9,12 @@ using FileProcessor.BusinessLogic.Services; using FileProcessor.Models; using MediatR; -using Microsoft.Extensions.Logging.Abstractions; using Imposter.Abstractions; using Shared.Results; using Shouldly; using SimpleResults; using FileProcessor.Testing; +using Shared.Logger; using FileProfileModel = global::FileProcessor.Models.FileProfile; using FileImportLogModel = global::FileProcessor.Models.FileImportLog; using Xunit; @@ -26,7 +26,7 @@ public class FileProfileDirectoryRecoveryServiceTests [Fact] public async Task RecoverInProgressFilesAsync_ReplaysFilesFoundInInProgressDirectory() { - Shared.Logger.Logger.Initialise(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + Shared.Logger.Logger.Initialise(new NullLogger()); IFileProcessorManagerImposter fileProcessorManager = new(); IMediatorImposter mediator = new(); @@ -72,7 +72,7 @@ public async Task RecoverInProgressFilesAsync_ReplaysFilesFoundInInProgressDirec [Fact] public async Task RecoverInProgressFilesAsync_WhenFileAggregateIsMissing_ReplaysUsingImportLogDetails() { - Shared.Logger.Logger.Initialise(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + Shared.Logger.Logger.Initialise(new NullLogger()); IFileProcessorManagerImposter fileProcessorManager = new(); IMediatorImposter mediator = new(); @@ -144,7 +144,7 @@ public async Task RecoverInProgressFilesAsync_WhenFileAggregateIsMissing_Replays [Fact] public async Task RecoverInProgressFilesAsync_ReplaysFilesFromHistoricalAndCurrentDirectories() { - Shared.Logger.Logger.Initialise(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + Shared.Logger.Logger.Initialise(new NullLogger()); IFileProcessorManagerImposter fileProcessorManager = new(); IMediatorImposter mediator = new(); @@ -198,7 +198,7 @@ public async Task RecoverInProgressFilesAsync_ReplaysFilesFromHistoricalAndCurre [Fact] public async Task RecoverInProgressFilesAsync_WhenVersionsShareDirectory_ReplaysFileOnce() { - Shared.Logger.Logger.Initialise(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + Shared.Logger.Logger.Initialise(new NullLogger()); IFileProcessorManagerImposter fileProcessorManager = new(); IMediatorImposter mediator = new(); diff --git a/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs b/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs index b350caa..e8fda84 100644 --- a/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs +++ b/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs @@ -62,27 +62,58 @@ internal static class FileLineProcessingDiagnostics { public static void LineProcessingStarted(FileLineProcessingContext context) { - Logger.LogInformation($"line-processing-started FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} FileProfileId={context.FileProfileId}"); + Logger.LogInformation( + "line-processing-started FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} FileProfileId={FileProfileId}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.FileProfileId); } public static void LineProcessingSkipped(FileLineProcessingContext context, ProcessingResult processingResult) { - Logger.LogInformation($"file-line-processing-skipped FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} ProcessingResult={processingResult}"); + Logger.LogInformation( + "file-line-processing-skipped FileId={FileId} LineNumber={LineNumber} ProcessingResult={ProcessingResult}", + context.Command.FileId, + context.Command.LineNumber, + processingResult); } public static void LineIgnored(FileLineProcessingContext context) { - Logger.LogInformation($"file-line-ignored FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} Operator={context.OperatorName} Reason=Ignored by file format handler"); + Logger.LogInformation( + "file-line-ignored FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} Operator={Operator} Reason={Reason}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.OperatorName, + "Ignored by file format handler"); } public static void LineRejected(FileLineProcessingContext context, String reason) { - Logger.LogWarning($"file-line-rejected FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} Operator={context.OperatorName} Reason={reason}"); + Logger.LogWarning( + "file-line-rejected FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} Operator={Operator} Reason={Reason}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.OperatorName, + reason); } public static void LineStateDetermined(FileLineProcessingContext context, ProcessingResult processingResult) { - Logger.LogInformation($"file-line-state-determined FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} ProcessingResult={processingResult} TransactionDispatchAttempted={context.TransactionDispatchAttempted} TransactionDispatchSucceeded={context.TransactionDispatchSucceeded} ResponseCode={context.ResponseCode}"); + Logger.LogInformation( + "file-line-state-determined FileId={FileId} LineNumber={LineNumber} ProcessingResult={ProcessingResult} TransactionDispatchAttempted={TransactionDispatchAttempted} TransactionDispatchSucceeded={TransactionDispatchSucceeded} ResponseCode={ResponseCode}", + context.Command.FileId, + context.Command.LineNumber, + processingResult, + context.TransactionDispatchAttempted, + context.TransactionDispatchSucceeded, + context.ResponseCode); } public static void ProcessingFailed(FileLineProcessingContext context, Result result) @@ -90,42 +121,112 @@ public static void ProcessingFailed(FileLineProcessingContext context, Result re String stage = context.Stage == FileLineProcessingStage.FileLineStatePersisted ? "file-line-state-persisted/failed" : GetStageName(context.Stage); - Logger.LogError($"{stage} failed FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} FileProfileId={context.FileProfileId} Operator={context.OperatorName} TransactionDispatchAttempted={context.TransactionDispatchAttempted} TransactionDispatchSucceeded={context.TransactionDispatchSucceeded} ResponseCode={context.ResponseCode} ResultStatus={result.Status} Error={result.Message}"); + Logger.LogError( + "{Stage} failed FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} FileProfileId={FileProfileId} Operator={Operator} TransactionDispatchAttempted={TransactionDispatchAttempted} TransactionDispatchSucceeded={TransactionDispatchSucceeded} ResponseCode={ResponseCode} ResultStatus={ResultStatus} Error={Error}", + stage, + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.FileProfileId, + context.OperatorName, + context.TransactionDispatchAttempted, + context.TransactionDispatchSucceeded, + context.ResponseCode, + result.Status, + result.Message); } public static void LineStatePersisted(FileLineProcessingContext context) { - Logger.LogInformation($"file-line-state-persisted FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} FileProfileId={context.FileProfileId} Operator={context.OperatorName} TransactionDispatchAttempted={context.TransactionDispatchAttempted} TransactionDispatchSucceeded={context.TransactionDispatchSucceeded} ResponseCode={context.ResponseCode}"); + Logger.LogInformation( + "file-line-state-persisted FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} FileProfileId={FileProfileId} Operator={Operator} TransactionDispatchAttempted={TransactionDispatchAttempted} TransactionDispatchSucceeded={TransactionDispatchSucceeded} ResponseCode={ResponseCode}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.FileProfileId, + context.OperatorName, + context.TransactionDispatchAttempted, + context.TransactionDispatchSucceeded, + context.ResponseCode); } public static void TransactionDispatchStarted(FileLineProcessingContext context) { - Logger.LogInformation($"transaction-dispatch-started FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} Operator={context.OperatorName} ClientOperation=PerformTransaction"); + Logger.LogInformation( + "transaction-dispatch-started FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} Operator={Operator} ClientOperation={ClientOperation}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.OperatorName, + "PerformTransaction"); } public static void TransactionDispatchFailed(FileLineProcessingContext context, Result result) { - Logger.LogError($"transaction-dispatch-failed FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} Operator={context.OperatorName} ClientOperation=PerformTransaction ResultStatus={result.Status} Error={result.Message}"); + Logger.LogError( + "transaction-dispatch-failed FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} Operator={Operator} ClientOperation={ClientOperation} ResultStatus={ResultStatus} Error={Error}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.OperatorName, + "PerformTransaction", + result.Status, + result.Message); } public static void TransactionDispatchCompleted(FileLineProcessingContext context, SaleTransactionResponse response) { - Logger.LogInformation($"transaction-dispatch-completed FileId={context.Command.FileId} LineNumber={context.Command.LineNumber} EstateId={context.EstateId} MerchantId={context.MerchantId} Operator={context.OperatorName} ClientOperation=PerformTransaction TransactionId={response.TransactionId} ResponseCode={response.ResponseCode} ResponseMessage={response.ResponseMessage}"); + Logger.LogInformation( + "transaction-dispatch-completed FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} Operator={Operator} ClientOperation={ClientOperation} TransactionId={TransactionId} ResponseCode={ResponseCode} ResponseMessage={ResponseMessage}", + context.Command.FileId, + context.Command.LineNumber, + context.EstateId, + context.MerchantId, + context.OperatorName, + "PerformTransaction", + response.TransactionId, + response.ResponseCode, + response.ResponseMessage); } public static void EventHandlerDispatchFailed(FileLineAddedEvent domainEvent, Result result) { - Logger.LogError($"event-handler-dispatch-failed FileId={domainEvent.FileId} LineNumber={domainEvent.LineNumber} EstateId={domainEvent.EstateId} MerchantId={domainEvent.MerchantId} EventType={nameof(FileLineAddedEvent)} ResultStatus={result.Status} Error={result.Message}"); + Logger.LogError( + "event-handler-dispatch-failed FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} EventType={EventType} ResultStatus={ResultStatus} Error={Error}", + domainEvent.FileId, + domainEvent.LineNumber, + domainEvent.EstateId, + domainEvent.MerchantId, + nameof(FileLineAddedEvent), + result.Status, + result.Message); } public static void EventHandlerDispatchCompleted(FileLineAddedEvent domainEvent) { - Logger.LogDebug($"event-handler-dispatch-completed FileId={domainEvent.FileId} LineNumber={domainEvent.LineNumber} EstateId={domainEvent.EstateId} MerchantId={domainEvent.MerchantId} EventType={nameof(FileLineAddedEvent)}"); + Logger.LogDebug( + "event-handler-dispatch-completed FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} EventType={EventType}", + domainEvent.FileId, + domainEvent.LineNumber, + domainEvent.EstateId, + domainEvent.MerchantId, + nameof(FileLineAddedEvent)); } public static void EventHandlerDispatchThrew(FileLineAddedEvent domainEvent, Exception exception) { - Logger.LogError($"event-handler-dispatch-threw FileId={domainEvent.FileId} LineNumber={domainEvent.LineNumber} EstateId={domainEvent.EstateId} MerchantId={domainEvent.MerchantId} EventType={nameof(FileLineAddedEvent)} Exception={exception}"); + Logger.LogError( + "event-handler-dispatch-threw FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} EventType={EventType}", + exception, + domainEvent.FileId, + domainEvent.LineNumber, + domainEvent.EstateId, + domainEvent.MerchantId, + nameof(FileLineAddedEvent)); } private static String GetStageName(FileLineProcessingStage stage) => stage switch diff --git a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs index f1b92ad..2f1f1b1 100644 --- a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs +++ b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs @@ -117,16 +117,15 @@ private async Task ApplyFileUpdates(Func> ac Result result = await action(fileAggregate); if (result.IsFailed) return ResultHelpers.CreateFailure(result); - Logger.LogWarning("About to save"); + Result saveResult = await this.FileAggregateRepository.SaveChanges(fileAggregate, cancellationToken); if (saveResult.IsFailed) return ResultHelpers.CreateFailure(saveResult); - Logger.LogWarning("About to return success after save"); + return Result.Success(); } catch (Exception ex) { - Logger.LogError(ex); return Result.Failure(ex.GetExceptionMessages()); } } @@ -359,7 +358,6 @@ private void DeleteDuplicateUploadFile(String filePath) public async Task ProcessUploadedFile(FileCommands.ProcessUploadedFileCommand command, CancellationToken cancellationToken) { Result result = await ApplyFileUpdates(async (FileAggregate fileAggregate) => { - Logger.LogWarning("In ProcessUploadedFile action"); Result operatorIdResult = await GetOperatorIdForFileProfile(command.FileId, command.EstateId, command.FileProfileId, @@ -368,13 +366,11 @@ public async Task ProcessUploadedFile(FileCommands.ProcessUploadedFileCo if (operatorIdResult.IsFailed) return ResultHelpers.CreateFailure(operatorIdResult); - Logger.LogWarning("About to Create File"); FileAggregateExtensions.CreateFileRequest request = new(command.FileImportLogId, command.EstateId, command.MerchantId, command.UserId, command.FileProfileId, command.FilePath, command.FileUploadedDateTime, operatorIdResult.Data, command.FileProfileVersionId); Result stateResult = fileAggregate.CreateFile(request); if (stateResult.IsFailed) return stateResult; - Logger.LogWarning("About to return success"); return Result.Success(); }, command.FileId, cancellationToken, false ); @@ -432,7 +428,7 @@ private async Task> GetOperatorIdForFileProfile(Guid fileId, cancellationToken); if (fileProfileResult.IsFailed){ - Logger.LogInformation($"file profile {fileProfileId} not found"); + Logger.LogInformation("file profile {fileProfileId} not found", fileProfileId); return ResultHelpers.CreateFailure(fileProfileResult); } FileProfileModel fileProfile = fileProfileResult.Data; @@ -503,7 +499,7 @@ public async Task ProcessTransactionForFileLine(FileCommands.ProcessTran if (result.IsFailed) { FileLineProcessingDiagnostics.ProcessingFailed(processingContext, result); - Logger.LogWarning($"{command.LineNumber} Status {result.Status} Message {result.Message}"); + Logger.LogWarning("Line Processing failed {LineNumber} Status {Status} Message {Message}", command.LineNumber, result.Status, result.Message); } else if (processingContext.Stage == FileLineProcessingStage.FileLineStatePersisted) { @@ -887,18 +883,18 @@ private async Task ProcessFile(Guid fileId, if (result.IsSuccess) { if (fileWasRejected) { - Logger.LogWarning($"About to move rejected file {inProgressFile.Name} to [{fileProfile.FailedDirectory}]"); + Logger.LogWarning("About to move rejected file {FileName} to [{Directory}]", inProgressFile.Name, fileProfile.FailedDirectory); inProgressFile.MoveTo($"{fileProfile.FailedDirectory}/{inProgressFile.Name}", true); return Result.Success(); } - Logger.LogInformation($"About to move file {inProgressFile.Name} to [{fileProfile.ProcessedDirectory}]"); + Logger.LogInformation("About to move file {{FileName}} to [{Directory}]", inProgressFile.Name, fileProfile.ProcessedDirectory); // Move file now inProgressFile.MoveTo($"{fileProfile.ProcessedDirectory}/{inProgressFile.Name}", true); } else { - Logger.LogWarning($"About to move file {inProgressFile.Name} to [{fileProfile.FailedDirectory}]. Reason(s) [{String.Join(",", result.Errors)}]"); + Logger.LogWarning("About to move file {{FileName}} to [{Directory}]. Reason(s) [{Reason}]", inProgressFile.Name, fileProfile.FailedDirectory, String.Join(",", result.Errors)); inProgressFile.MoveTo($"{fileProfile.FailedDirectory}/{inProgressFile.Name}", true); } @@ -933,7 +929,7 @@ private void EnsureDirectoryExists(String directory) if (this.FileSystem.Directory.Exists(directory)) return; - Logger.LogInformation($"Creating Directory {directory} as not found"); + Logger.LogInformation("Creating Directory {Directory} as not found", directory); this.FileSystem.Directory.CreateDirectory(directory); } diff --git a/FileProcessor.BusinessLogic/Services/FileProfileDirectoryRecoveryService.cs b/FileProcessor.BusinessLogic/Services/FileProfileDirectoryRecoveryService.cs index 0509071..fd2e132 100644 --- a/FileProcessor.BusinessLogic/Services/FileProfileDirectoryRecoveryService.cs +++ b/FileProcessor.BusinessLogic/Services/FileProfileDirectoryRecoveryService.cs @@ -8,6 +8,7 @@ using FileProcessor.BusinessLogic.Requests; using FileProcessor.Models; using MediatR; +using Shared.Logger; using Shared.Results; using SimpleResults; using FileProfileModel = global::FileProcessor.Models.FileProfile; @@ -73,9 +74,8 @@ private static StringComparer GetPathComparer() private async Task RecoverFileAsync(FileProfileModel fileProfile, string filePath, CancellationToken cancellationToken) { string fileName = Path.GetFileNameWithoutExtension(filePath); - if (TryParseIdentifiers(fileName, out Guid estateId, out Guid fileId) == false) - { - Shared.Logger.Logger.LogWarning($"Skipping leftover in-progress file [{filePath}] because the name does not contain recoverable identifiers"); + if (TryParseIdentifiers(fileName, out Guid estateId, out Guid fileId) == false) { + Shared.Logger.Logger.LogWarning("Skipping leftover in-progress file [{FileName}] because the name does not contain recoverable identifiers", filePath); return; } @@ -106,9 +106,8 @@ private async Task RecoverFileAsync(FileProfileModel fileProfile, string filePat FileImportLogModel importLog = importLogResult.Data; ImportLogFile importLogFile = importLog?.Files?.Find(file => file.FileId == fileId); - if (importLogResult.IsFailed || importLog == null || importLogFile == null) - { - Shared.Logger.Logger.LogWarning($"Skipping leftover in-progress file [{filePath}] because file [{fileId}] could not be loaded from the file aggregate or import log read model"); + if (importLogResult.IsFailed || importLog == null || importLogFile == null) { + Logger.LogWarning("Skipping leftover in-progress file [{FileName}] because file [{FileId}] could not be loaded from the file aggregate or import log read model", filePath, fileId); return; } @@ -127,11 +126,11 @@ private async Task RecoverFileAsync(FileProfileModel fileProfile, string filePat Result result = await this.Mediator.Send(command, cancellationToken); if (result.IsFailed) { - Shared.Logger.Logger.LogWarning($"Recovery processing failed for leftover in-progress file [{filePath}]: {result.Message}"); + Logger.LogWarning("Recovery processing failed for leftover in-progress file [{FileName}]: {Message}", filePath, result.Message); return; } - Shared.Logger.Logger.LogInformation($"Recovered leftover in-progress file [{filePath}] for file profile [{fileProfile.FileProfileId}]"); + Logger.LogInformation("Recovered leftover in-progress file [{FileName}] for file profile [{FileProfileId}]", filePath, fileProfile.FileProfileId); } private static bool TryParseIdentifiers(string fileName, out Guid estateId, out Guid fileId) diff --git a/FileProcessor.BusinessLogic/Services/FileProfileDirectorySynchronizer.cs b/FileProcessor.BusinessLogic/Services/FileProfileDirectorySynchronizer.cs index 0bc00bb..0f487d0 100644 --- a/FileProcessor.BusinessLogic/Services/FileProfileDirectorySynchronizer.cs +++ b/FileProcessor.BusinessLogic/Services/FileProfileDirectorySynchronizer.cs @@ -44,7 +44,7 @@ public async Task SyncAsync(CancellationToken cancellationToken) } this.FileSystem.Directory.CreateDirectory(temporaryFileLocation); - this.Logger.LogInformation("Ensured temporary file location exists at [{TemporaryFileLocation}]", temporaryFileLocation); + this.Logger.LogInformation("Ensured temporary file location exists at [{Directory}]", temporaryFileLocation); Result> fileProfilesResult = await this.FileProfileManager.GetAllFileProfileVersions(cancellationToken); if (fileProfilesResult.IsFailed) @@ -75,6 +75,6 @@ private static StringComparer GetPathComparer() private void EnsureDirectory(string directoryPath, string directoryName) { this.FileSystem.Directory.CreateDirectory(directoryPath); - this.Logger.LogInformation("Ensured {DirectoryName} directory exists at [{DirectoryPath}]", directoryName, directoryPath); + this.Logger.LogInformation("Ensured {Directory} directory exists at [{DirectoryPath}]", directoryName, directoryPath); } } diff --git a/FileProcessor.FileProfileAggregate/FileProfileAggregate.cs b/FileProcessor.FileProfileAggregate/FileProfileAggregate.cs index ab7f32e..3868d98 100644 --- a/FileProcessor.FileProfileAggregate/FileProfileAggregate.cs +++ b/FileProcessor.FileProfileAggregate/FileProfileAggregate.cs @@ -189,7 +189,7 @@ public static Result UpdateProfile(this FileProfileAggregate aggregate, Guid fil private static Result ResolveLineTerminator(FileProfileState profile, UpdateFileProfileRequest request) { if (request.LineTerminator.HasValue == false) - return Result.Success(profile.LineTerminator); + return Result.Success(profile.LineTerminator); Result result = request.LineTerminator.Value.ToLineTerminator(); return result.IsFailed ? ResultHelpers.CreateFailure(result) : result; diff --git a/FileProcessor.IntegrationTests/Common/GenericSteps.cs b/FileProcessor.IntegrationTests/Common/GenericSteps.cs index c2df755..5f1189e 100644 --- a/FileProcessor.IntegrationTests/Common/GenericSteps.cs +++ b/FileProcessor.IntegrationTests/Common/GenericSteps.cs @@ -32,9 +32,10 @@ public async Task StartSystem() { // Initialise a logger String scenarioName = this.ScenarioContext.ScenarioInfo.Title.Replace(" ", ""); - NlogLogger logger = new NlogLogger(); + NlogLogger logger = new(); logger.Initialise(LogManager.GetLogger(scenarioName), scenarioName); - LogManager.AddHiddenAssembly(typeof(NlogLogger).Assembly); + LogManager.Setup().SetupLogFactory(setup => + setup.AddCallSiteHiddenAssembly(typeof(NlogLogger).Assembly)); DockerServices dockerServices = this.GetDockerServicesForScenario(); diff --git a/FileProcessor/Common/Extensions.cs b/FileProcessor/Common/Extensions.cs index c59fe2a..2064f64 100644 --- a/FileProcessor/Common/Extensions.cs +++ b/FileProcessor/Common/Extensions.cs @@ -46,10 +46,10 @@ public static IServiceCollection AddInSecureEventStoreClient( switch (tt) { case TraceEventType.Critical: - Logger.LogCritical(new Exception(logMessage)); + Logger.LogCritical(logMessage); break; case TraceEventType.Error: - Logger.LogError(new Exception(logMessage)); + Logger.LogError(logMessage); break; case TraceEventType.Warning: Logger.LogWarning(logMessage); diff --git a/FileProcessor/Handlers/DomainEventHandlers.cs b/FileProcessor/Handlers/DomainEventHandlers.cs index 5273441..3a77df7 100644 --- a/FileProcessor/Handlers/DomainEventHandlers.cs +++ b/FileProcessor/Handlers/DomainEventHandlers.cs @@ -29,13 +29,13 @@ public static async Task HandleDomainEvent(HttpRequest request, try { - Logger.LogInformation($"Processing event - ID [{domainEvent.EventId}], Type[{domainEvent.GetType().Name}]"); + Logger.LogInformation("Processing event - ID [{eventId}], Type[{eventType}]", domainEvent.EventId, domainEvent.GetType().Name); Result> eventHandlersResult = resolver.GetDomainEventHandlers(domainEvent); if (eventHandlersResult.IsFailed) { - Logger.LogWarning($"No event handlers configured for Event Type [{domainEvent.GetType().Name}]"); + Logger.LogWarning("No event handlers configured for Event Type [{eventType}]", domainEvent.GetType().Name); return Results.Ok(); } @@ -49,7 +49,7 @@ public static async Task HandleDomainEvent(HttpRequest request, } catch (Exception ex) { - Logger.LogError(new Exception($"One or more handlers threw while processing event [{domainEvent.EventId}]", ex)); + Logger.LogError(ex, "One or more handlers threw while processing event [{eventId}]", domainEvent.EventId); } List failures = executions.SelectMany(GetFailures).ToList(); @@ -72,7 +72,7 @@ public static async Task HandleDomainEvent(HttpRequest request, catch (Exception ex) { string domainEventData = StringSerialiser.Serialise(domainEvent); - Logger.LogError($"Failed to process event. Data received [{domainEventData}]", ex); + Logger.LogError(ex, $"Failed to process event. Data received [{domainEventData}]"); throw; } } @@ -81,7 +81,7 @@ private static void Callback(CancellationToken cancellationToken, Guid eventId) { if (cancellationToken.IsCancellationRequested) { - Logger.LogInformation($"Cancel request for EventId {eventId}"); + Logger.LogInformation("Cancel request for EventId {eventId}", eventId); cancellationToken.ThrowIfCancellationRequested(); } } diff --git a/FileProcessor/Handlers/FileHandlers.cs b/FileProcessor/Handlers/FileHandlers.cs index feb25a0..1d995a8 100644 --- a/FileProcessor/Handlers/FileHandlers.cs +++ b/FileProcessor/Handlers/FileHandlers.cs @@ -59,11 +59,7 @@ public static async Task UploadFile(IMediator mediator, Result result = await mediator.Send(command, cancellationToken); uploadAccepted = result.IsSuccess; - - Shared.Logger.Logger.LogDebug($"Day is {request.UploadDateTime.Day}"); - Shared.Logger.Logger.LogDebug($"Month is {request.UploadDateTime.Month}"); - Shared.Logger.Logger.LogDebug($"Year is {request.UploadDateTime.Year}"); - + if (result.IsSuccess) return Results.Accepted(value: request.FileId); diff --git a/FileProcessor/Program.cs b/FileProcessor/Program.cs index a24c064..8ab2b0a 100644 --- a/FileProcessor/Program.cs +++ b/FileProcessor/Program.cs @@ -147,8 +147,9 @@ private static void ConfigurePipeline(WebApplication app) app.UseDeveloperExceptionPage(); } - ILogger logger = app.Services.GetRequiredService().CreateLogger("FileProcessor"); - Logger.Initialise(logger); + LogManager.Setup().LoadConfigurationFromFile("NLog.config"); + NLog.Logger logger = LogManager.GetLogger("FileProcessor"); + Shared.Logger.Logger.Initialise(logger, "FileProcessor"); IConfigurationRoot configuration = (IConfigurationRoot)app.Configuration; configuration.LogConfiguration(Logger.LogWarning); ConfigurationReader.Initialise(configuration); diff --git a/FileProcessor/Services/FileProfileDirectoryRefreshService.cs b/FileProcessor/Services/FileProfileDirectoryRefreshService.cs index f42d3cf..827a5d3 100644 --- a/FileProcessor/Services/FileProfileDirectoryRefreshService.cs +++ b/FileProcessor/Services/FileProfileDirectoryRefreshService.cs @@ -36,7 +36,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) pollingWindowInSeconds = Math.Max(1, pollingWindowInSeconds); TimeSpan pollingWindow = TimeSpan.FromSeconds(pollingWindowInSeconds); - this.Logger.LogInformation("File profile directory refresh started with a {PollingWindow} second interval", pollingWindowInSeconds); + this.Logger.LogInformation($"File profile directory refresh started with a {pollingWindowInSeconds} second interval"); using PeriodicTimer timer = new PeriodicTimer(pollingWindow); @@ -59,7 +59,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) Result result = await this.DirectorySynchronizer.SyncAsync(stoppingToken); if (result.IsFailed) { - this.Logger.LogWarning("File profile directory refresh failed: {Message}", result.Message); + this.Logger.LogWarning($"File profile directory refresh failed: {result.Message}"); } } catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) diff --git a/FileProcessor/nlog.config b/FileProcessor/nlog.config index adb458a..6cd2101 100644 --- a/FileProcessor/nlog.config +++ b/FileProcessor/nlog.config @@ -3,21 +3,21 @@ - + maxArchiveFiles="2"> + + + + + + + @@ -33,14 +33,6 @@ - - - - - - - - From d27f84b25554bbf72d8c8f27d49b42bd26c176de Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Mon, 26 Oct 2026 11:15:08 +0000 Subject: [PATCH 2/2] update from review comments --- .../Common/FileLineProcessingDiagnostics.cs | 3 +-- .../Services/FileProcessorDomainService.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs b/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs index e8fda84..c5a6467 100644 --- a/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs +++ b/FileProcessor.BusinessLogic/Common/FileLineProcessingDiagnostics.cs @@ -219,9 +219,8 @@ public static void EventHandlerDispatchCompleted(FileLineAddedEvent domainEvent) public static void EventHandlerDispatchThrew(FileLineAddedEvent domainEvent, Exception exception) { - Logger.LogError( + Logger.LogError(exception, "event-handler-dispatch-threw FileId={FileId} LineNumber={LineNumber} EstateId={EstateId} MerchantId={MerchantId} EventType={EventType}", - exception, domainEvent.FileId, domainEvent.LineNumber, domainEvent.EstateId, diff --git a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs index 2f1f1b1..95bbf6c 100644 --- a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs +++ b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs @@ -894,7 +894,7 @@ private async Task ProcessFile(Guid fileId, inProgressFile.MoveTo($"{fileProfile.ProcessedDirectory}/{inProgressFile.Name}", true); } else { - Logger.LogWarning("About to move file {{FileName}} to [{Directory}]. Reason(s) [{Reason}]", inProgressFile.Name, fileProfile.FailedDirectory, String.Join(",", result.Errors)); + Logger.LogWarning("About to move file {FileName} to [{Directory}]. Reason(s) [{Reason}]", inProgressFile.Name, fileProfile.FailedDirectory, String.Join(",", result.Errors)); inProgressFile.MoveTo($"{fileProfile.FailedDirectory}/{inProgressFile.Name}", true); }