ILogger<T> was implemented in such a way that it isn't being cached (for the same type) unlike non generic version #48473
Description
When using ILogger<T> in class dependency, and creating multiple instances of the same type, a new ILogger<T> instance is created for each instance instead of logger being cached and reused like in non generic ILogger
To Reproduce
This unit test can explain the issue:
//loggerFactory is of type LoggerFactory
var loggerFactory = container.Resolve<ILoggerFactory>();
//Non generic ILogger is being cached
var logger1 = loggerFactory.CreateLogger(typeof(MyClass).FullName);
var logger2 = loggerFactory.CreateLogger(typeof(MyClass).FullName);
Assert.AreEqual(logger1, logger2);
//Non generic ILogger is being cached when called from extension method
logger1 = loggerFactory.CreateLogger(typeof(MyClass));
logger2 = loggerFactory.CreateLogger(typeof(MyClass));
Assert.AreEqual(logger1, logger2);
//BUT - here is the problem
var genericLog1 = loggerFactory.CreateLogger<MyClass>();
var genericLog2 = loggerFactory.CreateLogger<MyClass>();
//This assert is failing...
Assert.AreEqual(genericLog1, genericLog2);Expected behavior
When calling loggerFactory.CreateLogger() multiple times with the same type - it should use the same ILogger instance
Exactly like how it is working in CreateLogger(type) or CreateLogger(string)
Additional context
From the source code of LoggerFactoryExtensions, it seems that the problem is that when calling CreateLogger(this ILoggerFactory factory) - it always created new instance -
/// <summary>
/// ILoggerFactory extension methods for common scenarios.
/// </summary>
public static class LoggerFactoryExtensions
{
/// <summary>
/// Creates a new <see cref="ILogger"/> instance using the full name of the given type.
/// </summary>
/// <param name="factory">The factory.</param>
/// <typeparam name="T">The type.</typeparam>
/// <returns>The <see cref="ILogger"/> that was created.</returns>
public static ILogger<T> CreateLogger<T>(this ILoggerFactory factory)
{
if (factory == null)
{
throw new ArgumentNullException(nameof(factory));
}
return new Logger<T>(factory);
}Unlike in the other method CreateLogger(type) that it redirect the responsibility to the factory
and the factory is caching all created logger
/// <summary>
/// Creates a new <see cref="ILogger"/> instance using the full name of the given <paramref name="type"/>.
/// </summary>
/// <param name="factory">The factory.</param>
/// <param name="type">The type.</param>
/// <return>The <see cref="ILogger"/> that was created.</return>
public static ILogger CreateLogger(this ILoggerFactory factory, Type type)
{
if (factory == null)
{
throw new ArgumentNullException(nameof(factory));
}
if (type == null)
{
throw new ArgumentNullException(nameof(type));
}
return factory.CreateLogger(TypeNameHelper.GetTypeDisplayName(type, includeGenericParameters: false, nestedTypeDelimiter: '.'));
}Code from LoggerFactory:
public ILogger CreateLogger(string categoryName)
{
if (CheckDisposed())
{
throw new ObjectDisposedException(nameof(LoggerFactory));
}
lock (_sync)
{
if (!_loggers.TryGetValue(categoryName, out var logger))
{
logger = new Logger(this)
{
Loggers = CreateLoggers(categoryName)
};
_loggers[categoryName] = logger;
}
return logger;
}
}