andrewlock / NetEscapades.Extensions.Logging

A rolling file logging provider for ASP.NET Core 2.0
MIT License
95 stars 33 forks source link

More generalization of BatchingLogger #20

Open daveaglick opened 5 years ago

daveaglick commented 5 years ago

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:

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; }
}
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));
}

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?

andrewlock commented 5 years ago

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 😉