TryAddSingleton possible misuse in AddHealthChecks method

[sungam3r]

Describe the bug

Maybe I'm wrong, but I think that in this line
https://github.com/aspnet/Extensions/blob/112c1a8ae1c0a921eae4490abd4ccdfcdb899574/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceCollectionExtensions.cs#L29
there must be AddSingleton method instead of TryAddSingleton method, because TryAddSingleton will check only ServiceType property:
https://github.com/aspnet/Extensions/blob/3ba072c25373c19ab2cfe34483e733f3b926b2c0/src/DependencyInjection/DI.Abstractions/src/Extensions/ServiceCollectionDescriptorExtensions.cs#L88

So, if we have multiple hosted services (one is enough) which were added to IServiceCollection
(probably through AddHostedService extension method) before AddHealthChecks call , then the TryAddSingleton won`t do anything.

Also, as I see in comments, AddHealthChecks method is supposed to be idempotent, so maybe it is better to use TryAddEnumerable method explicitly? https://github.com/aspnet/Extensions/blob/3ba072c25373c19ab2cfe34483e733f3b926b2c0/src/DependencyInjection/DI.Abstractions/src/Extensions/ServiceCollectionDescriptorExtensions.cs#L585

TryAddEnumerable will check both ServiceType and ImplementationType properties, so HealthCheckPublisherHostedService ultimately will be added to IServiceCollection and Host will be able to resolve it through Services.GetService<IEnumerable<IHostedService>>().

[rynowak]

Yes, you are right. This is a bug!

[sungam3r]

Will this be fixed in the next release?

[rynowak]

This will be fixed in 3.0 - is this blocking for you in 2.2? Do you need help finding a workaround?

[sungam3r]

No, now it is not critical. Just after reading the release notes about health checks in netcore 2.2, looking through the code found it. Thanks!

[espenrl]

Can this be fixed in a patch release of 2.2? We have several IHostedService services and thus the HealthCheckPublisherHostedService is never registered with the container. I can't do the registration myself because HealthCheckPublisherHostedService is an internal class. End result is that we can't use the publisher feature. A long time to 3.0...

EDIT: I moved the registrations of our custom IHostedService until after AddHealthChecks(). Then it worked.

[NatMarchand]

For people stuck, one possible workaround is:

services.AddHealthChecks();

// This is a hack to fix an issue with the AddApplicationInsightsPublisher() call above
services.TryAddEnumerable(ServiceDescriptor.Singleton(typeof(IHostedService), typeof(HealthCheckPublisherOptions).Assembly.GetType("Microsoft.Extensions.Diagnostics.HealthChecks.HealthCheckPublisherHostedService")));

[sungam3r]

@NatMarchand I see your workaround in docs but it is written incorrectly. HealthCheckServiceAssembly should be changed to "Microsoft.Extensions.Diagnostics.HealthChecks.HealthCheckPublisherHostedService".