Provide a logging abstraction

[rjrizzuto]

It might be helpful to log exception/error cases from the .Net library to help in problem resolution. I think Common.Logging is a good candidate for this. It is a logging facade rather than a logging framework, so the user of RabbitMQ.Client can choose to use NLog, Log4net, etc. If nothing is configured specifically, Console.Logging logs NoOpLoggerFactoryAdapter.

Here's a perfect case in point from AutorecoveringConnection.cs:

                catch (Exception e)
                {
                    // TODO: logging
                    Console.WriteLine("BeginAutomaticRecovery() failed: {0}", e);
                }

With common.logging:

                catch (Exception e)
                {
                    logger.Warn("BeginAutomaticRecovery() failed", e);
                }

The logger would log all the details, including stack trace (configurable in the logging config file) about the exception.

[cjbhaines]

I think it would be a bad idea to add dependencies to this library. Every single consumer of it would then be forced to reference Common.Logging which is not a desirable situation. You only really want it to log on bugs in the internal code but the test coverage should be good enough to catch those before it's released. I think an event on the AutorecoveringConnection to notify of RecoveryFailed would be more applicable.

[michaelklishin]

I'm not sure I agree with this. Exceptions do happen from time to time and it's not a great idea from the operations perspective to not have any logging.

We can make the client depend on Common.Logging, and if no logging is configured, it will be a no-op.

[michaelklishin]

This Twitter thread makes me wonder if emitting events is a safer bet. Java client-style exception handler is another option.

[rjrizzuto]

I will add my 2 cents that other libraries have dependencies, and that NuGet is designed to handle installing dependencies when a library is added. For example, we are using NEsper in our application, and it has several dependencies, including one on Common.Logging.

I read the twitter thread, and I see their points as well, and have hit some issues with dll dependencies on different versions of a library, so I can see the merit of callbacks vs common.logging, especially since you have no dependencies currently.

[michaelklishin]

We'll investigate LogLib and Logary.

[michaelklishin]

We've decided to provide more event handlers so that applications can use the logging library that suits them. We may still consider LogLib or Logary in the future.

[asbjornu]

I agree that event handlers and not depending on a logging library is the way to go. Microsoft is rolling out new infrastructure for this with DNX that might be an OK dependency to add, but it's still early days and event handlers are much more versatile and leaves the implementation and choice of logging framework up to the consumer. Which is a good thing, imho.

[kjnilsson]

Closed #267 in favour of this issue.

[kjnilsson]

Ok I've done a first draft here: d758b9e

I've used the standard .NET EventSource/EventListener API as it integrates with a lot of tooling on windows. It can also be used for metrics should we want to.

Typically I'd expect someone interested in rabbit client internal log events to implement their own EventListener. A simple console example can be found here

One awkward aspect of using EventSource is that I'm not able to pass full Exception types to the EventListeners as they don't serialise well. To get around this key exception details are available in a dictionary structure instead.

For net451 I've used a nuget package, mostly to get around limitations of the EventSource api on mono. If we bump the minimum .NET version to 4.6.1 we could get rid of this I think.