-
Notifications
You must be signed in to change notification settings - Fork 763
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
Microsoft.Extensions.Http.Resilience Breaks Application Insights Integration #5421
Comments
Could you please provide a small stand-alone repro? |
Sorry the original upgrade was to "Microsoft.Extensions.Http.Resilience" Version="8.6.0" |
Can you provide an email address where I can send the repo? |
You can submit your repro sample via the VS Feedback (an icon on the top RHS in VS or via via https://developercommunity.visualstudio.com/dotnet directly). There you'll be able to attach a zip archive and all the necessary repro instructions. Once submitted, please link the issue here so we can route it to our team. Please note, that we're not able to debug customer apps (unless you have a service agreement, in which case you need to speak with your CSR), that's why we ask for small standalone repros. |
The issue comes down to adding or removing this line, in a very minimal solution with only 3 NuGet package references. With this AddResilienceHandler() all App Insights integration stops. Without it, the logging works as expected.
|
Here's the minimalist repo. Steps to Reproduce:
|
It looks like this call somehow affects the AI registrations: Line 118 in 39f5d27
@martintmk @iliar-turdushev any thoughts? |
@phil000 Thank you for reporting this. We'll investigate and get back to you. |
The ticket is a duplicate of the following one #5222. The issue is caused by AppInsights. Here is detailed explanation of the issue #5222 (comment). To fix the issue you have to reorder registration of AppInsights and Http.Resilience, in particular, first register AppInsights, and then Http.Resilience. For example, in the sample you provided you need to use the following order: public void ConfigureServices(IServiceCollection services, IConfiguration configuration, bool isDevelopment)
{
AddControllers(services);
// First - AppInsights.
AddTelemetry(services, isDevelopment);
// Then - Http.Resilience and any other services that use Keyed Services feature of DI
// https://learn.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-8.0#keyed-services.
AddHttpClients(services);
} @phil000 Please, confirm that the workaround works for you. If it does then we'll close the issue. Thank you. |
Confirming that reordering the dependency registration solves the issue for us. |
The AI bug is here: microsoft/ApplicationInsights-dotnet#2879 |
Duplicate of #5222 |
@iliar-turdushev could you please update the package's readme with this known issue? |
Sure, I will. Two customers already faced the issue. |
Description
This may seem like an obscure one, but it's easily reproducible.
2 months ago we upgraded 2 solutions to Microsoft.Extensions.Http.Resilience 8.9.0. We didn't notice at the time but we have now noticed both projects have had all App Insights integration suppressed - all Traces, Requests and Dependencies stopped.
We have upgrade to Microsoft.Extensions.Http.Resilience 8.9.1 and the issue is reproducible in the local development environment.
NuGetSolver tool didn't really add much info.
Reproduction Steps
Add Microsoft.Extensions.Http.Resilience 8.9.1 to solution. App insights integration is disabled.
Remove Microsoft.Extensions.Http.Resilience 8.9.0 from solution App insights integration is working again.
Expected behavior
App insights integration is working
Actual behavior
App insights integration is disabled
Regression?
No
Known Workarounds
Remove Microsoft.Extensions.Http.Resilience
Configuration
.NET Core
Version: 8.0.8
Architecture: x64
Other information
No response
The text was updated successfully, but these errors were encountered: