search

serilog / serilog-aspnetcore

Serilog integration for ASP.NET Core
Apache License 2.0
1.32k stars 209 forks source link

Two-stage initialization: Can cause infinite recursion in the dependency injection engine if a service resolved by Serilog has an ILogger instance in the dependency graph #293

Closed marlon-tucker closed 2 years ago

marlon-tucker commented 2 years ago

Description Using NET6 and the new generic host builder, though I expect this has been an issue before as well, I ran into an issue where the hostBuilder.Build() call never returned, very similar behaviour to #289.

After much digging it was found that one of the dependencies on my sink was injecting a IMemoryCache instance, and that IMemoryCache service had a dependency on an ILogger.

The default Dependency Injection engine then calls back into the UseSerilog extension method restarting the cycle, and that cycle repeats until the call stack exceeds its maximum memory allocation.

Reproduction I have put together a gist https://gist.github.com/marlon-tucker/2eddb354ca381091a6692164100a8abe

Expected behavior This is tricky really, as architecturally this is probably valid behaviour because a sink should not be producing logs itself.

However, Serilog has the ability to resolve sinks from the service provider and when this issue is encountered, it is very difficult to diagnose and work out what's going on. In my case the ILogger dependency wasn't something I myself had introduced, and given how common it is to have a ILogger dependency buried deep in the dependency graph I think this needs to be investigated to see if a better solution can be designed.

Would it be possible for Serilog to register a singleton ILogger implementation which forwards calls to either the Bootstrap Logger or the Logger created by the factory method, or would this lead to worse performance?

Relevant package, tooling and runtime versions NET6 Serilog.AspNetCore 5.0.0

nblumhardt commented 2 years ago

Hi @marlon-tucker - thanks for dropping us a line. Definitely seems like a better diagnostic outcome should be possible, but I don't know if Serilog is in the right place to provide it (Autofac detects and reports on this situation, IIRC - maybe a tweak to MEDI could do the same?)

nblumhardt commented 2 years ago

Closing as outside the current scope of this project; thanks for the heads-up all the same!