Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Please ADD ALL Changes to the UNRELEASED SECTION and not a specific release
### Fixed
- Suppress known Scriban 6.2.0 vulnerabilities pending upgrade
- SnsMessage: Token property was never populated from the constructor argument
- Pass ILogger to HealthCheckClient.ExecuteAsync to match new API in Credfeto.Docker.HealthCheck.Http.Client 0.0.72.928
### Changed
- Dependencies - Updated NSubstitute.Analyzers.CSharp to 1.0.17
- Switched to use minimal APIs
Expand All @@ -41,7 +42,6 @@ Please ADD ALL Changes to the UNRELEASED SECTION and not a specific release
- Dependencies - Updated xunit.analyzers to 1.27.0
- Dependencies - Updated xunit.v3 to 3.2.2
- Dependencies - Updated Serilog to 4.3.1
- Dependencies - Updated Credfeto.Docker.HealthCheck.Http.Client to 0.0.61.659
- Dependencies - Updated Figgle to 0.6.6
- Dependencies - Updated FunFair.CodeAnalysis to 7.1.35.1745
- Dependencies - Updated AWSSDK to 4.0.8.11
Expand All @@ -66,6 +66,7 @@ Please ADD ALL Changes to the UNRELEASED SECTION and not a specific release
- Dependencies - Updated Microsoft.NET.Test.Sdk to 18.7.0
- Dependencies - Updated Meziantou.Analyzer to 3.0.109
- Dependencies - Updated FunFair.Test.Common to 6.3.1.2342
- Dependencies - Updated Credfeto.Docker.HealthCheck.Http.Client to 0.0.72.928
### Removed
### Deployment Changes
<!--
Expand Down
2 changes: 1 addition & 1 deletion src/BuildBot/BuildBot.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
<ProjectReference Include="..\BuildBot.Watchtower\BuildBot.Watchtower.csproj" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Credfeto.Docker.HealthCheck.Http.Client" Version="0.0.61.659" />
<PackageReference Include="Credfeto.Docker.HealthCheck.Http.Client" Version="0.0.72.928" />
<PackageReference Include="Microsoft.Extensions.Hosting.Systemd" Version="10.0.9" />
<PackageReference Include="Microsoft.Extensions.Hosting.WindowsServices" Version="10.0.9" />
<PackageReference Include="Serilog" Version="4.3.1" />
Expand Down
16 changes: 13 additions & 3 deletions src/BuildBot/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using BuildBot.Helpers;
using Credfeto.Docker.HealthCheck.Http.Client;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.Logging;

namespace BuildBot;

Expand All @@ -19,9 +20,18 @@ public static class Program
)]
public static async Task<int> Main(string[] args)
{
return HealthCheckClient.IsHealthCheck(args: args, out string? checkUrl)
? await HealthCheckClient.ExecuteAsync(targetUrl: checkUrl, cancellationToken: CancellationToken.None)
: await RunServerAsync(args);
if (HealthCheckClient.IsHealthCheck(args: args, out string? checkUrl))
{
using ILoggerFactory loggerFactory = LoggerFactory.Create(static builder => builder.AddConsole());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observability gap: non-2xx HTTP responses return exit code 1 with no log output

The console logger added here only surfaces diagnostics on the exception path. Decompiling Credfeto.Docker.HealthCheck.Http.Client 0.0.72.928 confirms the relevant code:

using HttpResponseMessage httpResponseMessage = await httpClient.GetAsync(uri, cancellationToken);
return (!httpResponseMessage.IsSuccessStatusCode) ? 1 : 0;

There is no ILogger call on the non-2xx path — only on the caught-exception path (logger.HealthCheckFailed(uri, exception)). If the health endpoint returns 503, 500, or any non-2xx status, the process exits with code 1 and the console logger produces zero output. Docker marks the container unhealthy, but the operator sees nothing in the logs.

This is a library limitation, not something introduced in this PR — but it means the stated goal of "surface HealthCheckFailed errors" is only partially achieved: exception failures are now visible, non-2xx failures remain silent.

Consider wrapping the call to add our own logging for the non-2xx case, or raise an issue against credfeto/credfeto-docker-healthcheck-http-client to add logging for non-success status codes.


return await HealthCheckClient.ExecuteAsync(
targetUrl: checkUrl,
logger: loggerFactory.CreateLogger(nameof(Program)),
cancellationToken: CancellationToken.None
);
}

return await RunServerAsync(args);
}

private static async ValueTask<int> RunServerAsync(string[] args)
Expand Down
Loading