Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: log: change Logger to be an interface #13182

Closed
srinathh opened this issue Nov 7, 2015 · 28 comments
Closed

proposal: log: change Logger to be an interface #13182

srinathh opened this issue Nov 7, 2015 · 28 comments
Labels
Milestone

Comments

@srinathh
Copy link
Contributor

srinathh commented Nov 7, 2015

Motivation

Go stdlib provides a log.Logger that is widely used in both Go programs and Go libraries (eg. most notably in http.Server) as a standardized logging component.

However, the current implementation of the log.Logger enforces a specific logging format with limited flexibility which requires text parsing to allow it to be connected to any other logging system implementation through the swappable io.Writer interface. This adds complexity and overhead. The other alternative of directly using third party logging libraries is not very scalable both because existing libraries including components like http.Server in the stdlib expect log.Logger and because of network effects of stdlib.

Proposal

This proposes a minor change to the log package to make log.Logger more extensible in a fully backwards compatible by allowing Logger.Output() to write logs using custom logging implementations.

We do this by defining a function type called OutputFn with the same function signature as the current Logger.Output() and introducing an unexported field outputfn in Logger to hold the function that should be used for output much like we use the out field to hold the io.Writer currently. By default, log.New() will set the outputfn to the current implementation which is moved to Logger.DefOutputFn(). This can be swapped to any other OutputFn implementation by calling Logger.SetOutputFn() in the same way as we currently can swap io.Writer by calling Logger.SetOutput()

For the top level log package, we provide log.SetOutputFn() and log.SetDefOutputFn() to set the OutputFn of the log.std logger and reset it to default.

Proof of Concept

I have implemented a proof of concept for this proposed change and added an example in the test suite that demonstrates logging to a Tab Separated Variable format. While in the example i am merely writing to a Writer, the Output could be to anything including over the network etc.

If this approach looks ok, I can package this into a CL incorporating any feedback.

Rationale for the Approach

Most of the actual work of logging happens in Logger.Output() (and in the unexported Logger.formatHeader() called by it) which makes it a prime candidate for flexibility. We cannot change log.Logger into an interface due to Go1 compatibility promise and this approach therefore allows a lot of flexibility with a very minor change.

@srinathh
Copy link
Contributor Author

Could I request a review of the proposal? Should I submit a CL?

@adg
Copy link
Contributor

adg commented Nov 12, 2015

I don't see the advantage that this provides over the default logger. Instead of parsing colons you parse tabs?

@srinathh
Copy link
Contributor Author

No no.... the idea is to define a minimal function type mimicking the current Logger.Output() function that lets anyone use the standard library log package and log.Logger to do logging in any format and using any communication mechanism (eg. a logging to an API service exposed over the network) without any parsing.

This would allow for instance to pass a log.Logger to http.Server that can plug into any other logging service merely by implementing an OutputFn() and passing it to the Logger.

Purely as an example here, I have implemented a version of OutputFn() that logs into a Tab separated variable format file but the actual implementation can be anything at all.

@srinathh
Copy link
Contributor Author

Hi - could I pls. request another evaluation of the proposal as I believe it may have been mis-understood originally. The objective is NOT to write logs with tabs formatting or whatever. The objective is to introduce flexibility in the standard Logger to allow alternative logging implementations (which could include writing to files, over network, databases etc.) to plug into it with a minimal interface while retaining full backwards compatibility.

@rsc rsc added the Proposal label Dec 28, 2015
@rsc rsc added this to the Proposal milestone Dec 28, 2015
@rsc
Copy link
Contributor

rsc commented Dec 28, 2015

Like @adg, I don't believe this proposal has enough benefit to warrant the complexity suggested for the API. The log package already has a way to divert output, namely SetOutput. Instead of adding a second, why not use the first?

Note that the log package guarantees to deliver each message in a different Write call. So if you really need to tweak the format, it's possible to do today with no changes to the standard library: tweak the log message itself. Quoting the docs:

A Logger represents an active logging object that generates lines of output
to an io.Writer. Each logging operation makes a single call to the Writer's
Write method. A Logger can be used simultaneously from multiple goroutines;
it guarantees to serialize access to the Writer.

The only possible benefit I can see is that the current output writer doesn't know the call depth on each Write, so it can't reconstruct the file and line number for itself. But if that's really important, it's easy to pull them from the written data.

@rsc rsc changed the title Proposal: Make stdlib log.Logger more extensible in a backwards compatible way proposal: add SetOutputFn to log.Logger Dec 28, 2015
@srinathh
Copy link
Contributor Author

