dadhi / DryIoc

DryIoc is fast, small, full-featured IoC Container for .NET
MIT License
1.01k stars 123 forks source link

Combining AspNet.Core, ApplicationInsights for AspNet.Core and DryIoc results in no Request logging in Application Insights #513

Closed cadwal closed 2 years ago

cadwal commented 2 years ago

Note that while looking into this problem I realized that DryIoc is no longer needed for the code affected. I now only needed to change four registrations to MS syntax instead instead of DryIoc to work around the issue so I wont be pursuing a solution to this problem at this time, this is now a "that didn't work" report. Once upon a time there were more shared code that required DryIoc.

Versions affected:

Net 6.0 / AspNetCore 6.0 Microsoft.ApplicationInsights 2.18.0 -> 2.21.0 DryIoc.Microsoft.DependencyInjection 6.0.2 DryIoc 5.1.0

Versions known to work:

Net Core 3.1 / AspNetCore 3.1 Microsoft.ApplicationInsights 2.18.0 DryIoc.Microsoft.DependencyInjection 5.0.0 DryIoc 4.5.0

The problem that occurs is that AspNet.Core does not log any requests to Application Insights with the affected version combo. Dependencies, traces etc works, but no requests.

Through lots of debugging I realized that this is caused by some strange DI effect when the Application Insights modules are configured.

Deep in the Build phase of the application setup an object of the Application Insights class TelemetryConfigurationOptionsSetup will be instantiated through DI. With the affected version combo, the constructor parameter IEnumerable\<ITelemetryModule> modules will end up with a list of, in my case, 4 modules, none of the them being the RequestTrackingTelemetryModule responsible for Request loggning. With the non-affected versions or without DryIoc as the ServiceProviderFactory, the number of modules are at least 8 including the RequestTrackingTelemetryModule.

TelemetryConfigurationOptionsSetup initializes only the modules it gets in its constructor parameter, any uninitialized modules, of course, logs nothing...

If I, after the build phase has finished, do a manual Services.GetServices\<ITelemetryModule>() I end up with a list of 8 modules, four of them not initialized and non-working with the affected version combo.

dadhi commented 2 years ago

@cadwal Thanks for reporting. Maybe you have an with repro?

cadwal commented 2 years ago

Perhaps later, right now after a day and a half of :headbang: I just want to forget about it for a while.

cadwal commented 2 years ago

Unfortunately it turns out that a cut-down sample does not exhibit the issue.

Cut-down in this case means:

But otherwise maintain the actual structure, startup-behavior etc.

Hard to tell what the trigger is then. :-(

My suspicion would be something with code spread out over a combination of Net Std 2 and Net Core 6 libraries in the "real" code, but not in the sample, but that is no more than a guess....

dadhi commented 2 years ago

Thanks for trying.

dadhi commented 2 years ago

@cadwal Just thinking, how your DryIoc registrations look like? How do you use DryIocAdapterFactory? Any non defaults rules?

cadwal commented 2 years ago

Both in the original code and in the sample I tried with and without the rules below for the container:

var container = new Container(rules => rules.With(propertiesAndFields: PropertiesAndFields.Auto)
                                   .WithoutThrowIfDependencyHasShorterReuseLifespan()
                                   .WithMicrosoftDependencyInjectionRules());

But the behavior was the same in both cases. Not working in the original, and working either way in the sample.

I don't have a runnable version of the previous code right now, but I seem to remember I had to add WithoutThrowIfDependencyHasShorterReuseLifespan for it to work in Net Core 3.1.x on a minor net core version update....

The Factory is setup as follows:

                    new DryIocServiceProviderFactory(container,
                    (registrator, descriptor) =>
                    {
#if DEBUG
                        if (descriptor.ServiceType == typeof(ILoggerFactory))
                        {
                            Console.WriteLine($"Logger factory is registered as instance: {descriptor.ImplementationInstance != null}");
                        }
#endif
                        return false; // fallback to default registration logic
                    }));

That logging is also a legacy from several versions back, I just never removed it.

dadhi commented 2 years ago

You may try to modify the rules:

var container = new Container(rules => rules
    .With(propertiesAndFields: PropertiesAndFields.Auto)
    .WithoutThrowIfDependencyHasShorterReuseLifespan()
    .WithMicrosoftDependencyInjectionRules()
    .WithVariantGenericTypesInResolvedCollection() // those rules where removed in a newer DryIoc version for conformity (but!)
   //.WithServiceProviderGetServiceShouldThrowIfUnresolved() // second step to turn off fallback to MS resolution and see the original exceptions. 
);
cadwal commented 2 years ago

I definitely think I want to forget all about this now...

"Obviously" I could now no longer reproduce this issue even in the original code, well, original in that I manually readded the DryIoc service factory code quoted above.

So moved the code around a bit to se if there where any changes by adding the factory later in the startup, like after adding AppInsights to the factory. I still had the debugging set up with breakpoints in the AppInsights code and on the first run with the code in a specific place in the startup, right after UseAppInsights, the module list that I mentioned earlier that would contain 4 instead of 8 modules when it did not work actually contained 11 modules, but I did not look at the list since I assumed it would be reproducible.

Well, it is not. I went and tried the rules changes you mentioned and everything still worked fine, went back to just having the two first rules and it still worked fine but with 8 modules in the list...

dadhi commented 2 years ago

Omg, those adventures :) So nah, then let it rest in piece. Thanks.

cadwal commented 2 years ago

Yeah, buried deep :-)