bchavez / RethinkDb.Driver

:headphones: A NoSQL C#/.NET RethinkDB database driver with 100% ReQL API coverage.
http://rethinkdb.com/api/java
Other
383 stars 134 forks source link

Use LibLog for Logging #130

Open bchavez opened 6 years ago

bchavez commented 6 years ago

https://github.com/damianh/LibLog

Copy LibLog.cs to the driver project. Basically, LibLog.cs contains some fancy reflection to find out what "logging" system is present in the execution environment.

Pros Allows us to break some hard library dependencies that are only used for logging. IE: Common.Logging (full framework) and Microsoft.Extensions.Logging.Abstractions (.net core). Generally, any opportunity we get to break a dependency is good.

Cons

IIRC, Npgsql is looking to move to LibLog.

This will be scheduled for the next major 2.4 driver release (with write hooks).

:zzz: :zzz: "You don't want to... Wake, push through"

wjrogers commented 6 years ago

A possible alternative is to just consolidate on Microsoft.Extensions.Logging.Abstractions for both build targets. I see in #132 you're considering bumping the full framework build to 4.6.1. That would make the Microsoft.Extensions.Logging.Abstractions package compatible, too. It has the advantage of supporting structured logging and scopes, and there are integrations for all the popular log libraries.

I don't have a strong objection to LibLog, but it comes with the same baggage and compatibility challenges as any third-party dependency. With the Microsoft API, the integration code is at least external to your own library, whereas with LibLog, it's baked in.

bchavez commented 6 years ago

Hey Will,

Thank you for the suggestion. It is most definitely worth considering Microsoft.Extensions.Logging.Abstractions if/when we make the move to 4.6.1. :+1: Whichever way we ultimately go, I'd like to consolidate on just one logging solution. I appreciate your input, it's important for me to hear from devs using the driver.

:fire: :volcano: "We're building it up, to break it back down..."

wjrogers commented 4 years ago

FWIW, LibLog did a final release for .NET Core 3.1, then archived the project and declared it deprecated:

My recommend course of action for library developers is to use Microsoft.Extensions.Logging.Abstractions as it is now the defacto standard logging interface in the .NET ecosystem. For library developers that wish to continue to use LibLog or need to make adjustments for their needs should copy the code into their project (after all, that's just what LibLog did).

damianh/LibLog#270

bchavez commented 4 years ago

Hi Will,

Thanks very much for the update. Oh dang, that's unfortunate. I'm not sure I would be happy totally going with Microsoft.Extensions.Logging.Abstractions. Do you know the versioning story behind Microsoft.Extensions.Logging.Abstractions? It seems like versioning is tightly coupled to ASP.NET Core.

Also, I need to check the dependency graph when pulling this thing down.

wjrogers commented 4 years ago

I am honestly not sure what their versioning story is. They don't do a great job documenting each library or providing release notes. (Probably a consequence of these libraries growing organically from a component of ASP.NET Core into a general purpose public release.)

The good news is the package has no dependencies. In practice, I'd expect you to be fine choosing any version and sticking with it.

In fact, I'm wondering why you are asking these questions when you already use Microsoft.Extensions.Logging.Abstractions version 2.0.0 in the netstandard2.0 build of the driver? You could just remove the condition on that reference and the associated code, bump the full framework build to net461, and be done.

wjrogers commented 4 years ago

Delayed after-thought: instead of bumping the full framework build to net461, you could just remove it. .NET Framework 4.6.1 and higher projects are compatible with netstandard2.0 libraries.