Add logging abstraction to use instead of ETW traces

[ricardSiliuk]

Add an extra dependency to Microsoft Logging Abstractions so a logger can be passed into RMQ client. I'd argue that using LogEventSource looks out of place in more modern dotnet applications when it comes to logging - mostly because it's aimed at file-based logging as opposed to structured logging.

By having ILogger attached like this you could still log into ETW by providing Logger that calls LogEventSource inside. The opposite is also true - you could create your own event listener that logs into a logger, but event is already flattened, so you can't extract additional info that could be present in an exception or scope (unless event source did that for you). Having it the other way around is more flexible.

[codecov[bot]]

Codecov Report

Base: 92.80% // Head: 91.64% // Decreases project coverage by -1.16% :warning:

Coverage data is based on head (2803400) compared to base (20ba4b6). Patch coverage: 93.17% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #156      +/-   ##
==========================================
- Coverage   92.80%   91.64%   -1.17%     
==========================================
  Files          94       77      -17     
  Lines        8005     5994    -2011     
  Branches      597      408     -189     
==========================================
- Hits         7429     5493    -1936     
+ Misses        468      383      -85     
- Partials      108      118      +10     

Impacted Files	Coverage Δ
RabbitMQ.Stream.Client/ClientExceptions.cs	62.50% <ø> (-3.22%)	:arrow_down:
...bitMQ.Stream.Client/Reliable/IReconnectStrategy.cs	73.68% <44.44%> (-7.27%)	:arrow_down:
RabbitMQ.Stream.Client/Reliable/ReliableBase.cs	75.00% <57.14%> (-21.30%)	:arrow_down:
RabbitMQ.Stream.Client/Subscribe.cs	65.21% <60.00%> (-13.73%)	:arrow_down:
RabbitMQ.Stream.Client/AMQP/Data.cs	76.66% <66.66%> (-17.46%)	:arrow_down:
Tests/Utils.cs	77.35% <72.00%> (+1.13%)	:arrow_up:
RabbitMQ.Stream.Client/Producer.cs	78.53% <79.31%> (ø)
RabbitMQ.Stream.Client/Client.cs	92.61% <83.33%> (+1.52%)	:arrow_up:
RabbitMQ.Stream.Client/AMQP/Header.cs	86.20% <85.71%> (ø)
RabbitMQ.Stream.Client/Consumer.cs	89.28% <89.28%> (ø)
... and 40 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[lukebakken]

I'm reviewing the long discussion here: https://github.com/rabbitmq/rabbitmq-dotnet-client/issues/357

[lukebakken]

Hi @ricardSiliuk - thanks for this and your other contributions. The discussion around this topic for the RabbitMQ .NET client centered around adding a dependency and versioning issues it could cause. However, major projects like npgsql have this dependency.

Do you think the chances of "DLL hell" are small enough to not be an issue?

[ricardSiliuk]

Small enough :) If you look at dependencies of this abstraction - there's not much to it, especially since this is targeting .Net 6 and up. Users will come with their own implementations so dependency footprint of this library stays small.

Though I see I have to resolve quite a few conflicts to merge this :D

[lukebakken]

We're evaluating this for 1.0.0. Having this dependency would be different than the AMQP .NET client.

[lukebakken]

@ricardSiliuk please resolve the conflicts in your PR and we will merge it. Thanks!

[lukebakken]

I'll see if I can resolve the conflicts, starting on it now.

[ricardSiliuk]

Left some minor comments. Can't take a deep look into it due to sad personal matters so don't wait up on me. The only real "issue" I'd investigate is whether ILogger is a good type of arg to pass. Maybe it should take in ILogger<T> (for reference).

Maybe it doesn't matter for this library - since client itself isn't created via DI, specifying category doesn't change much.

Will have to think about it...

[ricardSiliuk]

Sorry, messed up the branch a bit but while reworking it I got stumped by how Factory is being used here... I'll reach out to @Gsantomaggio tomorrow to raise concerns/clarify a thing or two.

[Gsantomaggio]

closed in favour of https://github.com/rabbitmq/rabbitmq-stream-dotnet-client/pull/190