Logger should allow for capturing of context

[mmlb]

Logging and Logger is a method for upper layers to allow lower layers to take the responsibility of error handling, so is a top-down mechanism. Errors are a bottom-up method of passing error handling responsibility up the call chain. Unfortunately having 2 different mechanisms means loss of information. Errors have a stack trace but also useful context that we'd want to log in a k=v manner, only the stack trace is logged neatly. kvps are transformed into a string and used in the log message, which would require knowledge/parsing of to be useful again.

This could be avoided by passing instance loggers all the way down and be able to have them add context into a reference field without having to return a child-logger. Then when we get to the decision-maker any log messages it chooses to emit will have machine-readable (and ultimately human friendly) log messages.

This might be confused with go-routine state but is not, this is function-call state. function-call state is best represented as function args but requires changes to go stacktrace capturing and changing coding practices to a more functional flavor. Both of these are unlikely to happen.

There may be an problem with capturing such state for all function calls due to increasing number of allocations and GC, and may induce a performance hit. We'd have to find out. I think most of our services would be fine with a hit, especially if the logging is greatly improved. We also would have discretion on what to add to the context which most likely will be much less info than all the args to all the functions.

[mmlb]

This is probably better of as an RFC.

[detiber]

+1 to having a mechanism for passing loggers down.

It might be nice to define (or consume) a logging interface for the purposes of functions/methods/packages that want to accept an externally defined logger, though. That way the library implementation is less tied to a specific concrete logging implementation, which may cause dependency conflicts between the library and consuming applications.

https://github.com/go-logr/logr is commonly used in the Kubernetes ecosystem for this, and the controller-runtime tooling has created a zap-based implementation that could likely serve as a guide if we wanted to do similar with the zap-based logger that exists in this repo.

[mmlb]

https://github.com/go-logr/logr looks like a great idea to implement here and change the code bases to make use of.

This issue isn't about pushing loggers down, we already do that all over the code bases. This is more about pushing a mutable context object around so that lower levels can add the context and the upper layer can decide to log or not. If so, the context from below is available. This would be wasteful if not-logging ends up being the common case as we'd be adding context and using up ram only to throw it away.

We'd also have to think about key conflict handling too.