Thanks for reviewing Russ. Here is the benefit I see of enabling a more flexible logger output via an approach like OutputFn vs. using current SetOuptut

  • Currently, doing any non-standard logging (either in format or in action) with the log package is a four step process using the SetOutput approach.
    1. Setup the standard logger to generate the fields required for your custom logging through SetFlags() and/or SetPrefix(). The standard flags may be insufficient.
    2. Implement a custom logger that supports the writer interface & call SetOutput() with it
    3. In the custom logger, parse each line of output from the writer to extract the required fields from the string
    4. Perform whatever is the custom logging required.
  • Point 1 means the application code and the logger implementation get tightly coupled and you need to have knowledge of the input format that the custom logger implementation expects to use it. This is fragile and also depends on logging output format being guaranteed.
  • Point 3 is an unnecessary extra step and possibly a performance penalty. Why should a custom logger contain code to parse a []byte and extract timestamps and file-paths and so on vs. focusing on the mechanics of the custom logging?
  • The OutputFn type of approach lets logging implementations be swapped out cleanly without tying the implementation to the application code and also eliminates text parsing.

@rsc
Copy link
Contributor

rsc commented Dec 29, 2015

With the exception of file name, you can shorten to:

  • SetFlags(0), SetPrefix("")
  • call SetOutput(w)
  • w.Write gets just the log message and can add whatever prefix or formatting it wants, without any parsing

If you want file names, then yes you have to use SetFlags(Llongfile) or Lshortfile and parse the file name. That's unfortunate, but it doesn't seem worth a whole new API. If we were starting over then yes I think we might reasonably make SetOutput take a func and have a separate SetWriter. But we're not starting over. We're working with what we have. If we created new API every time we realized we could have done something differently, we'd end up with a much larger (and arguably bloated) standard library. The bar is very high for additions.

@adg
Copy link
Contributor

adg commented Aug 15, 2016

Deferring to Go2.

@adg adg added the v2 A language change or incompatible library change label Aug 15, 2016
@robpike
Copy link
Contributor

robpike commented Aug 15, 2016

I think the right answer is to change Logger to be an interface, and to do that in Go 2.

@rsc rsc changed the title proposal: add SetOutputFn to log.Logger proposal: log: add SetOutputFn to Logger Jun 16, 2017
@ianlancetaylor ianlancetaylor changed the title proposal: log: add SetOutputFn to Logger proposal: log: change Logger to be an interface Jan 9, 2018
@ianlancetaylor ianlancetaylor added the Go2Cleanup Used by Ian and Robert for Go 2 organization. Unless you’re Ian or Robert, please do not use this. label Jan 9, 2018
@ianlancetaylor ianlancetaylor changed the title proposal: log: change Logger to be an interface proposal: Go 2: log: change Logger to be an interface Jan 9, 2018
@universonic
Copy link

@robpike Totally agree. We usually use multiple third-party libraries in one project, and each of these third-party libraries usually has its own logger implementation. This is really a nightmare. A simple and generic logger interface will make this better.

@srinathh
Copy link
Contributor Author

srinathh commented Apr 3, 2019

I agree with the approach with Go 2. What would be the development process?

@ianlancetaylor
Copy link
Contributor

Someone needs to develop a proposal. Past efforts have had trouble finding a coherent logging interface, as there are many different logging needs. So it's not a simple problem.

@montanaflynn
Copy link

montanaflynn commented Apr 5, 2019

There's a detailed proposal (#28412) which was well thought out in my opinion and has a decent amount of discussion in the comments that was closed as a duplicate of this issue.

Not sure where someone can develop a proposal if they get closed and referred back to here.

@gmosx
Copy link

gmosx commented Apr 11, 2019

Not sure where someone can develop a proposal if they get closed and referred back to here.

Maybe someone could sketch-out a preliminary proposal as a comment to this thread.

@MMulthaupt
Copy link

MMulthaupt commented Apr 18, 2019

We're running into the need for a revision of the log package in a project rigtht now. Currently we have agreed on the convention that every non-main package, which is also not coupled tightly with a main-package, may only log if it wants to communicate a warning, by doing log.Printf(). Then we can set it up to be processed with log.SetOutput(pointerToMyGreatLogWriter) where we send it to logrus.Warn(). Somewhere in there, we scavenge the log.Llongfile-information from the log message string to bring it into our format. A fun little field trip through the logging pipeline, but not quite ideal: we don't know what to do with debug calls, and our log formatter has to make some guesses to figure out if the message came from a call to the standard log library.

@stefanfrings2
Copy link

stefanfrings2 commented Feb 23, 2021

Very good idea. Loggers should be based interfaces so we can replace them by custom implementations.
And they should be context-aware, so we can include a request-ID or context-ID in the output.
The main problem is that we have now lots of 1st and 3rd party modules which log messages without severity and context information. This is really bad. It feels like someone repeated the same mistake that Java did 20 years ago.
Now we are free to introduce a new logger but that will not automatically apply to old modules.
You see, I'm a bit frustrated.

