Xabaril / AspNetCore.Diagnostics.HealthChecks

Enterprise HealthChecks for ASP.NET Core Diagnostics Package
Apache License 2.0
4.01k stars 782 forks source link

HealthChecks.AzureServiceBus can create duplicate clients #1890

Open TomMalow opened 1 year ago

TomMalow commented 1 year ago

What would you like to be added: The HealthChecks.AzureServiceBus makes use of ConcurrentDictionary to reuse clients for queues and topics. However, the connection key is generated by using both the service bus connection-string/endpoint+queue-name/topic-name. which means that multiple health check to queues/topics within the same Service Bus will create multiple clients, where it should only create a single client and us that for all of the queues/topics in that Service Bus.

I propose to change the connection key to only use the connection-string/endpoint as the key in the client dictionary and change the code to use the ClientCache instead of an internal ConcurrentDictionary to reuse clients across the individual HealthCheck implementations.

The relevant Azure ServiceBus HealthCheck implementations that I would like to be modified:

Why is this needed: Can reduce the amount of open connection to the same Service Bus and reduce the execution time for the first call where clients are created.

I don't see any conflict which this change unless I have overlooked a specific use case.

I also have a PR ready which this change if approved as a valid solution.

sungam3r commented 1 year ago

PR is welcome.

jkdmyrs commented 6 months ago

@sungam3r I was looking to pickup this issue but looks like it was completed in #1974, should the issue be closed?