log icon indicating copy to clipboard operation
log copied to clipboard

Inherent design problem with HandleLog function

Open tympanix opened this issue 6 years ago • 3 comments

Problem: It is allowed by the developer to construct a Entry struct (field are public), but you supply no way to submit the entry to a handler correctly using the HandleLog function.

How? The Entry struct requires to be finalized, which can only be obtained using the private, which is only called here: https://github.com/apex/log/blob/ff0f66940b829dc66c81dad34746d4349b83eb9e/logger.go#L146 This leaves the developer forced to use the public methods of the Logger struct, which does not support the HandleLog function

Why? Sometimes the developer may want to have complete control of the logging (e.g. setting the log level of the entry dynamically)

Possible solutions:

  • Make the finalize function public
  • Wrap all HandleLog functions and call finalize beforehand
  • Add HandleLog to the Logger struct and call finalize
  • Create a wrapper handler which only job is to finalize the entry

You are probably more suited to find a proper solution yourself. Your thoughts?

tympanix avatar Jun 26 '18 13:06 tympanix

Hmmm tough call, I feel like exposing .finalize would be a little gross, but I see what you mean. I think the part I dislike the most right now is that Entry.Logger, it would be nice if we could just pass an Entry to the handler without any reference to that

tj avatar Jun 26 '18 16:06 tj

I agree. Finalizing the Entry should be transparent to the developer. But with no gab between the actual Entry and the HandleLog function, figuring out a solution is tricky. Could we possibly do without a finalize method at all? The developer will be responsible for settings the timestamp on the Entry themselves (this would of course still be handled in the higher level functions Info, Warn Error etc.)

tympanix avatar Jun 26 '18 21:06 tympanix

Yeah I think that would be fine, I can't remember why I did it this way haha, maybe just a convenience thing but I'm down with that!

tj avatar Jun 29 '18 18:06 tj