damianh / LibLog

LibLog is a single file for you to either copy/paste or install via nuget, into your library/framework/application to provide a logging abstraction.
MIT License
929 stars 153 forks source link

Support LIBLOG_PROVIDERS_ONLY for libs with their own logging API #56

Closed adamralph closed 9 years ago

adamralph commented 9 years ago

Originally titled 'scriptcs use case'. After discussion, it seems there is a potential for a new feature here so I've updated the title accordingly.

Whether this is just an FYI, a discussion or a proposal for a feature I don't know, but I'm going to leave it here FWIW.

We are in the process of adopting LibLog for scriptcs (elbowing out Common.Logging).

The way logging has been done scriptcs historically is slightly different to most cases. Instead of using static log provision, i.e. private static readonly ILog log = LogProvider.For<Foo>(); etc, we inject logging types via constructors (currently ILog but we are taking advantage of the breaking change of removing Common.Logging to change to ILogProvider). When using the core library, it is the responsibility of the consumer to pass an ILogProvider instance when constructing any objects which demand it. When using the hosting library, an IoC container is used to provide all the services required so an ILogProvider only needs to be passed once. (There is also an outstanding issue to remove this burden from consumers who don't care by providing an overload which uses a default logger internally.)

The way we have adopted LibLog is to install it in our central Contracts library, which contains chiefly interfaces and some light abstract classes. It is the ILogProvider interface defined here which is used by all dependent libraries and indeed is the interface designed for consumption by script packs, modules and scripts when they demand an injected instance. I.e. it the officially supported interface for logging in scriptcs. Now, this does immediately couple the ecosystem to LibLog but, if the interfaces change in the future, we accept the burden of deciding whether to propagate the breaking change or hide/customise the LibLog interfaces as we see fit. For convenience, we also expose a DefautLogProvider which wraps the internal LibLog providers and a NullLogProvider.

Now, the pros and cons of constructor vs static logging provision can be debated, but it does have the advantage that when a custom logger is specified, as indeed it is when the hosting lib is used from scriptcs.exe, there is no need to adapt and propagate the library specific ILogProvider to the lower level libs. I did spike conversion of the whole thing to static log provision but I decided to abandon it since logging injection for script packs, modules and scripts is something we do want to support, so ultimately we will always end up doing this using centrally defined logging interaces in the Contracts library. Having each lib with it's own LibLog with all the adaptation required to get a provider all the way down into Contracts and the adaptation to get it back out as a Contracts interface just seemed like complete overkill.

So, the bottom line is, for this use case, we hack out about 60 lines of LibLog to reduce the API to only that which is required to support it. You can see the change here https://github.com/adamralph/scriptcs/commit/2ee8d6916ccd2e9ab855a7517558995fd612d169

To summarise:

Why am I raising this issue? Good question. I guess I'm looking for opinion on the above and/or a consideration of whether you'd like to support a conditional compilation symbol which could give us just the API required for this use case without having to customise LibLog after installation. I concede that it's likely few projects are using LibLog in this way, but I believe the approach of constructor based log provision, as opposed to static log provision, can be regarded as a general pattern.

</walloftext>

damianh commented 9 years ago

I understand ILog DI scenarios when you want to control things like logger name etc from the out side. I occassionally do it in conjuction with static member field approach (the latter being much more common).

LibLog as it currently stands doesn't prevent you from doing this as it is - you can still DI ILogProvider and/or ILog around...

But if one only wants to DI logging stuff, then yes, let's explore a conditional compilation symbol.

I'm not in favour of a seperate LogProviders package though - I want to keep it single file & single package.

damianh commented 9 years ago

As I mentioned, I think this will support scenarios where Libs with existing logging abstractions can slip LibLob in under. :+1:

adamralph commented 9 years ago

Just had a thought, this also makes it nice for people who are writing packages like MassTransit.LibLog and Topshelf.LibLog since all those should be doing is providing implementations of the interfaces defined in the core libs which forward to LibLog underneath.

adamralph commented 9 years ago

/cc @robinvanderknaap - see last comment

I guess @SimonCropp could also be interested WRT Anotar.LibLog.Fody although I'm unfamiliar with the mechanics of Fody so I'm not sure if applicable.