HealthCheck Extensions Project

[s4ndr0ne]

Hi @Havret,

Starting from the discussion (#528), this is the implementation.

As suggested, I created a separate project containing the healthcheck service and reported its use in the application.

Tell me what to improve.

Best,
Alessandro

[Havret]

@s4ndr0ne As I mentioned in our discussion, this is an excellent idea. 🙂 I left some comments with hints on how we can use the existing building blocks to polish it a bit.

[Havret]

This state should not be static. It may fall apart if we have more than one connection in a single application (which is the case in most production use cases I've seen). Instead, we should register this service per individual ActiveMQ connection.

[Havret]

Suggested change

	public static IHealthChecksBuilder AddActiveMqHealthCheck(this IHealthChecksBuilder builder, string name, string[] tags = null)
	public static IHealthChecksBuilder AddActiveMq(this IHealthChecksBuilder builder, string name, string[] tags = null)

[Havret]

I'd also suggest slightly changing the signature. If we could pass not only the health check name but also the IActiveMqBuilder (the same one we're using in the DI package) we would gain access to the connection or its events.

So this could like sth like this:

public static IHealthChecksBuilder AddActiveMqHealthCheck(
    this IHealthChecksBuilder builder,
    string name,
    IActiveMqBuilder activeMqBuilder,
    string[] tags = null)
{
    // Register a connection callback to wire up health check event handlers
    activeMqBuilder.ConfigureConnection((provider, connection) =>
    {
        var healthCheckService = provider.GetRequiredKeyedService<ArtemisHealthCheckService>(activeMqBuilder.Name);

        connection.ConnectionRecovered += healthCheckService.ConnectionRecovered;
        connection.ConnectionClosed += healthCheckService.ConnectionClosed;
        connection.ConnectionRecoveryError += healthCheckService.ConnectionRecoveryError;
    });

    // Ensure the ArtemisHealthCheckService is registered as a keyed singleton
    builder.Services.AddKeyedSingleton<ArtemisHealthCheckService>(activeMqBuilder.Name);

    // Register the health check using the same keyed instance to ensure consistency
    builder.Add(new HealthCheckRegistration(
        name,
        provider => provider.GetRequiredKeyedService<ArtemisHealthCheckService>(activeMqBuilder.Name),
        HealthStatus.Unhealthy,
        tags));

    return builder;
}

[s4ndr0ne]

What do you think if I directly extend IActionBuilder like this to enable it?

        public static IActiveMqBuilder EnableHealthChecks(this IActiveMqBuilder builder)
        {
            return builder.ConfigureConnection((provider, connection) =>
            {
                var healthCheckService = provider.GetRequiredKeyedService<ArtemisHealthCheckService>(builder.Name);

                healthCheckService.ConnectionOpen();

                // Attach event handlers to the connection for health check updates

                connection.ConnectionClosed += (sender, args) => healthCheckService.ConnectionClosed(sender, args);
                connection.ConnectionRecovered += (sender, args) => healthCheckService.ConnectionRecovered(sender, args);
                connection.ConnectionRecoveryError += (sender, args) => healthCheckService.ConnectionRecoveryError(sender, args);
            });
        }

[Havret]

I'm not a fan of this kind of indirect reference. It may be problematic and not easy for users to figure out.

[s4ndr0ne]

Ok reverted to previous version with parameter

// Add health checks for ActiveMQ Artemis
            services
             .AddHealthChecks()
             .AddActiveMq(name: "my-artemis-cluster", activeMqBuilder: activeMqBuilder, tags: new[] { "activemq" });

[Havret]

If I'm not mistaken, this registration will create its own instance of ArtemisHealthCheckService, regardless of what you registered above as a singleton.

[s4ndr0ne]

you are right by registering it this way we can have it for the necessary instances

services.AddActiveMq(name: "my-artemis-cluster" ...
   .EnableQueueDeclaration()                 
   .EnableHealthChecks(); // Enable health check integration

services
             .AddHealthChecks()
             .AddActiveMq("my-artemis-cluster", tags: new[] { "activemq" });

[Havret]

Then we wouldn't need this method.

[Havret]

This should, in fact, be false. When the connection opens, it should trigger a change in state.

[s4ndr0ne]

add method

public void ConnectionOpen() { IsOpen = true; Description = "Connection is open"; }

for the connected state change

[Havret]

Please make this internal.

[s4ndr0ne]

Done

[Havret]

Thanks @s4ndr0ne, merged!