AspNetCore.HealthChecks.Rabbitmq leaks connections when using the factory

[JanEggers]

What happened: when using

 services.AddHealthChecks()
.AddRabbitMQ(factory: c => /* my factory does not matter*/)

the healthcheck does not close the connection causing a massive ammount of connections to be opened

What you expected to happen: either one of these:

• the healthcheck should cache the connection after it was initially created and use that one connection for subsequent health checks
• the healthcheck should close the connection after healthcheck is completed if the connection was created with the factory
• drop the factory support and require the connection to be provided by DI (I would discourage this approach as the connection requires async code to be created which does not play nicely with di as far as im aware and the creation may fail if there is no broker running)

Version 8.0.2 was working fine Version 9.0.0 contains the issue.

propably this change: https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/pull/2343

[JanEggers]

I actually read the readme and now its clear. but I was just updating from one version to the next and was not expecting that a parameter called factory was supposed to provide a cached singleton.

[JanEggers]

FYI my solution for the time beeing:

services.AddSingleton<Lazy<Task<IConnection>>>(c => new Lazy<Task<IConnection>>(() => /* my factory does not matter*/);

services.AddHealthChecks() .AddRabbitMQ(factory: c => c.GetRequiredService<Lazy<Task<IConnection>>>().Value.Result);

that way I still have access to the service provider while creating the connection (I use it to resolve configuration)

[adamsitnik]

Why do you register a Lazy factory? The DI container won't create it until it's needed for the first time.

cc @eerhardt

[JanEggers]

I guess its not needed then.

[kooshan]

This is a really fucked up way to setup a so called FACTORY!!! for health check. Wasted my whole day on finding the source of resource leakage. It looks more like an unintentional bug now being disguised as an intentional clever design. Please do fix this atrocity.

That setup guide in the readme offers no help either.

1. What if I have another service which will use IConnection and I don't want to use singleton registration for that?
2. How can I access my IConfiguration for rabbit user/pass when I use the static Lazy to initialize a factory? Should I always hardcode my user/pass in my code?

Wouldn't it be easier to add a using at the place where you invoke the factory instead of this nonsense?

[VsevolodIO]

Here is the working solution.

    public static IHealthChecksBuilder AddRabbitMQ(this IHealthChecksBuilder healthChecksBuilder, string rabbitMqConnectionString)
    {
        healthChecksBuilder.Services.AddSingleton(_ =>
        {
            var connectionFactory = new ConnectionFactory { Uri = new Uri(rabbitMqConnectionString) };
            return connectionFactory.CreateConnectionAsync().GetAwaiter().GetResult();
        });

        // It will take the above singleton IConnection from DI container according to the source code.
        healthChecksBuilder.AddRabbitMQ();

        return healthChecksBuilder;
    }

[cpterwort]

Hello! I was wondering if this would be addressed sometime soon? We are seeing a lot of open connections within our app as I came across this issue.

[ThumbGen]

A probably naive question... Isn't a http ping healthcheck safer and more lightweight? Ping api/health/checks/alarms at 15672 (as an alternative for those having the plugin activated, of course)