Add Support for Microsoft.Extensions.Logging

[SkyeHoefling]

Description of Problem

Logging in DNN is using log4net and I believe we are using a DNN specific fork of it. This works and I don't intend to change how this works at all for the .NET Framework version of DNN. As we get closer to .NET Core migrations, we need to consider how to decouple the platform from any hard dependencies on .NET Framework to make the transition easier.

DNN currently has an ILog interface that exists in the DotNetNuke.Instrumentation library. This is the interface that is used for logging throughout the platform and ecosystem.

public interface ILog
{
    bool IsDebugEnabled { get; }
    bool IsInfoEnabled { get; }
    bool IsTraceEnabled { get; }
    bool IsWarnEnabled { get; } 
    bool IsErrorEnabled { get; }
    bool IsFatalEnabled { get; }
        
    void Debug(object message);
    void Debug(object message, Exception exception);
    void DebugFormat(string format, params object[] args);
    void DebugFormat(IFormatProvider provider, string format, params object[] args);

    void Info(object message);
    void Info(object message, Exception exception);
    void InfoFormat(string format, params object[] args);
    void InfoFormat(IFormatProvider provider, string format, params object[] args);

    void Trace(object message);
    void Trace(object message, Exception exception);
    void TraceFormat(string format, params object[] args);
    void TraceFormat(IFormatProvider provider, string format, params object[] args);

    void Warn(object message);
    void Warn(object message, Exception exception);
    void WarnFormat(string format, params object[] args);
    void WarnFormat(IFormatProvider provider, string format, params object[] args);

    void Error(object message);
    void Error(object message, Exception exception);
    void ErrorFormat(string format, params object[] args);
    void ErrorFormat(IFormatProvider provider, string format, params object[] args);

    void Fatal(object message);
    void Fatal(object message, Exception exception);
    void FatalFormat(string format, params object[] args);
    void FatalFormat(IFormatProvider provider, string format, params object[] args);
}

This topic was discussed in some capacity in the following item:

• RFC: DNN & Log4Net #2801 - The discuss was more focused on upgrading log4net

Proposed Solution Option 1

I propose we refactor our current implementation of the DotNetNuke.Instrumentation.ILog interface and our log4net implementation to use Microsoft.Extensions.Logging and deprecate our custom interface and current usage for v11. This will allow us to register the Microsoft.Extensions.Logging interface ILogger with the Dependency Injection Container and then anyone can resolve it anywhere in the platform or extensions.

As with a breaking change like this, the current usage of LoggerSource.Instance.GetLogger() should return the registered instance of ILogger but this creates more complexity to prevent any type of break since DotNetNuke.Instrumentation.ILog != Microsoft.Extensions.Logging.ILogger

How Will This Work?

I'm glad you asked!

1. Deprecate all usages of DotNetNuke.Instrumentation.ILog for removal in v11.0
2. Deprecate all methods in the LoggerSource that returns ILog for removal in v11. This will make it clear that we can still use this for building our ILogger but the DNN ILog interface is going to be removed.
3. Create an implementation of the current logger that implements Microsoft.Extensions.Logging.ILogger. This means our current logging implementation will not change
4. Update all usages of the DotNetNuke.Instrumentation.ILog in the platform to use the Microsoft.Extensions.Logging.ILogger via Dependency Injection and where not available allow it ot use the Service Locator pattern.

Should DotNetNuke.Instrumentation be .NET Standard?

I would really love for this project to become .NET Standard 2.0 compliant but I believe that will be a follow-up issue if we proceed with this proposal. The DotNetNuke.Instrumentation library has hard dependencies on System.Web that is used for logging some of the exceptions. After an analysis of some of the code we may be able to remove it, but I'm not 100% certain at this time.

In the below code snippet, the BuildManager object is a System.Web dependency. This is just 1 example of the System.Web dependency in the DotNetNuke.Instrumentation library

Dnn.Platform/DNN Platform/DotNetNuke.Instrumentation/DnnLog.cs

Lines 60 to 64 in e03f3b0

	while (reflectedType == BuildManager.GetType("DotNetNuke.Services.Exceptions.Exceptions", false) || reflectedType == typeof(DnnLogger) || reflectedType == typeof(DnnLog))
	{
	frameDepth++;
	reflectedType = stack[frameDepth].GetMethod().ReflectedType;
	}

How to Register ILogger

This will require new registration code in the DotNetNuke.Instrumentation project which will be using the new IDnnStartup interface. The LoggerSource or similar object will be responsible for creating a Microsoft.Extensions.Logging.ILogger Implementation and registering it to the container. At that point any part of the platform or extensions can resolve it using Dependency Injection

The new ILogger Interface

The ILogger is a much smaller interface than our current ILog but the same rules can still be applied.

https://github.com/aspnet/Extensions/blob/66007df6e5b68683de0cab0dc6a8552265318f20/src/Logging/Logging.Abstractions/src/ILogger.cs#L8-L39

copied from link above

/// <summary>
/// Represents a type used to perform logging.
/// </summary>
/// <remarks>Aggregates most logging patterns to a single method.</remarks>
public interface ILogger
{
	/// <summary>
	/// Writes a log entry.
	/// </summary>
	/// <param name="logLevel">Entry will be written on this level.</param>
	/// <param name="eventId">Id of the event.</param>
	/// <param name="state">The entry to be written. Can be also an object.</param>
	/// <param name="exception">The exception related to this entry.</param>
	/// <param name="formatter">Function to create a <see cref="string"/> message of the <paramref name="state"/> and <paramref name="exception"/>.</param>
	/// <typeparam name="TState">The type of the object to be written.</typeparam>
	void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter);

	/// <summary>
	/// Checks if the given <paramref name="logLevel"/> is enabled.
	/// </summary>
	/// <param name="logLevel">Level to be checked.</param>
	/// <returns><c>true</c> if enabled.</returns>
	bool IsEnabled(LogLevel logLevel);

	/// <summary>
	/// Begins a logical operation scope.
	/// </summary>
	/// <param name="state">The identifier for the scope.</param>
	/// <typeparam name="TState">The type of the state to begin scope for.</typeparam>
	/// <returns>An <see cref="IDisposable"/> that ends the logical operation scope on dispose.</returns>
	IDisposable BeginScope<TState>(TState state);
}

Deprecated ILog Implementation

As we migrate away from the ILog implementation we can configure it to use the new ILogger in the implementation details. This will create a clear migration strategy of what methods map to the new interface. As well this will create 1 code path for logging and limit logging side-effects between having 2 different implementations

Alternatives Researched

N/A

Affected version

I propose we target this change for 9.x so we can properly deprecate the ILog interface for v11.0

• 10.0.0 nightly build
• 9.4.1 nightly build
• 9.4.0 latest supported release

[donker]

Don't take the following as wanting to tack something onto this (i.e. scope creep): I second this and I'd love to see this move "closer" to event logging. I.e. similar implementation. As a dev I find it confusing that we have the two (and currently completely different technical solutions). So the "front facing part" that developers see would be near each other and the "back end" decoupled as suggested here. Thoughts?

[valadas]

Hmm, I am not sure I understand but I think it is useful to have a level of logging that admins can view and some other level that is not visible by admins and is more reserved to only people with hosting access... If that was your meaning...