Live metrics stops working when adding KeyedServices BEFORE AddApplicationInsightsTelemetry() in dotnet 8 rc2 about applicationinsights-dotnet HOT 7 OPEN

Just to add to this as i was about to type up a bug ticket for the same issue until i found this.

You can get past this for now by changing your DI ordering. If you do the application insights before you do the keyed service injections then it will work.

The reason this is failing is because of the internal "AddSingletonIfNotExists" extension method in the library which is looping through the already registered services.

If you take the example code below and change the order so Example2 is before Example1 you will notice it works as the extension method is not conflicting with keyed service that have already been registered. Its not a DI issue in itself, its more a bug with how they are looping over the pre-registered services when trying to register more.

I have abstracted the code below from the library which is causing the issue, to help show why its breaking, so people have an example to work from:-

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

HostApplicationBuilder builder = Host.CreateApplicationBuilder(args);
builder.Services.AddKeyedScoped<IExample, Example>("example");
builder.Services.AddSingletonIfNotExists<IExampleTwo, ExampleTwo>();//Breaks due to extension method

using IHost host = builder.Build();

public interface IExample { }
public class Example : IExample { }

public interface IExampleTwo { }
public class ExampleTwo : IExampleTwo { }

//Abstracted extension from Application Insights library
public static class Extensions
{
    public 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>();
        }
    }
}

from applicationinsights-dotnet.

Wow, just spent hours trying to figure out why a simple application insights setup didn't work in my .net core 8 web api.
I put the application insights setup above the keyed services and its working now.
Get it fixed ASAP please.

from applicationinsights-dotnet.

I just spent a day debugging some changes unrelated to Application Insights that was hit by this bug. Luckily the changes never made it to production, since the application crashed on startup due to this setup in our Serilog config:

WriteTo.ApplicationInsights(services.GetRequiredService<TelemetryConfiguration>(), TelemetryConverter.Traces)

The TelemetryConfiguration was not registered due the exception thrown in AddSingletonIfNotExists.

After hours of debugging to figure out what I changed to cause this issue, I isolated it to adding resilience to an HttpClient with AddStandardResilienceHandler() from Microsoft.Extensions.Http.Resilience.

Either AddSingletonIfNotExists has to be updated to handle keyed services, or the ServiceDescriptor implementation has to be changed to not throw exception on property access.

The workaround of having to register Application Insights (or anything else not updated to handle keyed services) first, before anything using keyed service, is not acceptable in the long run. This will not scale as keyed services usage increases in the future.

from applicationinsights-dotnet.

We've just hit this. Right now it prevents targeting net8.0 without workarounds, and worse - it's enough if a project references Microsoft.Extensions.DependencyInjection.Abstractions v8 (which is supported on net6 as well), so simply by a dependency update (not retargeting to new framework) it can crash. Precondition of inserting Keyed registration is still required.

Edit: I've raised dotnet/runtime#95789 since it seems to be a very problematic regression.

from applicationinsights-dotnet.

@tore-hammervoll This was not meant as a permanent fix, it was simply to allow people to progress while the Microsoft team look into it and resolve the problem. My suggestion was to save people having to go through and look at the internal code just to get their setup working.

I do agree with you that in the longer term, this issue will need to be looked at but i would assume thats why this ticket is still marked as "open"?

from applicationinsights-dotnet.

@wc-whiteheadd My post was not intended as a response to the workaround, that was unintended. The workaround you posted has unblocked us from this bug, and is much appreciated.

I only intended to stress the importance of an actual fix, before this problem gets bigger when more third party packages (or official Microsoft packages) starts relying on keyed services. The consequences of this issue is a bug that is hard to debug, especially when it is triggered by completely unrelated changes. That coupled with the fact that it can even be hard to discover since the exception is caught in AddApplicationInsights(), and only manifests as live metrics not working in a running application (I suspect their are other potential issues caused by this, as there are several services that are not registered properly when this exception is thrown). I do not envy anyone finding out their live metrics (or whatever else) has not worked for days, weeks, or even months, and need to find out what caused it.

from applicationinsights-dotnet.

@tore-hammervoll that makes senses and i completely agree with you, this certainly needs fixing quite urgently. Hopefully our information will help their investigation into fixing it faster 😄

from applicationinsights-dotnet.