Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Refactor probes and add more health checks #1159

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
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
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">

<ItemGroup>
<PackageReference Include="AspNetCore.HealthChecks.NpgSql" Version="8.0.2" />
<PackageReference Include="AspNetCore.HealthChecks.Redis" Version="8.0.1" />
<PackageReference Include="Altinn.ApiClients.Maskinporten" Version="9.1.0"/>
<PackageReference Include="AspNetCore.HealthChecks.UI.Client" Version="8.0.1" />
<PackageReference Include="HotChocolate.Subscriptions.Redis" Version="13.9.12" />
<PackageReference Include="Microsoft.Extensions.Caching.StackExchangeRedis" Version="8.0.8" />
<PackageReference Include="Altinn.Authorization.ABAC" Version="0.0.8"/>
<PackageReference Include="Bogus" Version="35.6.1"/>
<PackageReference Include="Microsoft.Extensions.Configuration.UserSecrets" Version="8.0.0"/>
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
<PackageReference Include="Microsoft.Extensions.Http.Polly" Version="8.0.8" />
<PackageReference Include="Npgsql" Version="8.0.4"/>
<PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="8.0.4"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace Digdir.Domain.Dialogporten.Infrastructure.HealthChecks;

internal sealed class WellKnownEndpointsHealthCheck : IHealthCheck
{
private readonly IHttpClientFactory _httpClientFactory;
private readonly ILogger<WellKnownEndpointsHealthCheck> _logger;
private readonly IConfiguration _configuration;

public WellKnownEndpointsHealthCheck(
IHttpClientFactory httpClientFactory,
ILogger<WellKnownEndpointsHealthCheck> logger,
IConfiguration configuration)
{
_httpClientFactory = httpClientFactory;
_logger = logger;
_configuration = configuration;
}

public async Task<HealthCheckResult> CheckHealthAsync(
HealthCheckContext context,
CancellationToken cancellationToken = default)
{
var wellKnownEndpoints = _configuration.GetSection("WebApi:Authentication:JwtBearerTokenSchemas")
.GetChildren()
.Select(section => section.GetValue<string>("WellKnown"))
.ToList();
arealmaas marked this conversation as resolved.
Show resolved Hide resolved

if (wellKnownEndpoints.Count == 0)
{
_logger.LogWarning("No Well-Known endpoints found in configuration.");
return HealthCheckResult.Unhealthy("No Well-Known endpoints are configured.");
}

var client = _httpClientFactory.CreateClient("HealthCheckClient");
arealmaas marked this conversation as resolved.
Show resolved Hide resolved

var unhealthyEndpoints = new List<string>();

foreach (var url in wellKnownEndpoints)
{
try
{
var response = await client.GetAsync(url, cancellationToken);
if (!response.IsSuccessStatusCode)
{
_logger.LogWarning("Health check failed for Well-Known endpoint: {Url}. Status Code: {StatusCode}", url, response.StatusCode);
unhealthyEndpoints.Add($"{url} (Status Code: {response.StatusCode})");
}
}
catch (HttpRequestException ex)
{
_logger.LogError(ex, "Exception occurred while checking Well-Known endpoint: {Url}", url);
return HealthCheckResult.Unhealthy($"Exception occurred while checking Well-Known endpoint {url}.");
}
catch (Exception ex)
{
_logger.LogError(ex, "An unexpected error occurred while checking Well-Known endpoint: {Url}", url);
return HealthCheckResult.Unhealthy($"An unexpected error occurred while checking Well-Known endpoint {url}.");
}
}

if (unhealthyEndpoints.Count > 0)
{
var description = $"The following endpoints are unhealthy: {string.Join(", ", unhealthyEndpoints)}";
return HealthCheckResult.Unhealthy(description);
}

return HealthCheckResult.Healthy("All Well-Known endpoints are healthy.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@
using Digdir.Domain.Dialogporten.Infrastructure.Persistence.Configurations.Actors;
using Digdir.Domain.Dialogporten.Infrastructure.Persistence.Repositories;
using HotChocolate.Subscriptions;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using StackExchange.Redis;
using ZiggyCreatures.Caching.Fusion;
using ZiggyCreatures.Caching.Fusion.NullObjects;
using Digdir.Domain.Dialogporten.Infrastructure.HealthChecks;

namespace Digdir.Domain.Dialogporten.Infrastructure;

Expand Down Expand Up @@ -182,6 +184,31 @@ public static IServiceCollection AddInfrastructure(this IServiceCollection servi
})
.AddPolicyHandlerFromRegistry(PollyPolicy.DefaultHttpRetryPolicy);

services.AddHttpClient("HealthCheckClient")
.ConfigureHttpClient((services, client) =>
{
client.Timeout = TimeSpan.FromSeconds(5);
})
.SetHandlerLifetime(TimeSpan.FromSeconds(10));

services.AddHealthChecks()
.AddRedis(
redisConnectionString: infrastructureSettings.Redis.ConnectionString,
name: "redis",
failureStatus: HealthStatus.Unhealthy,
tags: ["dependencies"])
.AddNpgSql(
connectionString: infrastructureSettings.DialogDbConnectionString,
name: "postgres",
healthQuery: "SELECT 1",
failureStatus: HealthStatus.Unhealthy,
tags: ["dependencies"])
.AddCheck<WellKnownEndpointsHealthCheck>(
"Well-Known Endpoints",
failureStatus: HealthStatus.Unhealthy,
tags: ["dependencies", "auth"]);


if (environment.IsDevelopment())
{
var localDeveloperSettings = configuration.GetLocalDevelopmentSettings();
Expand Down
33 changes: 28 additions & 5 deletions src/Digdir.Domain.Dialogporten.WebApi/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@
using FastEndpoints;
using FastEndpoints.Swagger;
using FluentValidation;
using HealthChecks.UI.Client;
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Diagnostics.HealthChecks;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using NSwag;
using Serilog;

Expand Down Expand Up @@ -123,14 +126,14 @@ static void BuildAndRun(string[] args)
};
})
.AddControllers(options => options.InputFormatters.Insert(0, JsonPatchInputFormatter.Get()))
.AddNewtonsoftJson()
.Services

