fix: address MonitorServer review — dispose resources, add cancellation, improve test reliability

This commit is contained in:
Joseph Doherty
2026-02-22 22:30:14 -05:00
parent 63198ef83b
commit 818bc0ba1f
3 changed files with 27 additions and 10 deletions

View File

@@ -13,6 +13,7 @@ public sealed class MonitorServer : IAsyncDisposable
{ {
private readonly WebApplication _app; private readonly WebApplication _app;
private readonly ILogger<MonitorServer> _logger; private readonly ILogger<MonitorServer> _logger;
private readonly VarzHandler _varzHandler;
public MonitorServer(NatsServer server, NatsOptions options, ServerStats stats, ILoggerFactory loggerFactory) public MonitorServer(NatsServer server, NatsOptions options, ServerStats stats, ILoggerFactory loggerFactory)
{ {
@@ -25,7 +26,7 @@ public sealed class MonitorServer : IAsyncDisposable
_app = builder.Build(); _app = builder.Build();
var basePath = options.MonitorBasePath ?? ""; var basePath = options.MonitorBasePath ?? "";
var varzHandler = new VarzHandler(server, options); _varzHandler = new VarzHandler(server, options);
_app.MapGet(basePath + "/", () => _app.MapGet(basePath + "/", () =>
{ {
@@ -44,10 +45,10 @@ public sealed class MonitorServer : IAsyncDisposable
stats.HttpReqStats.AddOrUpdate("/healthz", 1, (_, v) => v + 1); stats.HttpReqStats.AddOrUpdate("/healthz", 1, (_, v) => v + 1);
return Results.Ok("ok"); return Results.Ok("ok");
}); });
_app.MapGet(basePath + "/varz", async () => _app.MapGet(basePath + "/varz", async (HttpContext ctx) =>
{ {
stats.HttpReqStats.AddOrUpdate("/varz", 1, (_, v) => v + 1); stats.HttpReqStats.AddOrUpdate("/varz", 1, (_, v) => v + 1);
return Results.Ok(await varzHandler.HandleVarzAsync()); return Results.Ok(await _varzHandler.HandleVarzAsync(ctx.RequestAborted));
}); });
// Stubs for unimplemented endpoints // Stubs for unimplemented endpoints
@@ -106,6 +107,8 @@ public sealed class MonitorServer : IAsyncDisposable
public async ValueTask DisposeAsync() public async ValueTask DisposeAsync()
{ {
await _app.StopAsync();
await _app.DisposeAsync(); await _app.DisposeAsync();
_varzHandler.Dispose();
} }
} }

View File

@@ -8,7 +8,7 @@ namespace NATS.Server.Monitoring;
/// Handles building the Varz response from server state and process metrics. /// Handles building the Varz response from server state and process metrics.
/// Corresponds to Go server/monitor.go handleVarz function. /// Corresponds to Go server/monitor.go handleVarz function.
/// </summary> /// </summary>
public sealed class VarzHandler public sealed class VarzHandler : IDisposable
{ {
private readonly NatsServer _server; private readonly NatsServer _server;
private readonly NatsOptions _options; private readonly NatsOptions _options;
@@ -21,17 +21,17 @@ public sealed class VarzHandler
{ {
_server = server; _server = server;
_options = options; _options = options;
var proc = Process.GetCurrentProcess(); using var proc = Process.GetCurrentProcess();
_lastCpuSampleTime = DateTime.UtcNow; _lastCpuSampleTime = DateTime.UtcNow;
_lastCpuUsage = proc.TotalProcessorTime; _lastCpuUsage = proc.TotalProcessorTime;
} }
public async Task<Varz> HandleVarzAsync() public async Task<Varz> HandleVarzAsync(CancellationToken ct = default)
{ {
await _varzMu.WaitAsync(); await _varzMu.WaitAsync(ct);
try try
{ {
var proc = Process.GetCurrentProcess(); using var proc = Process.GetCurrentProcess();
var now = DateTime.UtcNow; var now = DateTime.UtcNow;
var uptime = now - _server.StartTime; var uptime = now - _server.StartTime;
var stats = _server.Stats; var stats = _server.Stats;
@@ -100,6 +100,11 @@ public sealed class VarzHandler
} }
} }
public void Dispose()
{
_varzMu.Dispose();
}
/// <summary> /// <summary>
/// Formats a TimeSpan as a human-readable uptime string matching Go server format. /// Formats a TimeSpan as a human-readable uptime string matching Go server format.
/// </summary> /// </summary>

View File

@@ -27,8 +27,17 @@ public class MonitorTests : IAsyncLifetime
{ {
_ = _server.StartAsync(_cts.Token); _ = _server.StartAsync(_cts.Token);
await _server.WaitForReadyAsync(); await _server.WaitForReadyAsync();
// Give monitoring server time to start // Wait for monitoring HTTP server to be ready
await Task.Delay(200); for (int i = 0; i < 50; i++)
{
try
{
var response = await _http.GetAsync($"http://127.0.0.1:{_monitorPort}/healthz");
if (response.IsSuccessStatusCode) break;
}
catch (HttpRequestException) { }
await Task.Delay(50);
}
} }
public async Task DisposeAsync() public async Task DisposeAsync()