Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service registration fails #2852

Open
vijuhe opened this issue Mar 1, 2024 · 8 comments
Open

Service registration fails #2852

vijuhe opened this issue Mar 1, 2024 · 8 comments

Comments

@vijuhe
Copy link

vijuhe commented Mar 1, 2024

ASP.NET Core API service registration of Application Insights classes is not completed correctly and the API requests fail with the following message: Unable to resolve service for type 'Microsoft.ApplicationInsights.TelemetryClient'.
TelemetryClient is a dependency of a class in the API.
Application Insights was registered with the IServiceCollection extension method AddApplicationInsightsTelemetry.
Version of package Microsoft.ApplicationInsights.AspNetCore is 2.22.0.

The error started occurring after updating another API dependency Microsoft.Extensions.Http.Resilience to version 8.2.0.
I'm guessing that the error has something to do with this change in the service collection dotnet/runtime#64427. There have been service collection usage errors in other Microsoft packages as well after the change (e.g. Microsoft.Identity.Web: AzureAD/microsoft-identity-web#2676).

@onionhammer
Copy link

onionhammer commented Mar 12, 2024

I'm also seeing this same issue after upgrading these two dependencies (within an Aspire project)

Maybe related:

@4865783a5d
Copy link

We're also seeing this after we migrated from Polly to Microsoft.Extensions.Http.Resilience 8.3.0

@rajkumar-rangaraj
Copy link
Member

Could one of you assist with a demo app that could replicate this issue?

@vijuhe
Copy link
Author

vijuhe commented May 6, 2024

The issue can be replicated with this API https://github.com/vijuhe/ApplicationInsightsIssue. Just start the API and you get the error.

@kevincathcart-cas
Copy link

This is fundamentally just a bug with AddSingletonIfNotExists, which cannot handle iterating through keyed services. It is not trivial to directly fix this because one needs to check the new in 8.0 ServiceKey property before touching the implementation related properties on ServiceDescriptor, which would mean needing to reference Microsoft.Extensions.DependencyInjection.Abstractions 8.0.

Generally, the safe way to handle this short of logic is with .TryAddSingleton (if you want to skip registration if any implementation of the service type is present), or the much lesser known .TryAddEnumerable (to skip registration only if there is a registration of the given implementation type for the given service type). The name of the latter hinting at the fact that this is often desirable if you expect to resolve this service type as an IEnumerable, while often not as useful if you expect to resolve the service type directly.

I think .TryAddEnumerable is what is wanted here. I see only three relevant differences vs the existing method beyond tolerating keyed services. The first is that AddSingletonIfNotExists will skip registering if the implementation type is registered for literally any service type, not just the service type we are trying to register (that seems like a bug). The second difference is that AddSingletonIfNotExists also looks for subtypes of the implementation type, rather than just an exact type match (which could be a feature), but since the only usage of that method is for the sealed implementation type of DiagnosticsTelemetryModule, that difference really doesn't matter.

So simplest fix is probably as simple as replacing the body of AddSingletonIfNotExists with:

services.TryAddEnumerable(ServiceDescriptor.Singleton<TService, TImplementation>());

That simple fix also has the benefit of working with Microsoft.Extensions.DependencyInjection.Abstractions going back all the way to 2.0.

@fleed
Copy link

fleed commented Nov 6, 2024

Encountered a similar issue. Any plans to fix it?

@vmachacek
Copy link

I'm bitten by this yet again, and need to introduce ugly hacks because of this. Any plans to fix it?

@angularsen
Copy link

Fixed just now, in #2908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants