Keep it simple (abstraction only)

[ejsmith]

I really like the ILogger and ILoggerFactory interfaces. They are great abstractions that the community can build upon. The problem is that this package includes some extra things that are opinionated and that not everyone agrees with. Can we please move those extension methods to another package and keep this package to exactly what it says it is?

[ejsmith]

Any thoughts on this? I know you guys don't want to make any more breaking changes, but I personally think this is very important. Let's just keep this package extremely simple with just the abstractions. It should be pretty easy to migrate projects if another package is created with the extensions in this package. The migration would just be a matter of adding another package to the project.

[niemyjski]

+1

[herecydev]

The problem is that this package includes some extra things that are opinionated and that not everyone agrees with

It's probably best to quantify that surely?

[ejsmith]

So basically there are a ton of extension methods here:

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Abstractions/LoggerExtensions.cs

and here:

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Abstractions/LoggerFactoryExtensions.cs

I just want this package to be the core interfaces only because a lot of people have differing opinions on what these extension method signatures should look like and there is also another whole use case of people creating strongly typed logger extension methods on top of the ILogger and not wanting to use any of these extension methods at all.

They can't make that choice because they have to bring this package in and use the namespace that they ILogger types are in. Which is also the same namespace as the ILogger extension methods are in. Which means I have no choice but to have all of them on my logger classes.

[herecydev]

Then again, from my experience these are invaluable. I don't want to log out the EventId or worry about the formatter, which makes methods like this invaluable.

It's hard to make a one-size-fits-all package that isn't going to upset some people

[niemyjski]

@herecydev if everything is a package we could just add this to a logger.extensions package and then you can bring it in and then it's a buy in and every one is happy.

[ejsmith]

The extension methods would live in a different package. I'm talking about just simplifying the abstractions package. I think most people will install the Microsoft.Extensions.Logging package anyway and it could have the extension methods in it or it could have a dependency that brings them in. Most people would never know.

This is purely about keeping the logging contracts (the interfaces) in a package on their own. Short of assembly neutral interfaces, the best we can do is to have just the interfaces in a tiny package by themselves and hope that the community agrees with them and everyone uses them.

[herecydev]

@niemyjski I understand what you're saying and I don't necessarily disagree. The problem is that you assume that everyone will be happy without understanding the disadvantages; If a new developer tried to incorporate the base Microsoft.Extensions.Logging.Abstractions package, it's not immediately obvious that they needed the extensions package to have the easier methods of logging. Ultimately, there always be a trade off and not everyone can be happy.

[ejsmith]

You are right you can't please everyone which is why it's soooo important that the logging contract be kept as simple and unopinionated as possible. If we can get the community to agree on a simple logging contract then we avoid fragmentation hell where every library author picks different logging abstractions or just makes their own. Which results in app developers having to create all kinds of logging proxies to get the differing contracts to talk to each other.

I think we can make this change even less obtrusive by creating a new package called Microsoft.Extensions.Logging.Contract the abstraction package would include it and nobody would have do anything to their projects.

[ejsmith]

Are you guys open to a pull request for this? Not sure who would make the decision @davidfowl ?

[davidfowl]

Personally I think it's overkill to split the extension methods from the abstractions package.

[ejsmith]

I think its super important. There is no way you will get everyone to agree with all of the methods that are included in the abstractions package which means it will cause people to do something else and we will continue to have logging fragmentation and have to build proxies to translate from one to the other. This is a simple change and it gets the surface area of the logging contract down to the minimum.

I submitted a very simple pull request #373 that should not cause any breaking changes. Please consider it.

[nblumhardt]

It's an important consideration, but it's also strongly possible that this would follow in the footsteps of previous (mostly unsuccessfull) attempts to build a universal logging abstraction in .NET.

Libraries like log4net and Serilog already include their own ILogger concepts, and would take a negative usability hit by porting to a new one; just tacking on m.e.Logging's interfaces won't reconcile differences like level names, concepts ("Event Id") and so-on. There would need to be a compelling advantage before taking a direct dependency on the interfaces package would be a winning proposition.

There's also still a lot of detail to consider. Even this subset won't necessarily be implementable - e.g. Serilog uses an immutable pipeline and has no factory concept in which AddProvider() would make sense.

There's a chance for this library to help consolidate some of the ecosystem, but to me, going from "a single logging abstraction for the ASP.NET framework itself" to "a generic logging abstraction for .NET" is a big leap to make and one that would need a lot more baking.

[ejsmith]

The simple fact that this is the logging abstraction used by MS for the entire Core framework means that most people will just use it. So to me, this is an opportunity to take a big step towards unifying things. If I can avoid having to have multiple logging concepts in my app then I will. And the bigger problem is what do library authors use?? A lot of people roll their own (me included) because there is no ubiquitous choice and we don't want to pick sides which forces users of our libs to pick the same side as we did, but they can't because the other lib they are using picked a different abstraction. This is a crappy situation that a TON of lib authors are faced with. Having a standard logging abstraction would be a HUGE help. These interfaces are very simple and could even be simplified further. I think the community would adopt them and we would save everyone a LOT of trouble.

[muratg]

@lodejard

If we decide to do this, the simplest way to do it would be as described in #370 (comment)

This would not require changes in any other repo.

[ejsmith]

I did a pull request #373 that makes the change that way.

[lodejard]

I would suggest we consider those methods to be part of the client interface. They're extension methods only to make the interface easier to implement and delegate, not because they're optional or swappable.

If you have a different projection of the logging interface you want to consume - create it as a different interface and make it available add a service in your app which calls through to the underlying abstraction (or even calls through to Serilog).

@nblumhardt hit the nail on the head though that this is not attempting to be a universal logging abstraction for things like Serilog to build on. Instead - it is an abstraction for re-usable components and frameworks to be connected to the concrete implementation which the application has chosen to use. Beyond that (for example) if you are writing an application that has chosen to use the Serilog provider exclusively - it's a perfectly fine for your application your first-party components to use Serilog's interfaces and abstractions directly, which would be a way of leveraging the more advanced features that aren't available through the ILogger abstraction.

[ejsmith]

Did you see my comment about library authors?? What are we supposed to use? If we pick a logging library to do logging inside of our libs then we require users to install that logging lib as well which really pisses people off if they aren't using that same logging lib for their apps. This is a BIG problem for lib authors. I believe that these interfaces are simple enough and performant enough that they could become the basis for the majority of logging libs or at least supported by them out of the box.

@mgravell what do you think about this? I know you were looking to use these interfaces for your StackExchange.Redis library as well.

Moving the interfaces out to their own package doesn't seem like it would cause any issues for anyone. Am I missing something?

[lodejard]

Yep, I saw the comments about library authors. Just to be clear though - when you say library authors you mean people writing third-party re-usable class libraries to be shared via nuget? In that case using ILogger seems like a perfectly valid choice of abstraction, and you wouldn't bring in a concrete logger. Like NHibernate used to bring in log4net by direct dependency (does it still?).

However ILogger alone simply isn't a complete interface without the extension methods, especially if you are interested in flowing structured logging data from your class library. Or if you want to use some of the zero-allocation string-less mechanisms that are available.

That means the class library author would need to bring in the second package which extends the interface - that is the extra code which is in abstractions package today.

Then you end up with a new problem, if you're an application author now and you're using more than one of these third-party class libraries packages in your app - they may have each chosen to extend ILogger with a different enhancement package. Now those multiple forms of extension methods are projected up to the application author as transitive dependencies.

Hopefully they're in a different namespace or they can't use one without the other, but even if the extension methods are in different namespaces now they now need two using statements in my code instead of one.

[ejsmith]

Yes, people writing 3rd party nuget packages.

ILogger by itself is all that is needed. If the extension methods are part of the contract then IMO this logging abstraction will not get mass adoption because the extension methods are too opinionated. In my library I would add a nuget reference to the logging contracts package (just the interfaces) and I would either source import the extension methods into my library internally or I would create my own. But the end result is that my library only requires the contract package to be installed by apps that use my library and all of the logging implementations (NLog, Serilog) would just support those contracts and everything would work with the least amount of intrusion possible on the users of my library.

[Eilon]

@ejsmith I'm curious in the 3rd party NuGet package how will it call the core ILogger interface method? In what way will the code be different from the currently available extension methods?

[davidfowl]

@ejsmith Well for one, they would remove the extra 3 character Log prefix from the methods 😜

[ejsmith]

Ugh. I can see that this is going nowhere. I tried.

[cwe1ss]

As said by @lodejard I also think it's a burden to include yet another package. It's true that .NET Core has many packages, but afaik their packages usually aren't for such a small use-case.

The ILogger just doesn't seem useful to me without the extensions.

[nblumhardt]

Howdy all- tangential, but relevant in case anyone's missed it, LibLog by @damianh is a nice solution for libraries that need general logging capabilities.

It's a zero-dependency, source-only package that dynamically detects and attaches to a variety of logging frameworks (log4net, NLog, Serilog, ...) without requiring any adapter or interface packages.

I personally think it deserves a tonne more publicity :-) but it's already being used in quite a few projects that I've spotted (just noticed it's used in HangFire recently, I'm guessing Damian has a longer list).

[ejsmith]

@nblumhardt thanks! Didn't know about that library. This is exactly the problem that all library developers are faced with. This wiki page does a great job of explaining the problem:

https://github.com/damianh/LibLog/wiki

The problem with LibLog is that it's using reflection to bridge the gap between different logging implementations and that is the kind of hoops that lib authors have to jump through to not piss of users of our libs.

Short of having a feature like Assembly Neutral Interfaces in .NET itself, our best hope is to have a very simple contract that can be agreed upon and shared by the community. ILogger is pretty darn simple, doesn't force any further dependencies and I believe that the community could accept it and we could avoid this issue.

You guys think that I am worried about "Log" being in the name of the extension methods. I'm not and you are missing the point. ILogger is a very simple contract. Why would we want to make that contract larger than it needs to be and thereby reduce the likelihood that it will be accepted by the community as a defacto standard?? Not everyone agrees with your extension method names and signatures and they won't want their logging interfaces cluttered with your methods. And logging implementations don't care about them either. They just need to implement the single method in the ILogger interface.

As far ILogger not being useful by itself, you are right, but I can easily do something like LibLog does and bring in a single file full of useful extension methods and use them internal to my library and not expose those external to my library.

[lodejard]

@ejsmith Well, first off thank you for the interest! Obviously it's a system you like already or you wouldn't care to spend the energy to improve it.

That said - I have to reiterate the extension methods and other types in the abstraction do nothing more than provide support for structured logging, and a zero-allocation calling pattern for hot-path code. It's important to note that none of that code adds more dependencies than you would find in the interface alone. Well, maybe it does to things like generic collections, but you wouldn't write an app without those in the first place.

On the other hand, having two packages by splitting the interface and extension methods apart, does double the dependencies you add to use ILogger in a class library.

The point that logging providers only need to implement a few methods on two interfaces is already true. They also don't need to have that dependency in their core library, the ILoggerProvider is often implemented in a crossover package which only the top-level application needs to depend on.

Also, it's entirely possible to use ILogger interface without the default extensions with a "using alias". In fact you can alias it down to a specific ILogger<YourComponent> and save yourself some angle-brackets in the class.

// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.AspNetCore.Mvc;
using ILogger = Microsoft.Extensions.Logging.ILogger<WebApp1.Controllers.HomeController>;

namespace WebApp1.Controllers
{
    public class HomeController : Controller
    {
        private readonly ILogger _logger;

        public HomeController(ILogger logger)
        {
            _logger = logger;
        }

        public IActionResult Index()
        {
            return View();
        }
    }
}

[davidfowl]

What if we changed the namespsce of the default logging extensions ?

[lodejard]

Two using statements for the common case? Nah, that's no good.

[cwe1ss]

@ejsmith Could you try to explain the benefit again in a different way? Not having the extension methods in the same package would only make sense IMO if there was a reason for people to use other extension methods. I do think that some important overloads are missing ( #294 ) but I'm not seeing what other methods would be useful (except removing the "Log"-prefix).

I also really want this library to get adapted widely because IMO the current situation is pretty painful ( #332 ). I just don't think we should go too far if there is no real advantage. So some further explanation would be great.

[davidfowl]

Two using statements for the common case? Nah, that's no good.

Yah, I know, separate packages are much worse though. I agree we normally put extension methods in the same namespace for this exact reason 😄

[erik-am]

I would like the extension methods to stay in there. It's much more user-friendly and convenient as a shorthand. When I write a library myself I would strive towards a minimum set of dependencies. Then I need to choose:

• not using the extension methods and only depending on Abstractions, which ends up in less readable code.
• Or I should depend on both the Abstractions as well an Extensions package, and so the consumers of my library will also need to depend on both packages...

If you split the packages you'll see that in a lot of cases you'll depend on Abstractions as well as Extensions. So there is cohesion. In my opinion, keeping them in the same package, as is the current situation, is in line with the Common Reuse Principle.

Of course, if the extension methods don't cut it for you, you can always add your own in a separate nuget package. As long as the (number of) arguments are different, it's comparable to overloading. We did this in our project by adding extension methods that also accept a Request object, to log request details alongside the message itself.

[304NotModified]

We at NLog think it's a good idea to indeed split the packages as described at the topic start.

Of course, if the extension methods don't cut it for you, you can always add your own in a separate nuget package. As long as the (number of) arguments are different,

We don't think this is true. Adding method overloads is for example not binary source-level compatible so it could bite. Also more method isn't a win for the end-user and at last the method interface should be consistent - the current extensions could be inconsistent for a particular library.

[Eilon]

Adding method overloads is definitely binary compatible. Changing/removing method overloads is not binary compatible. Adding method overloads I think can be a compilation compatibility problem, depending on optional parameters, etc.

[304NotModified]

You're right @Eilon, it isn't source-level compatible but it is binary compatible.

[damianh]

@ejsmith

The problem with LibLog is that it's using reflection to bridge the gap between different logging implementations and that is the kind of hoops that lib authors have to jump through to not piss of users of our libs.

Yes I built this out of sheer frustration with anything more than a trivial application. I really wish I didn't have too.

Short of having a feature like Assembly Neutral Interfaces in .NET itself, our best hope is to have a very simple contract that can be agreed upon and shared by the community. ILogger is pretty darn simple, doesn't force any further dependencies and I believe that the community could accept it and we could avoid this issue.

There is little hope in coming up with "agreed upon and shared by the community" ILogger interface. The real solution, as you have alluded to with "Assembly Neutral Interfaces", is interface structural typing a-la Go. Then one or more ILog like interfaces will be developed where lib developers can choose to support and end users can easily plug or adapt.

Then I can kill LibLog :)

[ejsmith]

@damianh completely agreed. Although I would argue that the community could agree on something as simple as this:

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Abstractions/ILogger.cs

It would just serve as a basic contract or protocol for all the varying libraries to communicate. The common logging project got decent traction, but if MS were to release something similar that just defines the very basics of logging communication and is even simpler than the common logging abstraction was. Then I think it would have a REALLY good chance to get a lot of traction.

[muratg]

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.