Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Logging Abstraction Layer #10

Closed
wants to merge 2 commits into from
Closed

Conversation

rwkarg
Copy link

@rwkarg rwkarg commented May 3, 2016

Addressing Issue #5 by adding an internal Logging layer.
This defaults to NoOp logging but has an implementation of a Console logger to maintain the existing console logging functionality.

Expected usage is for users to implement ILog and LogManager for their specific logging solution. The LogManager and ILog interfaces are intentionally kept small so there's little to implement.

LogManager.Current needs to be assigned before using any of the framework. See the test console for example usage setting the Console logger.

@NickCraver
Copy link
Contributor

I like the idea, but I think it would be far better to pull in the new common logging interfaces from .Net Core (that don't depend on .Net Core) as they advance - likely safe after RC2: https://www.nuget.org/packages/Microsoft.Extensions.Logging

Every module we have with their own logging interfaces simply doesn't scale - it creates more and more pain points and adapters. To maintain sanity, we need to standardized. I nudge Microsoft to clarify intent on the package, which they have done: aspnet/Logging#349

@mgravell @gdalgas thoughts?

@rwkarg
Copy link
Author

rwkarg commented May 4, 2016

I understand the concern having a isolated internal logging abstraction and the associated setup work.

This was done because there was a lack of stable third-party logging abstraction layer. If there is one from Microsoft that we're relatively certain:

  • won't introduce breaking and/or conflicting changes
  • will be used by multiple libraries (so we get the benefits of scale)

then that is better than having an internally defined one.

Some background on why the internally defined layer was proposed:
We've used Common.Logging in the past as a "common" logging abstraction layer for our internal shared libraries (partially because many third-party libraries we use were already using it) but are now in the situation where some of our dependencies require C.L v2 and some v3. These are not able to run side-by-side so we have had to maintain our own forks of several libraries (internal and third-party) to standardize on v3. We've actually removed the hard C.L dependency on all of our internal libraries and are starting to look at contributing updates to our third-party dependencies.

Definitely want to investigate the Microsoft offering.

@rwkarg
Copy link
Author

rwkarg commented May 4, 2016

Looks like there are other opinions in favor of separate internal ILog implementations:
aspnet/Logging#332 (comment)

That said, the M.E.L conversion looks pretty straight forward (after updating to 4.5.1)

@rwkarg
Copy link
Author

rwkarg commented Jul 19, 2017

This abstraction was implemented by e43fbea

@rwkarg rwkarg closed this Jul 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants