serilog / serilog

Simple .NET logging with fully-structured events
https://serilog.net
Apache License 2.0
7.31k stars 797 forks source link

Support ILogger<T> #2025

Open AleyCZ opened 8 months ago

AleyCZ commented 8 months ago

We use Serilog for many years. In older projects we used it directly, creating a logger in each constructor by calling Log.ForContext<MyClass>(). In newer projects we tried to use Microsoft logging abstractions which is at the end very similar, but it has one interesting feature which is either missing in Serilog or we don't know about it. So I would like to ask you to implement it natively in Serilog.

Microsoft logging abstractions contain generic interface ILogger<T> so we can add a constructor dependency to ILogger<MyClass> and it creates a new logger each time constructor is called. (I don't know how is it implemented, because it is neither singleton, nor transient, nor per-request, but it seems to work as expected. Our applications are ASP.NET services. We defined a factory method and registered it in Microsoft logging infrastructure, so it calls Log.ForContext<T> behind the curtain.)

As a workaround, we can still manually call the above-mentioned Log.ForContext<ClassName>() in each constructor, but some team members complain about this being a not-clean and repeated code since this should rather be another constructor dependency like any other.

Some team members tend to use Microsoft abstractions just to get the ILogger<T> feature, but then we cannot use Serilog Analyzer nuget which we would like to use.

So another workaround would be to create our own implementation of ILogger<T> for Serilog. But that leads me to a question why it isn't already implemented in Serilog itself? Or is it already somewhere?

Numpsy commented 8 months ago

Or is it already somewhere?

https://github.com/serilog/serilog-extensions-logging ?

AleyCZ commented 8 months ago

Or is it already somewhere?

https://github.com/serilog/serilog-extensions-logging ?

Of course we use that. But that has nothing to do with this feature request. We miss interface Serilog.ILogger<T>, there exist only Serilog.ILogger without generic T parameter.

nblumhardt commented 8 months ago

Hi @AleyCZ! Semi-related - which IoC container are you using? For Autofac, you can use this https://github.com/nblumhardt/autofac-serilog-integration to have the right contextual logger injected without a generic type involved.

I'm open to the idea of supporting this more deeply, though I'm not sure yet how it would fit into the overall design of Serilog. Since this is normally a feature relating to constructor injection, it would be interesting to explore what other options are available with the container you're using.

werebear73-tritelph commented 7 months ago

This would be a nice addition

asleire commented 7 months ago

Stumbled upon this looking at C# 12's primary constructors.

Currently, all my services that use Serilog inject Serilog.ILogger logger and instantiates the private field as logger.ForContext<MyClass>. Using primary constructors this would require explicitly defining a field separate from the constructor parameter which makes it very easy to make mistakes:

public class MyClass(ILogger logger)
{
    private readonly ILogger _logger = logger.ForContext<MyClass>();

    void MyMethod()
    {
        _logger.Information("Calling MyMethod");
    }

    void MyMethod2()
    {
        logger.Information("Calling MyMethod2"); // oops, wrong logger
    }
}

With a Serilog.ILogger<T> just like Microsoft's ILogger<T> I could embrace the cleanliness of primary constructors 😄

nblumhardt commented 7 months ago

FWIW, I think it's pretty straightforward to define your own Serilog-based ILogger<T> and register it as a generic component in your IoC container/MEDI.

It'd look like:

public interface IMyLogger<T>: ILogger
{
}

public class MyLogger<T>(ILogger inner): ILogger<T>
{
    readonly ILogger _inner = inner.ForContext<T>();

    // ... delegate ILogger methods to _inner
}

builder.Services.AddTransient(typeof(IMyLogger<>), typeof(MyLogger<>));

Interested to hear whether anyone has success taking this route. HTH!

asleire commented 7 months ago

It certainly is an easy interface to implement :) I gave it a try in one project, but I'm not sure it's something I want to keep doing as I would prefer not to create my own logging interface

public interface ISeriLogger<T> : ILogger
{
}

internal class SeriLogger<T>(ILogger logger) : ISeriLogger<T>
{
    private readonly ILogger _seriLogger = logger.ForContext<T>();

    public void Write(LogEvent logEvent)
    {
        _seriLogger.Write(logEvent);
    }
}
waynebrantley commented 3 months ago

@nblumhardt would be nice to use this as we make everything a primary constructor. Just allow Serilog.Ilogger in the same way that MEF.Ilogger works.