App Insights breaks when a keyed service is added

[sander1095]

See the following bug report:

https://github.com/dotnet/extensions/issues/5222

When a keyed service is added, app insights breaks silently (no logging is sent to app insights) or breaks with an exception when the user tries to retrieve a telemetryclient.

Comments that lead to this issue being related to App Insights:

---

@kevincathcart-cas

ApplicationInsights has a AddSingletonIfNotExists method it uses internally that does not work correctly if there are any keyed services registered. And AddStandardResilienceHandler does register at least one keyed service.

ApplicationInsights is generally designed as best effort observability, which means that if it encounters errors it is not supposed to take down your service, and this is taken to the point where it mostly silently catches exceptions that occur inside .AddApplicationInsightsTelemetryWorkerService() (although it does call WorkerServiceEventSource.Instance.LogError(...) with the exception).

(This also means that doing .GetRequiredService<TelemetryClient>() is dangerous, as it is very deliberately the case that the registration could be missing if there were problems when trying to register the service.)

All that said, this is fundamentally just a bug with Application Insight's AddSingletonIfNotExists method, which really should be updated able to handle a ServiceCollection that contains keyed services, since that is a standard DI feature now, and users should not need to defer registering such services until after calling AddApplicationInsightsTelemetryWorkerService.

---

@joperezr

Thanks for the info @kevincathcart-cas, I actually didn't know that so today I learned 😃. I was able to confirm this by swapping the call to AddStandardResilienceHandler in your repro to a simple builder.Services.AddKeyedSingleton<MyService>("myService"); and that still repros if called before adding AppInsights. Given this isn't really an issue with the our resilience library, I'll go ahead and close this issue, and I'd suggest you opening one in the App Insights repo so that they can fix this issue.

[sander1095]

Related to #2852 and #2828

[fplanjer]

Just spent a good few days trying to figure out why App Insights was not working until I finally discovered it is because I added some keyed services. I think this should be fixed with the highest priority.

[remcoros]

We worked around it by registering application insights BEFORE any other (keyed) services. In our case that was Orleans 8

[rosebyte]

The root cause is at this line which fails if any keyed service is added to the service collection at the time. What needs to be done is to first check if the service is a keyed service.

Anyway, there is another place that might be discussed. It effectively swallows any exception and makes debugging incredibly difficult. Could we possibly change the function to throw and let its caller to decide if it's safe to swallow the exception or not?

[phil000]

We have had the same issue. App Insights just silently stopped working due to a new dependency being used.

See: #5421

[edenmize]

Same here, spent two days trying to figure out what the issue was. When we commented out the KeyedService everything works fine.

[nil-biribiri]

We worked around it by registering application insights BEFORE any other (keyed) services. In our case that was Orleans 8

Thanks! This workaround does work for me.

[vejhe]

I had the same issue and it was only surfaced because of Scrutor uses KeyedServices for the decorator pattern starting with version 5 (released September 25). Just for the next one struggling with AI and Scrutor to find.

[mattchenderson]

For my own reproes of this, the culprit appears to be this method: https://github.com/microsoft/ApplicationInsights-dotnet/blob/e34286b4c8175d7ea47f9227c5b981d661cac470/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs#L293-L301

o.ImplementationInstance is null for my keyed services. I believe it would be a quick change to at least tolerate such cases by adding some defensiveness:

internal static void AddSingletonIfNotExists<TService, TImplementation>(this IServiceCollection services)
    where TService : class
    where TImplementation : class, TService
{
-    if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? o.ImplementationInstance.GetType())))
+    if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? (o.ImplementationInstance is not null ? o.ImplementationInstance.GetType() : null))))
    {
        services.AddSingleton<TService, TImplementation>();
    }
}

That's probably not the cleanest way, but I just wanted to share the general idea.

[pcn-michaelulloa]

We worked around it by registering application insights BEFORE any other (keyed) services. In our case that was Orleans 8

It worked, or at least I thought. It stopped throwing the exception, but nothing was being sent to my app insights Can't believe it's been so long with this problem still

[pinkfloydx33]

Any update on this. Just spent hours to find out this was the issue. Any chance there's a fix in a preview version?

[kwdowns]

For my own reproes of this, the culprit appears to be this method:

ApplicationInsights-dotnet/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs

Lines 293 to 301 in e34286b

internal static void AddSingletonIfNotExists<TService, TImplementation>(this IServiceCollection services) where TService : class where TImplementation : class, TService { if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? o.ImplementationInstance.GetType()))) { services.AddSingleton<TService, TImplementation>(); } } o.ImplementationInstance is null for my keyed services. I believe it would be a quick change to at least tolerate such cases by adding some defensiveness:

internal static void AddSingletonIfNotExists<TService, TImplementation>(this IServiceCollection services) where TService : class where TImplementation : class, TService {

• if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? o.ImplementationInstance.GetType())))

• if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? (o.ImplementationInstance is not null ? o.ImplementationInstance.GetType() : null)))) { services.AddSingleton<TService, TImplementation>(); } } That's probably not the cleanest way, but I just wanted to share the general idea.

Just spent my afternoon tracking down what was going on and traced it down to this line as well, you are on the right track.

[asvishnyakov]

Guys, this is not funny. Why this is still not fixed? I just spend a day trying to understand why my library isn't compatible with Application Insights (and it will fail randomly), just to find this issue. It even has already approved PR which fixes it. Why it isn't merged and released already?! It's unacceptable.

[asvishnyakov]

Also, it seems like PR #2908 should resolve the issue, but still not merged and not released.

[OlegoO]

Do you have any plans/date to release this fix?

[shellisj-spc]

Any news on when this change will get merged?

[TimothyMothra]

Just released 2.23.0-beta1 which contains the fix for this issue https://www.nuget.org/packages/Microsoft.ApplicationInsights.AspNetCore/2.23.0-beta1