StackExchange / NetGain

A high performance websocket server library powering Stack Overflow.
MIT License
928 stars 141 forks source link

Logging Abstraction Layer #10

Closed rwkarg closed 7 years ago

rwkarg commented 8 years ago

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 commented 8 years ago

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: https://github.com/aspnet/Logging/issues/349

@mgravell @gdalgas thoughts?

rwkarg commented 8 years ago

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:

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 commented 8 years ago

Looks like there are other opinions in favor of separate internal ILog implementations: https://github.com/aspnet/Logging/issues/332#issuecomment-184636080

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

rwkarg commented 7 years ago

This abstraction was implemented by https://github.com/StackExchange/NetGain/commit/e43fbeaf7220e4cb4b74109f5aba9d3c2ccc026f