Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Liblog is deprecated #991

Closed
ChrisMcKee opened this issue Dec 19, 2019 · 7 comments · Fixed by #1300
Closed

Liblog is deprecated #991

ChrisMcKee opened this issue Dec 19, 2019 · 7 comments · Fixed by #1300

Comments

@ChrisMcKee
Copy link

https://github.com/damianh/LibLog
"⚠️ LibLog is now deprecated (see damianh/LibLog#270).

My recommend course of action for library developers is to use Microsoft.Extensions.Logging.Abstractions as it is now the defacto standard logging interface in the .NET ecosystem. For library developers that wish to continue to use LibLog or need to make adjustments for their needs should copy the code into their project (after all, that's just what LibLog did).

Thanks to all contributors and users for the support over the years.

Damian"

@sungam3r
Copy link
Contributor

Interesting news. Honestly, I am glad about this, because the way to connect the LibLog caused certain difficulties for me. In fact, the LibLog acts as a ServicLocator. I prefer to serve all dependencies with DI tools.

My recommend course of action for library developers is to use Microsoft.Extensions.Logging.Abstractions

Please do not. You should not include this dependency in a package. It will be much more convenient to have your own stable logging abstraction. Next, you just need to write a few small adapter packages: NLog, Serilog, MEL, whatever.

I will explain my case. I develop libraries that form a specific technology stack. Logging is one of the functions of this stack, which is present on many layers. I did not want to be attached to any one logging application library, even MEL. The choice of a specific logging implementation occurs at the Composition Root level of application.

Although MEL is positioned as an abstraction over all api-logging, which should satisfy everyone, in reality this is not so. And indirectly this is evidenced even by the fact that MS did not dare to place MEL in System.*-like package/namespace.

Logging abstraction example with a comprehensive set of methods:

public interface ILogger
    {
        void Debug(string message);
        void Debug(string message, params object[] propertyValues);
        void Debug(Exception exception);
        void Debug(Exception exception, string message);
        void Debug(Exception exception, string message, params object[] propertyValues);
        void Error(string message);
        void Error(string message, params object[] propertyValues);
        void Error(Exception exception);
        void Error(Exception exception, string message);
        void Error(Exception exception, string message, params object[] propertyValues);
        void Info(string message);
        void Info(string message, params object[] propertyValues);
        void Info(Exception exception);
        void Info(Exception exception, string message);
        void Info(Exception exception, string message, params object[] propertyValues);
        void Fatal(string message);
        void Fatal(string message, params object[] propertyValues);
        void Fatal(Exception exception);
        void Fatal(Exception exception, string message);
        void Fatal(Exception exception, string message, params object[] propertyValues);
        void Warn(string message);
        void Warn(string message, params object[] propertyValues);
        void Warn(Exception exception);
        void Warn(Exception exception, string message);
        void Warn(Exception exception, string message, params object[] propertyValues);
        void Trace(string message);
        void Trace(string message, params object[] propertyValues);
        void Trace(Func<string> messageFunc);
    }

    public interface ILogger<out TCategory> : ILogger
    {
    }

It is worth noting that the caller is responsible for connecting one or another implementation in DI, although by default you can use something like ConsoleLogger or EmptyLogger.

@micdenny
Copy link
Member

Please do not. You should not include this dependency in a package. It will be much more convenient to have your own stable logging abstraction. Next, you just need to write a few small adapter packages: NLog, Serilog, MEL, whatever.

In fact Liblog was chosen to have our abstraction, liblog is a non-dependency, is a file, and we can manage it, so nowadays there's no rush to change this, I liked the way liblog works because if you already have a logging system in place it uses that without doing anything, so the fact that is a service locator it's true if you need to directly access liblog infrastructure, otherwise you have to write zero line of code if you already use one of the auto-discovered logging provider (log4net, serilog, ....), and you can use any other abstraction you want in your app and liblog. So basically to me the idea behind liblog is not so bad, true that when you want to push a custom implementation it's more difficult and a simple interface it's much more easy.

@zidad
Copy link
Member

zidad commented Dec 19, 2019

Indeed, I don't see a big difference between liblog or creating your own abstraction.

@sungam3r what problems are you experiencing with liblog?

@sungam3r
Copy link
Contributor

Ok. I want to use my own ILogProvider. I should call LogProvider.SetCurrentLogProvider(ILogProvider logProvider); It is static method. At what point should I call this method? On the one hand, the sooner the better. On the other hand, I cannot call it until then, until an ILogProvider instance is created. The problem is that the implementation of my ILogProvider is created through DI - it needs its own dependencies. I have to choose some class and pass the necessary dependency to it just to use it when calling the static method. In addition, I have to defend myself against double registration, since otherwise a static object registered in this way will begin to conflict with registrations from other DI containers:

// DI-friendly class
 internal sealed class MessageTypeNameSerializer : ITypeNameSerializer
    {
        public MessageTypeNameSerializer(LoggingContext context)
        {
            EasyNetQLogging.Initialize(context); // HACK!
        }
}

 public static class EasyNetQLogging
    {
        private static bool _alreadyBinded;

        public static void Initialize(LoggingContext context)
        {
            if (!_alreadyBinded)
            {
                _alreadyBinded = true;
                LogProvider.SetCurrentLogProvider(new ShimToLibLogProvider(context)); // NOT DI-friendly API
            }
        }
    }

I tried to explain that the current way of registering a provider goes against the principle of dependency injection.

@sungam3r
Copy link
Contributor

Indeed, I don't see a big difference between liblog or creating your own abstraction.

The difference is DI-friendly vs not DI-friendly API (static API). LibLog has ILogProvider interface, ok, good. But it’s not enough just to have it, it still needs to be installed inside LibLog.

@micdenny
Copy link
Member

micdenny commented Dec 20, 2019

The difference is DI-friendly vs not DI-friendly API (static API). LibLog has ILogProvider interface, ok, good. But it’s not enough just to have it, it still needs to be installed inside LibLog.

Because liblog is meant to work with the most known log libraries, not to give you another abstraction, in other words you can use known abstractions like ILogging from net core, and if it's configured to work with log4net, serilog, ... easynetq can use the same provider with zero configuration. In the same way you can use your own abstraction in your application, and if that abstraction then use one of the liblog supported providers to log, you're ready to go. If you have your own logging implementation then yes I admit liblog is not so gentle/clean/modern.

We can consider to have both world, an injected interface in easynetq, that by default is populated with a factory that uses liblog service locator LogProvider.GetLogger() something like this:

public interface ILogger
{
	bool Log(LogLevel logLevel, Func<string> messageFunc, Exception exception = null, params object[] formatParameters);
}

public interface ILogger<out TCategoryName> : ILogger
{
}

public class DefaultLogger : ILogger
{
	private readonly ILog logger = LogProvider.GetLogger(null);

	public bool Log(LogLevel logLevel, Func<string> messageFunc, Exception exception = null, params object[] formatParameters)
	{
		return logger.Log(logLevel, messageFunc, exception, formatParameters);
	}
}

public class DefaultLogger<TCategoryName> : ILogger<TCategoryName>
{
	private readonly ILog logger = LogProvider.GetLogger(typeof(TCategoryName));

	public bool Log(LogLevel logLevel, Func<string> messageFunc, Exception exception = null, params object[] formatParameters)
	{
		return logger.Log(logLevel, messageFunc, exception, formatParameters);
	}
}

To do the categorized logger we also need to extend the IServiceRegister:

/// <summary>
/// Registers the <paramref name="serviceType"/> with the <paramref name="implementingType"/>.
/// Note that the first registration wins. All subsequent registrations will be ignored.
/// </summary>
/// <param name="serviceType">The service type to register.</param>
/// <param name="implementingType">The implementing type.</param>
/// <returns>itself for nice fluent composition</returns>
public IServiceRegister Register(Type serviceType, Type implementingType, Lifetime lifetime = Lifetime.Singleton);

Then we can register a couple of logger interface:

// default service registration
container
	.Register<ILogger, DefaultLogger>()
	.Register(typeof(ILogger<>), typeof(DefaultLogger<>))

An extension then will do the rest LoggerExtensions:

using System;

namespace EasyNetQ.Logging
{
    static class LoggerExtensions
    {
        internal static readonly object[] EmptyParams = new object[0];

        public static bool IsDebugEnabled(this ILogger logger)
        {
            GuardAgainstNullLogger(logger);
            return logger.Log(LogLevel.Debug, null, null, EmptyParams);
        }

        // ...
        
		public static void Debug(this ILogger logger, Func<string> messageFunc)
        {
            GuardAgainstNullLogger(logger);
            logger.Log(LogLevel.Debug, WrapLogInternal(messageFunc), null, EmptyParams);
        }

        public static void Debug(this ILogger logger, string message)
        {
            if (logger.IsDebugEnabled()) logger.Log(LogLevel.Debug, message.AsFunc(), null, EmptyParams);
        }

        public static void Debug(this ILogger logger, string message, params object[] args)
        {
            logger.DebugFormat(message, args);
        }

        public static void Debug(this ILogger logger, string message, object arg0)
        {
            if (logger.IsDebugEnabled()) logger.DebugFormat(message, arg0);
        }

        // ...
    }
}

I've made a branch with a proof of concept (#992) to let you @sungam3r tell us if that would be good for you, and then probably will be good for us as well cause it will not break anything on the main easynetq interface, of course it will break the IServiceRegister if someone has did some other implementation of it, I think with not so effort can be done this with few or zero breaking change with the actual logging mechanism.

@sungam3r
Copy link
Contributor

I am OK with #992

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants