Prism: Replace ILoggerFacade with Microsoft.Extensions.Logging.ILogger

[dansiegel]

Summary

The ILoggerFacade besides just having a terrible name can be somewhat problematic since it forces Prism to do a ToString on any caught exceptions that may be logged. Having the actual Exception object can be very powerful particularly when debugging.

Proposed Changes

We should first make ILoggerFacade obsolete. We will remove any internal references to it, but will keep it in the Core so as to not break anyone who may currently be using it. This will better give people an opportunity to migrate without a failed build.

There is still some benefit to the existing API as it does provide some ability to create a logger that filters based on Priority or Category. To this end we would keep a similar but updated API.

public interface ILogger
{
    void Log(string message, Category category, Priority priority);
    void Log(string message, Exception ex, Category category, Priority priority);
}

Additional extensions could be included here to make this easier like:

public static void Log(this ILogger logger, string message, Priority priority = Priority.None) =>
    logger.Log(message, Category.Info, priority);

public static void Debug(this ILogger logger, string message, Priority priority = Priority.None) =>
    logger.Log(message, Category.Debug, priority);

public static void Warn(this ILogger logger, string message, Priority priority = Priority.None) =>
    logger.Log(message, Category.Warn, priority);

Additional Notes

No implementations will remain for ILoggerFacade in Prism, and thus it will not be registered out of the box as we will no longer be using it internally and only keeping the obsolete interface for those who may be using it with their own logger currently.

We will be removing the File Logger in Prism.WPF, and move the TraceLogger to Prism.Core. The DebugLogger will be replaced with a simple Console Logger.

[hnabbasi]

This makes sense. I can help with this.

[bartlannoeye]

Any reason to not go for Microsoft.Extensions.Logging.ILogger if we redo logging? Then the user can chose to use Debug or Console logger and multiple sinks like Serilog available out of the box without the need to write custom code to link the Prism logger to a known logging framework.

[dansiegel]

this is not a bad point

[brianlagunas]

I think this is a good idea

[dansiegel]

after further discussion... new plan... we really like @bartlannoeye's idea... we'll go ahead and take the dependency on Microsoft.Extensions.Logging, this creates a pretty good win for everyone as it means we'll be using a standardized Logging interface that there are already a ton of logging implementations out there for...

We will be simply removing the entire Prism.Logging namespace. Everything in there can come out @hnabbasi

[hnabbasi]

Agreed.

[brianlagunas]

There has been more discussions around this and this would required taking dependencies on MS Dependency Injection, which we do not want to do. We are going to take it slow with this issue, and find an approach that works best before making a final decision.

[hnabbasi]

Yup. The library takes in these dependencies,

Microsoft.Extensions.Configuration.Binder 
Microsoft.Extensions.DependencyInjection 
Microsoft.Extensions.Logging.Abstractions 
Microsoft.Extensions.Options 

[hnabbasi]

Perhaps we can continue with initial plan to replace ILoggerFacade with ILogger and table Microsoft.Extensions.Logging for now ?

[brianlagunas]

I'm questioning if Prism needs a logger at all

[d3fkn1ght]

Would it make sense to have a std logging interface so ppl can plug in different logging frameworks similar to how Prism does it with DI containers?

[brianlagunas]

@d3fkn1ght Prism already has this abstraction with ILoggerFacade, which is what this issue is discussing removing.

[Jesus805]

I may be in the minority, but I love that Prism has a logger right out the box. I use it for debug tracing and it's one less library to install and configure. Although I agree that ILoggerFacade is a bizzare name.