AzureServiceBusHealthCheck should not choose the connection method based solely on Credentials being null or not

[martincostello]

Please, fill the following sections to help us fix the issue

What happened:

I have an Aspire application where I configure Service bus like this:

var credential = new DefaultAzureCredential(new DefaultAzureCredentialOptions()
{
    ExcludeVisualStudioCredential = true,
});

builder.AddAzureServiceBusClient("AzureServiceBus", (p) => p.Credential = credential);

Aspire or the environment configuration provides the connection string, which may be either a connection string or just a fully-qualified namespace, but I always set a credential.

This works fine with Aspire configuring the ServiceBus client appropriately based on the content of the AzureServiceBus connection string.

However, when using the Azure ServiceBus emulator for local development, the health checks do not work because of the following code:

AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/AzureServiceBusHealthCheck.cs

Lines 35 to 43 in 52eb1e1

	protected ServiceBusClient CreateClient() =>
	Options.Credential is null
	? _clientProvider.CreateClient(Options.ConnectionString)
	: _clientProvider.CreateClient(Options.FullyQualifiedNamespace, Options.Credential);
	protected ServiceBusAdministrationClient CreateManagementClient() =>
	Options.Credential is null
	? _clientProvider.CreateManagementClient(Options.ConnectionString)
	: _clientProvider.CreateManagementClient(Options.FullyQualifiedNamespace, Options.Credential);

The health check is choosing how to connect based on Credentials being null or not, regardless of which connection method is configured on the options. This causes an exception to be thrown if Credentials is non-null but a connection string is in use:

fail: Microsoft.Extensions.Diagnostics.HealthChecks.DefaultHealthCheckService[103]
      Health check Azure_ServiceBusClient with status Unhealthy completed after 0.113ms with message '(null)'
      System.ArgumentException: The value '' is not a well-formed Service Bus fully qualified namespace. (Parameter 'fullyQualifiedNamespace')
         at Azure.Core.Argument.AssertWellFormedServiceBusNamespace(String argumentValue, String argumentName)
         at Azure.Messaging.ServiceBus.ServiceBusConnection..ctor(String fullyQualifiedNamespace, TokenCredential credential, ServiceBusClientOptions options)
         at Azure.Messaging.ServiceBus.ServiceBusConnection.CreateWithCredential[TCredential](String fullyQualifiedNamespace, TCredential credential, ServiceBusClientOptions options)
         at Azure.Messaging.ServiceBus.ServiceBusClient..ctor(String fullyQualifiedNamespace, Object credential, ServiceBusClientOptions options)
         at Azure.Messaging.ServiceBus.ServiceBusClient..ctor(String fullyQualifiedNamespace, TokenCredential credential)
         at HealthChecks.AzureServiceBus.ServiceBusClientProvider.CreateClient(String fullyQualifiedName, TokenCredential credential) in /home/runner/work/AspNetCore.Diagnostics.HealthChecks/AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/ServiceBusClientProvider.cs:line 13
         at HealthChecks.AzureServiceBus.AzureServiceBusHealthCheck`1.CreateClient() in /home/runner/work/AspNetCore.Diagnostics.HealthChecks/AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/AzureServiceBusHealthCheck.cs:line 36
         at HealthChecks.AzureServiceBus.AzureServiceBusQueueHealthCheck.<>c__DisplayClass3_0.<CheckHealthAsync>b__2(String _) in /home/runner/work/AspNetCore.Diagnostics.HealthChecks/AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/AzureServiceBusQueueHealthCheck.cs:line 41
         at ClientCache.GetOrAddAsyncDisposableAsync[T](String key, Func`2 clientFactory) in /home/runner/work/AspNetCore.Diagnostics.HealthChecks/AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/ClientCache.cs:line 44
         at HealthChecks.AzureServiceBus.AzureServiceBusQueueHealthCheck.<>c__DisplayClass3_0.<<CheckHealthAsync>g__CheckWithReceiver|0>d.MoveNext() in /home/runner/work/AspNetCore.Diagnostics.HealthChecks/AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/AzureServiceBusQueueHealthCheck.cs:line 41
      --- End of stack trace from previous location ---
         at HealthChecks.AzureServiceBus.AzureServiceBusQueueHealthCheck.CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken) in /home/runner/work/AspNetCore.Diagnostics.HealthChecks/AspNetCore.Diagnostics.HealthChecks/src/HealthChecks.AzureServiceBus/AzureServiceBusQueueHealthCheck.cs:line 28

What you expected to happen:

The ServiceBus client should be created based on whether ConnectionString or FullyQualifiedNamespace has a value if only one is provided and the other is null.

If both are provided, then ConnectionString would be chosen if Credentials is null.

If all three properties are set, then I'm not sure which is the best option.

How to reproduce it (as minimally and precisely as possible):

Source code sample:

var options = new AzureServiceBusQueueHealthCheckOptions()
{
    Credentials = new DefaultAzureCredential(),
    ConnectionString = "some valid connection string",
    FullyQualifiedNamespace = null,
    QueueName = "some valid queue",
};

var healthCheck = new AzureServiceBusQueueHealthCheck(options);
await healthCheck.CheckHealthAsync(context); // will return unhealthy due to an exception

Anything else we need to know?:

No.

Environment:

• .NET Core version: 9.0.7
• Healthchecks version: Whatever is shipped with Aspire 9.3.1
• Operative system: Windows 11
• Others: N/A

[tj-spyro]

In a similar vein to above, if Credentials is null and FullyQualifiedNamespace is configured, then it should default to a new DefaultAzureCredential.

This would align with the direction and behaviour the dotnet Aspire Team take for which options take precedence and default values.