Open snowchenlei opened 5 years ago
ok
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.
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
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?
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.
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.
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.
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.
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);
Bringing this old issue back up but if there was an overload that provides IServiceProvider
that would work better. Example
/// <summary>
/// Overload that provides <see cref="IServiceProvider"/> to <paramref name="configure"/> delegate
/// </summary>
/// <param name="collection">The services collection.</param>
/// <param name="fromConfiguration">The <see cref="IConfiguration"/> section which contains a <c>cacheManagers</c> section.</param>
/// <param name="configure">Can be used to further configure the configuration.</param>
/// <returns>The services collection</returns>
public static IServiceCollection AddCacheManagerConfiguration(
this IServiceCollection services,
IConfiguration configuration,
Action<ConfigurationBuilder, IServiceProvider> configure = null)
{
if (services is null)
{
throw new ArgumentNullException(nameof(services));
}
if (configuration is null)
{
throw new ArgumentNullException(nameof(configuration));
}
return services.AddSingleton(sp =>
{
var builder = configuration.GetCacheConfiguration().Builder;
configure?.Invoke(builder, sp);
var result = builder.Build();
return result;
});
}
Usage
// Use our own method that will provide IServiceProvider to the configure delegate so we can properly access ILoggerFactory
services.AddCacheManagerConfiguration(configuration, (cfg, sp) =>
{
cfg.WithMicrosoftLogging(sp.GetRequiredService<ILoggerFactory>());
});
run AspnetCore.WebApp,page error:No instance of ILoggerFactory found in serviceCollection. change
services.AddCacheManagerConfiguration(Configuration, cfg => cfg.WithMicrosoftLogging(services));
toservices.AddCacheManagerConfiguration(Configuration);
is ok.