Skip to content

Commit

Permalink
Fix logging leak (#357)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@Jossec101
Jossec101 authored Dec 12, 2023
1 parent bd6d840 commit 7d65c2b
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 37 deletions.
15 changes: 15 additions & 0 deletions src/Data/Repositories/ChannelRepository.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,21 @@ public async Task<List<Channel>> 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)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Jobs/NodeChannelSubscribeJob.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
3 changes: 0 additions & 3 deletions src/Jobs/ProcessNodeChannelAcceptorJob.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
29 changes: 11 additions & 18 deletions src/Jobs/SweepNodeWalletsJob.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,24 @@ namespace NodeGuard.Jobs;
[DisallowConcurrentExecution]
public class SweepNodeWalletsJob : IJob
{
private readonly ILightningClientService _lightningClientService;
private readonly ILogger<SweepNodeWalletsJob> _logger;
private readonly INodeRepository _nodeRepository;
private readonly IWalletRepository _walletRepository;
private readonly INBXplorerService _nbXplorerService;

public SweepNodeWalletsJob(ILogger<SweepNodeWalletsJob> 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)
Expand Down Expand Up @@ -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
{
{
Expand Down
14 changes: 8 additions & 6 deletions src/NodeGuard.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<PackageReference Include="Blazorise.Bootstrap" Version="1.2.4" />
<PackageReference Include="Blazorise.Components" Version="1.2.4" />
<PackageReference Include="Blazorise.Icons.FontAwesome" Version="1.2.4" />
<PackageReference Include="Grpc.AspNetCore" Version="2.55.0" />
<PackageReference Include="Grpc.AspNetCore" Version="2.59.0" />
<PackageReference Include="Humanizer.Core" Version="2.14.1" />
<PackageReference Include="Microsoft.AspNetCore.DataProtection.EntityFrameworkCore" Version="7.0.14" />
<PackageReference Include="Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore" Version="7.0.9" />
Expand All @@ -33,6 +33,7 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Extensions.Logging.Configuration" Version="7.0.0" />
<PackageReference Include="Microsoft.VisualStudio.Web.CodeGeneration.Design" Version="7.0.8" />
<PackageReference Include="Moq.EntityFrameworkCore" Version="7.0.0.2" />
<PackageReference Include="NBitcoin" Version="7.0.30" />
Expand All @@ -48,14 +49,15 @@
<PackageReference Include="OpenTelemetry.Extensions.Hosting" Version="1.5.1" />
<PackageReference Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.5.1-beta.1" />
<PackageReference Include="OpenTelemetry.Instrumentation.EntityFrameworkCore" Version="1.0.0-beta.7" />
<PackageReference Include="OpenTelemetry.Instrumentation.Quartz" Version="1.0.0-alpha.3" />
<PackageReference Include="OpenTelemetry.Instrumentation.Runtime" Version="1.5.0" />
<PackageReference Include="Quartz" Version="3.6.3" />
<PackageReference Include="Quartz.AspNetCore" Version="3.6.3" />
<PackageReference Include="Quartz.Serialization.Json" Version="3.6.3" />
<PackageReference Include="OpenTelemetry.Instrumentation.Runtime" Version="1.5.1" />
<PackageReference Include="Quartz" Version="3.8.0" />
<PackageReference Include="Quartz.AspNetCore" Version="3.8.0" />
<PackageReference Include="Quartz.Serialization.Json" Version="3.8.0" />
<PackageReference Include="Blazorise.SpinKit" Version="1.2.4" />
<PackageReference Include="RestSharp" Version="106.13.0" />
<PackageReference Include="Serilog" Version="3.1.1" />
<PackageReference Include="Serilog.AspNetCore" Version="7.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="5.0.1" />
</ItemGroup>

<ItemGroup>
Expand Down
13 changes: 7 additions & 6 deletions src/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -294,7 +293,6 @@ public static void Main(string[] args)
options.Endpoint = new Uri(Constants.OTEL_EXPORTER_ENDPOINT);
})
.AddEntityFrameworkCoreInstrumentation()
.AddQuartzInstrumentation()
);
}

Expand Down Expand Up @@ -347,6 +345,9 @@ public static void Main(string[] args)
//TODO Auth in the future, DAPR(?)
app.MapGrpcService<NodeGuardService>();

var loggerFactory = new LoggerFactory()
.AddSerilog(Log.Logger);
Quartz.Logging.LogContext.SetCurrentLogProvider(loggerFactory);
app.Run();
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/Services/LightningClientService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -53,7 +54,7 @@ public LightningClientService(ILogger<LightningClientService> 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
Expand All @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion test/NodeGuard.Tests/NodeGuard.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="6.12.0" />
<PackageReference Include="Grpc.Core.Api" Version="2.55.0" />
<PackageReference Include="Grpc.Core.Api" Version="2.59.0" />
<PackageReference Include="Grpc.Core.Testing" Version="2.46.6" />
<PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="7.0.9" />
<PackageReference Include="Microsoft.Extensions.Logging.Configuration" Version="7.0.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.3" />
<PackageReference Include="Moq" Version="4.18.4" />
<PackageReference Include="Moq.EntityFrameworkCore" Version="7.0.0.2" />
Expand Down

0 comments on commit 7d65c2b

Please sign in to comment.