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

App Insights breaks when a keyed service is added #2879

Open
sander1095 opened this issue Jun 12, 2024 · 6 comments
Open

App Insights breaks when a keyed service is added #2879

sander1095 opened this issue Jun 12, 2024 · 6 comments
Labels

Comments

@sander1095
Copy link

sander1095 commented Jun 12, 2024

See the following bug report:

dotnet/extensions#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() 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"); 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 sander1095 added the bug label Jun 12, 2024
@sander1095 sander1095 changed the title Possible DI conflict with App Insights worker service and Microsoft.Extensions.Http.Resilience App Insights breaks when a keyed service is added Jun 20, 2024
@sander1095
Copy link
Author

Related to #2852 and #2828

@fplanjer
Copy link

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
Copy link

remcoros commented Sep 4, 2024

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

@rosebyte
Copy link

rosebyte commented Sep 9, 2024

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
Copy link

phil000 commented Sep 15, 2024

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

See: #5421

@edenmize
Copy link

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

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

No branches or pull requests

6 participants