CacheManager icon indicating copy to clipboard operation
CacheManager copied to clipboard

No instance of ILoggerFactory found in serviceCollection.

Open snowchenlei opened this issue 6 years ago • 10 comments

run AspnetCore.WebApp,page error:No instance of ILoggerFactory found in serviceCollection. change services.AddCacheManagerConfiguration(Configuration, cfg => cfg.WithMicrosoftLogging(services)); to services.AddCacheManagerConfiguration(Configuration); is ok.

snowchenlei avatar Apr 11 '19 02:04 snowchenlei

ok

MichaCo avatar Apr 11 '19 18:04 MichaCo

I've had a developer also report this as we migrate apps from .Net Framework 4.x and using a homegrown log4net adapter, to .Net Core 2.x and trying to use WithMicrosoftLogging(). It seems that during IServiceCollection setup, WithMicrosoftLogging() is trying to get the ILoggerFactory to create an ILogger, but I'm speculating that it's too early to do so. I'm trying to reproduce in a simple sample, I'll share that if and when I can.

erizzo avatar Nov 06 '19 23:11 erizzo

The implementation of attaching the Microsoft logger factory to logging from CacheManager is a bit flaky for historical reasons.

I'm not really sure yet how to change that in a good way but I'll have a look into that again

MichaCo avatar Nov 07 '19 11:11 MichaCo

With Microsoft.Extensions.Logging it seems like CacheManager having its own logging abstraction is a bit redundant; the point of ILogger and ILoggerFactory is to allow apps to chose their underlying logging impl without coupling code to any one. Would you consider for CacheManager 2.0 eliminating the abstraction and just using ILogger directly?

erizzo avatar Nov 07 '19 15:11 erizzo

Not really, I don't want to have a hard dependency on any external library in the base code because that would move that dependency up the chain to everything and everyone using CM.

I think the wrapper for the logging calls from my internal loggers to the Microsoft logger is totally fine, its very simple and doesn't do much. The problem is the setup part, connecting the 2 parts together through DI is the part which isn't clean right now and can be improved.

MichaCo avatar Nov 07 '19 17:11 MichaCo

I understand, but I'd say that most .Net Core apps are likely already using Mocrosoft.Extensions.ILogger anyway. Think about it this way: what will happen if every library built its own logging abstraction like CM does? Any app of significant size would now have to think about dozens of adapters/bridges. As opposed to everyone just standardizing on the single Microsoft-endorsed abstraction.

erizzo avatar Nov 07 '19 18:11 erizzo

In either case, if the setup portion gets fixed, I request that it be back-ported into a 1.x release. My client and our project can not tolerate the major upgrade to CM any time soon but we definitely need .Net Core compatible logging through ILogger. Thanks.

erizzo avatar Nov 07 '19 18:11 erizzo

Here's a strange result. I set up a basic Web API test project, in which I call WithMicrosoftLogging() from Startup.ConfigureServices(IServiceCollection) When I step into the CM code of WithMicrosoftLogging() I end up at the point in the screenshot. What's odd is that the IServiceCollection does indeed include a descriptor for Microsoft.Extensions.Logging.ILoggerFactory, but the CM code doesn't find it. Untitled 14 Untitled 14

erizzo avatar Nov 08 '19 21:11 erizzo

This seems to work fine in a .Net Core app (tested in 2.1 and 2.2): ILoggerFactory loggerFactory = services.BuildServiceProvider().GetRequiredService<ILoggerFactory>(); ICacheManagerConfiguration config = new ConfigurationBuilder("Foo") .WithMicrosoftLogging(loggerFactory);

erizzo avatar Dec 02 '19 23:12 erizzo