NLog / NLog.Web

NLog integration for ASP.NET & ASP.NET Core 2-8
https://nlog-project.org
BSD 3-Clause "New" or "Revised" License
320 stars 166 forks source link

Using NLog with ASP.NET Core 2 and a Different DI Library #227

Closed richardcox13 closed 6 years ago

richardcox13 commented 6 years ago

Summary: ASP.NET Core allows the use of a different DI Library. But NLog's integration (here) assumes use of the inbuilt DI.

I am using Simple Injector (need to reuse other assemblies that already depend on Simple Injector and use NLog), but the UseNLog extension methods depends on the internals of NLog.Web to integrate (eg. NLog.Web.Internal.ServiceLocator)

How can I access these internals to build an equivalent when working with a different DI Library?

304NotModified commented 6 years ago

Good question. I think the relevant parts are public, so please check the body of useNlog.

I will check it also, but will be in the beginning of next week.

richardcox13 commented 6 years ago

ServiceLocator is internal for a start. I expect the layout renders in NLog.Web won't work without this.

304NotModified commented 6 years ago

Well you need to inject the IHttpContextAccessor into every AspNetLayoutRenderer (see AspNetLayoutRendererBase).

Isn't the ServiceLocator already MS DI dependent? I could make it public, but I think that won't help much

richardcox13 commented 6 years ago

Other DI containers implement IServiceProvider (this hasn't changed from pre-Core1).

So plan is to initially ignored that part and set up an ILoggerProvider so controllers using ILogger<T> work.


1 I ran into one page on docs.microsoft.com which linked IServiceProvider back to the MVC5 documentation on msdn.microsoft.com …

richardcox13 commented 6 years ago

This, other than dependencies within NLog.Web on IServiceLocator this seems to work. Specifically controllers dependent on ILogger<TCategoryName> don't cause an error.

Testing continues...

(So many interfaces and types matching /.*Logger.*/ doesn't help working through this :-) .)

As noted above I'm using SimpleInjector and diContainer is an instance of SimpleInjector.Container.

Called from Startup.Configure:

ConfigurationItemFactory.Default.RegisterItemsFromAssembly(typeof(NLog.Web.AspNetExtensions).GetType().Assembly);
 LogManager.AddHiddenAssembly(typeof(NLog.Web.NLogBuilder).Assembly);
var nLogOpts = NLog.Web.NLogAspNetCoreOptions.Default;
diContainer.Register<ILoggerFactory>(() => new NLogLoggerFactory(nLogOpts), Lifestyle.Singleton);
diContainer.Register(typeof(ILogger<>), typeof(GenericLogger<>), Lifestyle.Scoped);

With a simple forwarding helper (resolving Microsoft.Extensions.Logging.ILogger was easy, but the generic version took several vastly more complex tries before seeing this approach):

internal class GenericLogger<T> : ILogger<T>, Microsoft.Extensions.Logging.ILogger {

    private ILogger<T> underlying;

    public GenericLogger(ILoggerFactory factory) {
        underlying = factory.CreateLogger<T>();
    }

    public IDisposable BeginScope<TState>(TState state) => underlying.BeginScope<TState>(state);

    public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => underlying.IsEnabled(logLevel);

    public void Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) 
            => underlying.Log<TState>(logLevel, eventId, state, exception, formatter);
}
304NotModified commented 6 years ago

But is this issue solved if ServiceLocator is made public?

richardcox13 commented 6 years ago

I expect so (still learning to get all the bits I want to play together nicely... ). A public helper extension over the applicable interface might be a little cleaner.

Thanks for the pointers.

304NotModified commented 6 years ago

maybe had some time to check this?

made a PR: https://github.com/NLog/NLog.Web/pull/231