madelson / DistributedLock

A .NET library for distributed synchronization
MIT License
1.75k stars 182 forks source link

Adding logging #143

Open gliljas opened 1 year ago

gliljas commented 1 year ago

In the same vein as #137 I would appreciate more logging in the system, e.g. when troubleshooting leases. This could give a bit more context, such as when the lease was last renewed etc.

These days, many if not most libraries take a dependency on Microsoft.Extensions.Logging. I think that's feasible, but it has its own set of weaknesses and the dependency might be undesired. Requiring an implementation of a DistributedLock specific logger factory is certainly not a bad thing. And of course, it can be completely opt-in, with the default being a no-op logger.

madelson commented 1 year ago

@gliljas do you think it would make sense to have both metric instrumentation and logging instrumentation? Are they complementary or not? Which do you see as a better fit for what you are doing?

gliljas commented 1 year ago

I believe it would make sense, yes. There may be some overlap, but not much. A metric could indicate e.g an increasing delay in the lease refreshes, but when troubleshooting a specific error, like in my case, a log entry would be preferable.