AluisioASG added a commit to AluisioASG/snowweb that referenced this issue Apr 28, 2021
While Go's standard log package is not used in SnowWeb directly, it is
still used by the rest of the standard library, like in http.Server, and
even though the latter lets one specify an alternative Logger instance
(as the ErrorLog field) it's still bound the log package because Logger
is a struct, not an interface other packages can fulfill (see
github:golang/go#13182 for a proposal to convert it into an interface).

The end result is that we have two competing logging solutions
throughout the program, writing to possibly different places in
definitively different formats.

Luckily for us, Logger amounts to a glorified Printf and there is no
metadata (like logging level) to lose if we treat it as such, so we can
just have it use zerolog's logger as its output stream.
@codjust
Copy link

codjust commented Jun 23, 2021

Hello, everyone! Any new developments?

burakkoken added a commit to burakkoken/go that referenced this issue Sep 18, 2021
The current implementation of the log.Logger provides us with limited flexibility.
With the new changes, it will provide more flexibility. The changes introduce new
features such as Log Level, Formatter, Root Logger, Context support etc. but
the changes made won't break any existing functionalities or affect any libraries
which use log package. Thanks to the changes made, any custom logger implementation
won't be needed and developer needs will be met. Instead, the formatter can be used
to format the output.

Fixes golang#13182
burakkoken added a commit to burakkoken/go that referenced this issue Sep 18, 2021
The current implementation of the log.Logger provides us with limited flexibility.
With the new changes, it will provide more flexibility. The changes introduce new
features such as Log Level, Formatter, Root Logger, Context support etc. but
the changes made won't break any existing functionalities or affect any libraries
which use log package. Thanks to the changes made, any custom logger implementation
won't be needed and developer needs will be met. Instead, the formatter can be used
to format the output.

Fixes golang#13182
@gopherbot
Copy link

Change https://golang.org/cl/350869 mentions this issue: log: improve logger implementation

@burakkoken
Copy link

Hi All, I have PR related to this issue. I have come up with a different idea that does not break any existing functionalities instead of changing log.Logger into an interface. In my opinion, it will both meet our needs and won't break any existing functionalities. Could you please review it? #48464

burakkoken added a commit to burakkoken/go that referenced this issue Sep 19, 2021
The current implementation of the log.Logger provides us with limited flexibility.
With the new changes, it will provide more flexibility. The changes introduce new
features such as Log Level, Formatter, Root Logger, Context support etc. but
the changes made won't break any existing functionalities or affect any libraries
which use log package. Thanks to the changes made, any custom logger implementation
won't be needed and developer needs will be met. Instead, the formatter can be used
to format the output.

Fixes golang#13182
burakkoken added a commit to burakkoken/go that referenced this issue Sep 19, 2021
The current implementation of the log.Logger provides us with limited flexibility.
With the new changes, it will provide more flexibility. The changes introduce new
features such as Log Level, Formatter, Root Logger, Context support etc. but
the changes made won't break any existing functionalities or affect any libraries
which use log package. Thanks to the changes made, any custom logger implementation
won't be needed and developer needs will be met. Instead, the formatter can be used
to format the output.

Fixes golang#13182
@stefanfrings2

This comment has been minimized.

burakkoken added a commit to burakkoken/go that referenced this issue Oct 4, 2021
The current implementation of the log.Logger provides us with limited flexibility.
With the new changes, it will provide more flexibility. The changes introduce new
features such as Log Level, Formatter, Root Logger, Context support etc. but
the changes made won't break any existing functionalities or affect any libraries
which use log package. Thanks to the changes made, any custom logger implementation
won't be needed and developer needs will be met. Instead, the formatter can be used
to format the output.

Fixes golang#13182
@ccpaging
Copy link

ccpaging commented Nov 2, 2021

After reading the level logger issues, I worked it for some coding as the 3rd package based on go std log.

I mean let the std log be a single-level logger. the level logger need only Ouput() interface.

@eikenb
Copy link

eikenb commented May 19, 2022

Hello, everyone! Any new developments?

IMO this question was pretty well resolved by the discussion for #28412. The takeaway, for me at least, was that if a library wants to support logging the best practice is to provide a callback interface like httptrace.

@ianlancetaylor ianlancetaylor changed the title proposal: Go 2: log: change Logger to be an interface proposal: log: change Logger to be an interface Jun 21, 2023
@ianlancetaylor ianlancetaylor removed v2 A language change or incompatible library change Go2Cleanup Used by Ian and Robert for Go 2 organization. Unless you’re Ian or Robert, please do not use this. labels Jun 21, 2023
@rsc rsc closed this as completed Jun 21, 2023
@rsc
Copy link
Contributor

rsc commented Jun 21, 2023

Marking this obsoleted by #56345.

@rsc
Copy link
Contributor

rsc commented Jun 21, 2023

This proposal has been declined as obsolete.
— rsc for the proposal review group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

Successfully merging a pull request may close this issue.