.AddNewtonsoftJson()
.Services

// Auth
.AddDialogportenAuthentication(builder.Configuration)
.AddAuthorization()
.AddHealthChecks();
.AddHealthChecks()
.AddCheck("self", () => HealthCheckResult.Healthy(), tags: ["self"]);

if (builder.Environment.IsDevelopment())
{
Expand Down Expand Up @@ -200,7 +203,27 @@ static void BuildAndRun(string[] args)
uiConfig.DocumentPath = dialogPrefix + "/swagger/{documentName}/swagger.json";
});
app.MapControllers();
app.MapHealthChecks("/healthz");
app.MapHealthChecks("/startup", new HealthCheckOptions
{
Predicate = check => check.Tags.Contains("dependencies"),
ResponseWriter = UIResponseWriter.WriteHealthCheckUIResponse
});
Comment on lines +206 to +210
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 20, 2024

Choose a reason for hiding this comment

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

Missing registration of dependency health checks with 'dependencies' tag.

The endpoints /startup, /readiness, and /health are configured to report on health checks tagged with "dependencies", but no health checks are registered with this tag in the provided code. As a result, these endpoints may not return any checks, leading to incomplete health status reports.

Consider registering health checks for your dependencies (e.g., Redis, PostgreSQL) and tagging them with "dependencies". Here's how you might update your health check registration:

             .AddHealthChecks()
                 .AddCheck("self", () => HealthCheckResult.Healthy(), tags: ["self"])
+                .AddRedis(
+                    builder.Configuration.GetConnectionString("Redis"),
+                    name: "Redis",
+                    tags: new[] { "dependencies" })
+                .AddNpgSql(
+                    builder.Configuration.GetConnectionString("PostgreSQL"),
+                    name: "PostgreSQL",
+                    tags: new[] { "dependencies" });

Ensure that the connection strings "Redis" and "PostgreSQL" are correctly configured in your appsettings.json or environment variables.

Also applies to: 216-220, 221-225

Copy link
Collaborator

Choose a reason for hiding this comment

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

But these are registered in InfrastructureExtensions though right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@arealmaas, you're absolutely correct. The dependency health checks are registered in InfrastructureExtensions.cs with the 'dependencies' tag. I apologize for the oversight in my previous comment.

🐰🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries mate! We all do mistakes 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

@arealmaas, thank you for your understanding! 😊

(_/)
( ^.^)
/(")(")

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make me a haiku poem about the code changes in this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

@arealmaas,

Sure, here's a haiku about the code changes in this file:

New health checks in place,
Startup, liveness endpoints,
Dependencies checked.

app.MapHealthChecks("/liveness", new HealthCheckOptions
{
Predicate = check => check.Tags.Contains("self"), // Retains the self check for liveness
ResponseWriter = UIResponseWriter.WriteHealthCheckUIResponse
});
app.MapHealthChecks("/readiness", new HealthCheckOptions
{
Predicate = check => check.Tags.Contains("dependencies"),
ResponseWriter = UIResponseWriter.WriteHealthCheckUIResponse
});
app.MapHealthChecks("/health", new HealthCheckOptions
{
Predicate = check => check.Tags.Contains("dependencies"),
ResponseWriter = UIResponseWriter.WriteHealthCheckUIResponse
});

app.Run();
}

Expand Down
Loading