Feature: Add ILogger
Description
This pull request adds support for the ILogger interface from the Microsoft.Extensions.Logging package. It replaces LogManager and the other components with ILogger, ILoggerFactory, and ILoggerProvider.
Changes
I have deleted Logger, LogManager, LogMessage and LogSeverity from Discord.Net.Core and instead replaced them with a DefaultLogger, DefaultLoggerFactory and DefaultLoggerProvider. Also, I have added Microsoft.Extensions.Logging.Abstractions v5.0.0 to the Discord.Net.Core package. Finally, I have changed all the Logger implementation to ILogger so the code has changed like this
--- await _gatewayLogger.DebugAsync("...").ConfigureAwait(false);
+++ _gatewayLogger.LogDebug("...");
Motivation
The ILogger implementation has more flexibility and allows the use of different logging provider (Serilog, Nlog) and just pass a LoggerFactory instead of having to use a Log event
Breaking changes
I have removed the LogLevel Property in DiscordConfig not sure if I should have instead kept the property and just add the Obsolete attribute
I have removed the Log event property from the client again not sure if I should have instead kept the property and just add the Obsolete attribute
The CommandService is affected as well, I have removed the LogLevel property and added LoggerFactory not sure if I should have instead kept the property and just add the Obsolete attribute
Now to get logs from the DiscordSocketClient you can do something like this
var client = _client = new DiscordShardedClient(new DiscordSocketConfig
{
--- LogLevel = LogSeverity.Info,
+++ LoggerFactory = yourLoggerFactory
});
--- client.Log += (msg) => ...
I don't think this would benefit a lot of people, if you really needed to you could just proxy the discord log event to your own implementation of ILogger. I've never had issues with using a log event or needed to implement my own type of logs so it could just be my own bias here.
The main issues I see is everyone coming from 2.x to 3.x and there old code breaking because of logging changes, I would try to avoid this as much as possible
Personally, I think this pull would be useful for the next version of DNet as ILogger<> is used in a wide range of other libraries already, and would make it easier to integrate as you wouldn't need to write your own conversion methods between the two types. As this class is what is preferred by many in the C# community, broadly, to use to log, and the differences between the DNet and Microsoft loggers are minute, the only major issue with this is the breaking changes that would be caused, a sacrifice that would be a (relatively?) easy fix for many developers.
As far as I know, DSharp also uses ILogger, as there's no particular need for a custom implementation for a logger on either libraries, while this class exists. For my own use case, before migrating to DNet, this made integration with ASP.Net, which a lot of particularly larger bots use for creating RESTful APIs, very simple and straight forward as they used the same logging class (ILogger).
Just my two cents though! I completely agree that breaking changes should be avoided where possible, though it makes more sense to be using the default logger interface that Microsoft provides for this better integration than a custom one that does essentially the same thing. Using ILogger would be more versatile than the current logger that DNet supports.
It is unfortunate that ILogger is not supported by the default System libraries like IServiceProvider. It would be elegant and easy if we did not need to depend on an exterior service (Microsoft.Extensions.Logging) to add the feature. For me personally this is enough reason not to implement it, and keep the library self dependent for the most part.