NetEscapades.Extensions.Logging icon indicating copy to clipboard operation
NetEscapades.Extensions.Logging copied to clipboard

More generalization of BatchingLogger

Open daveaglick opened this issue 6 years ago • 1 comments

I love the general intent behind BatchingLogger and BatchingLoggerProvider but they suffer from some of the same problems as the stock ConsoleLogger in that the actual message is created within the BatchingLogger and can't be customized. I'd love to use the classes as the basis for totally customized logging (I.e., my own console logger). My proposal:

  • Change LogMessage to the following:
internal class LogMessage
{
    public LogMessage(
        DateTimeOffset timestamp,
        LogLevel logLevel,
        EventId eventId,
        string formattedMessage,
        Exception exception)
    {
        Timestamp = timestamp;
        LogLevel = logLevel;
        EventId = eventId;
        FormattedMessage = formattedMessage;
        Exception = exception;
    }

    public DateTimeOffset Timestamp { get; }
    public LogLevel LogLevel { get; }
    public EventId EventId { get; }
    public string FormattedMessage { get; }
    public Exception Exception { get; }
}
  • Change the Log method in BatchingLogger to:
public void Log<TState>(DateTimeOffset timestamp, LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
{
    if (!IsEnabled(logLevel))
    {
        return;
    }

    _provider.AddMessage(new FlexibleLogMessage(timestamp, logLevel, eventId, formatter(state, exception), exception));
}
  • Defer the rendering of a log message to the WriteMessagesAsync method in each BatchingLoggerProvider implementation.

The idea is that the FileLoggerProvider would then generate the message string inside WriteMessagesAsync the way the BatchingLogger used to using the new properties in LogMessage. Any new implementation could render the message string however they want to since the LogMessage now contains all the information like log level.

What do you think?

daveaglick avatar Sep 20 '19 00:09 daveaglick

On the one hand I absolutely agree with you, but I'm a bit conflicted.

The implementation of BatchingLogger is pretty much a direct copy from the implementation in the aspnet/Extensions repo. That was intentional, as I can just pull in any tweaks from there into this library - it may not be the best implementation but it does the job. This whole library was really just a proof of concept - I generally recommend people use Serilog for file logging instead of this library!

That said, it is being used, and so probably should be improved when there's obvious deficiencies in it like this.

That's a long winded way of me saying yes, I like it, thanks 😉

andrewlock avatar Sep 22 '19 21:09 andrewlock