From 7d65c2b750433c637569b2ad7770fc7419eadf16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Tue, 12 Dec 2023 21:21:02 +0100 Subject: [PATCH] Fix logging leak (#357) * feat(ChannelRepository.cs): add managed node check to prevent errors when no managed node is found feat(JobTypes.cs): disallow concurrent execution of jobs to prevent potential conflicts fix(NodeChannelSubscribeJob.cs): change NodeUpdateManagement to async to ensure proper execution order refactor(ProcessNodeChannelAcceptorJob.cs): remove unnecessary loggerFactory creation chore(NodeGuard.csproj): update package versions for better compatibility and performance refactor(Program.cs): use Serilog for logging, adjust thread pool concurrency, and set up loggerFactory for Quartz feat(LightningClientService.cs): add logging for grpc channel creation for better debugging chore(NodeGuard.Tests.csproj): update Grpc.Core.Api version and add Microsoft.Extensions.Logging.Configuration for testing * refactor(Program.cs): remove QuartzInstrumentation from OpenTelemetry setup due to redundancy feat(LightningClientService.cs): add NullLogger for better logging refactor(LightningClientService.cs): change CreateLightningClient method from static to instance for better encapsulation and testability * refactor(SweepNodeWalletsJob.cs): inject ILightningClientService to reduce code duplication fix(SweepNodeWalletsJob.cs): replace direct instantiation of LightningClient with ILightningClientService.GetLightningClient to improve testability and maintainability style(SweepNodeWalletsJob.cs): remove unnecessary loggerFactory variable and whitespace for cleaner code --- src/Data/Repositories/ChannelRepository.cs | 15 +++++++++++ src/Jobs/NodeChannelSubscribeJob.cs | 2 +- src/Jobs/ProcessNodeChannelAcceptorJob.cs | 3 --- src/Jobs/SweepNodeWalletsJob.cs | 29 ++++++++------------- src/NodeGuard.csproj | 14 +++++----- src/Program.cs | 13 ++++----- src/Services/LightningClientService.cs | 9 +++++-- test/NodeGuard.Tests/NodeGuard.Tests.csproj | 3 ++- 8 files changed, 51 insertions(+), 37 deletions(-) diff --git a/src/Data/Repositories/ChannelRepository.cs b/src/Data/Repositories/ChannelRepository.cs index 76a56ce9..8b635acb 100644 --- a/src/Data/Repositories/ChannelRepository.cs +++ b/src/Data/Repositories/ChannelRepository.cs @@ -236,6 +236,21 @@ public async Task> GetAllManagedByUserNodes(string loggedUserId) } var channels = await _lightningClientService.ListChannels(channelOperationRequest.SourceNode); + Node managedNode; + + //If the source is not a managed node lets take the destination + if (channelOperationRequest.SourceNode.IsManaged) + managedNode = channelOperationRequest.SourceNode; + else + managedNode = channelOperationRequest.DestNode; + + if (managedNode == null) + { + _logger.LogError( + "Error while marking as closed with id: {ChannelId}. No managed node found", + channel.Id); + return (false, "No managed node found"); + } if (channels == null) { diff --git a/src/Jobs/NodeChannelSubscribeJob.cs b/src/Jobs/NodeChannelSubscribeJob.cs index 98b82ff4..2ff8c687 100644 --- a/src/Jobs/NodeChannelSubscribeJob.cs +++ b/src/Jobs/NodeChannelSubscribeJob.cs @@ -80,7 +80,7 @@ public async Task Execute(IJobExecutionContext context) { var channelEventUpdate = result.ResponseStream.Current; _logger.LogInformation("Channel event update received for node {@NodeId}", node.Id); - NodeUpdateManagement(channelEventUpdate, node); + await NodeUpdateManagement(channelEventUpdate, node); } catch (Exception e) { diff --git a/src/Jobs/ProcessNodeChannelAcceptorJob.cs b/src/Jobs/ProcessNodeChannelAcceptorJob.cs index 413bcb5c..4c7600cd 100644 --- a/src/Jobs/ProcessNodeChannelAcceptorJob.cs +++ b/src/Jobs/ProcessNodeChannelAcceptorJob.cs @@ -60,9 +60,6 @@ public async Task Execute(IJobExecutionContext context) var managedNodeId = context.JobDetail.JobDataMap.GetIntValueFromString("managedNodeId"); if (managedNodeId <= 0) throw new JobExecutionException(new Exception("Invalid managedNodeId"), false); - - var loggerFactory = GRPCLoggerFactoryHelper.LoggerFactory(); - #region Local Functions async Task AcceptChannelOpeningRequest( diff --git a/src/Jobs/SweepNodeWalletsJob.cs b/src/Jobs/SweepNodeWalletsJob.cs index 12f40c11..19d5f818 100644 --- a/src/Jobs/SweepNodeWalletsJob.cs +++ b/src/Jobs/SweepNodeWalletsJob.cs @@ -32,21 +32,24 @@ namespace NodeGuard.Jobs; [DisallowConcurrentExecution] public class SweepNodeWalletsJob : IJob { + private readonly ILightningClientService _lightningClientService; private readonly ILogger _logger; private readonly INodeRepository _nodeRepository; private readonly IWalletRepository _walletRepository; private readonly INBXplorerService _nbXplorerService; public SweepNodeWalletsJob(ILogger logger, - INodeRepository _nodeRepository, + INodeRepository nodeRepository, IWalletRepository walletRepository, - INBXplorerService nbXplorerService - ) + INBXplorerService nbXplorerService, + ILightningClientService lightningClientService) { + _logger = logger; - this._nodeRepository = _nodeRepository; + _nodeRepository = nodeRepository; _walletRepository = walletRepository; _nbXplorerService = nbXplorerService; + _lightningClientService = lightningClientService; } public async Task Execute(IJobExecutionContext context) @@ -150,23 +153,13 @@ async Task SweepFunds(Node node, Wallet wallet, Lightning.LightningClient lightn return; } - var loggerFactory = GRPCLoggerFactoryHelper.LoggerFactory(); + try { - using var grpcChannel = GrpcChannel.ForAddress($"https://{node.Endpoint}", - new GrpcChannelOptions - { - HttpHandler = new HttpClientHandler - { - ServerCertificateCustomValidationCallback = - HttpClientHandler.DangerousAcceptAnyServerCertificateValidator - }, - LoggerFactory = loggerFactory, - }); - - var client = new Lightning.LightningClient(grpcChannel); - + + var client = _lightningClientService.GetLightningClient(node.Endpoint); + var unspentResponse = await client.ListUnspentAsync(new ListUnspentRequest { MinConfs = 1, MaxConfs = Int32.MaxValue }, new Metadata { { diff --git a/src/NodeGuard.csproj b/src/NodeGuard.csproj index d3325ecd..32680fdf 100644 --- a/src/NodeGuard.csproj +++ b/src/NodeGuard.csproj @@ -23,7 +23,7 @@ - + @@ -33,6 +33,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive + @@ -48,14 +49,15 @@ - - - - - + + + + + + diff --git a/src/Program.cs b/src/Program.cs index fb9068ad..fb2b4dec 100644 --- a/src/Program.cs +++ b/src/Program.cs @@ -65,11 +65,10 @@ public static void Main(string[] args) .Enrich.With(new DatadogLogEnricher()) .WriteTo.Console(jsonFormatter) .CreateLogger(); + + builder.Host.UseSerilog(); - builder.Logging.ClearProviders(); - builder.Logging.AddSerilog(Log.Logger); - - // Add services to the container. + //Add services to the container. builder.Services.AddDatabaseDeveloperPageExceptionFilter(); @@ -194,7 +193,7 @@ public static void Main(string[] args) options.UseJsonSerializer(); }); - q.UseDedicatedThreadPool(x => { x.MaxConcurrency = 500; }); + q.UseDefaultThreadPool(x => { x.MaxConcurrency = 100; }); //This allows DI in jobs q.UseMicrosoftDependencyInjectionJobFactory(); @@ -294,7 +293,6 @@ public static void Main(string[] args) options.Endpoint = new Uri(Constants.OTEL_EXPORTER_ENDPOINT); }) .AddEntityFrameworkCoreInstrumentation() - .AddQuartzInstrumentation() ); } @@ -347,6 +345,9 @@ public static void Main(string[] args) //TODO Auth in the future, DAPR(?) app.MapGrpcService(); + var loggerFactory = new LoggerFactory() + .AddSerilog(Log.Logger); + Quartz.Logging.LogContext.SetCurrentLogProvider(loggerFactory); app.Run(); } } diff --git a/src/Services/LightningClientService.cs b/src/Services/LightningClientService.cs index fcfcad94..2d14626d 100644 --- a/src/Services/LightningClientService.cs +++ b/src/Services/LightningClientService.cs @@ -22,6 +22,7 @@ using Grpc.Core; using Grpc.Net.Client; using Lnrpc; +using Microsoft.Extensions.Logging.Abstractions; using NBitcoin; using NodeGuard.Data.Models; using NodeGuard.Helpers; @@ -53,7 +54,7 @@ public LightningClientService(ILogger logger) _logger = logger; } - private static GrpcChannel CreateLightningClient(string? endpoint) + private GrpcChannel CreateLightningClient(string? endpoint) { //Setup of grpc lnd api client (Lightning.proto) //Hack to allow self-signed https grpc calls @@ -64,7 +65,11 @@ private static GrpcChannel CreateLightningClient(string? endpoint) }; var grpcChannel = GrpcChannel.ForAddress($"https://{endpoint}", - new GrpcChannelOptions { HttpHandler = httpHandler }); + new GrpcChannelOptions + {HttpHandler = httpHandler, LoggerFactory = NullLoggerFactory.Instance}); + + + _logger.LogInformation("New grpc channel created for endpoint {endpoint}", endpoint); return grpcChannel; } diff --git a/test/NodeGuard.Tests/NodeGuard.Tests.csproj b/test/NodeGuard.Tests/NodeGuard.Tests.csproj index 1b95e251..823bae4c 100644 --- a/test/NodeGuard.Tests/NodeGuard.Tests.csproj +++ b/test/NodeGuard.Tests/NodeGuard.Tests.csproj @@ -10,9 +10,10 @@ - + +