golang
proposal: log/slog: add `slog.DiscardHandler`
Proposal
Add a package-level variable slog.DiscardHandler (type slog.Handler) that will discard all log output.
Rationale
I have been happily busying myself integrating log/slog into old packages and replacing the exp import in new ones, however I've found myself implementing the same "nop handler" again and again in packages with optional logging APIs.
A common pattern in logging injection is to consume an interface or a pointer to a logging type to allow the dependency to use a derived logger with some field marking the messages as originating from that component, enabling both a standard format as well as per-component level thresholds to keep the output sane. For packages that support slog, this will likely often end up being a *slog.Logger or a slog.Handler, however I've also had to develop a series of log adapters for common dependencies which use their own concrete types, interfaces and callbacks.
In both of these cases, it is helpful to have a default fallback which logs nothing, so that the packages can be tested or used on their own without needing boilerplate guard clauses on every logging method to check for nil values. It's also helpful for less well-behaved dependencies which create their own loggers if one is not provided, some of whom are exceptionally chatty for no good reason. Packages like this are sadly far too common, and we're likely to continue seeing them, so a way to bring some sanity to the process would be very useful.
While it's true that checks for the presence of logging can be reduced by funneling all handler interaction through a single point, this introduces another sort of boilerplate for slog where Records need to be created manually, adjusting their PC to account for the extra frame as seen in the wrapping example. This is a low-level and overly-manual process just to enable conditional logging.
Currently, there doesn't seem to be a great answer for this in slog:
- there are no guards for a nil receiver on
*Loggermethods - the zero value of
Loggeris not usable and will panic Loggers initialized with anilhandler will panic- there is no way to modify a logger to change its handler or logging level once it's created, making
slog.Default()unworkable as a fallback defaultHandleris unexported, so it cannot act as a fallback, and handlers do not provide aWithLevelorWithLevelermethod, so it wouldn't work as a fallback even if it were exported.HandlerOptions, which allows specifying a minimum level, only applies to the built-in handlers, and there is noWithHandlerOptionsthat would allow derived handlers to adjust their logging level using this type.
Leaving aside the arguments on the merits of logging in packages, this pattern is nonetheless common enough that it would probably be useful to have a type DisabledHandler struct{} or even a func DisabledLogger()*Logger convenience method that could act as a default. when the zero values are nil.
log.Logger has an optimization where it detects when the writer is io.Discard and does no allocations. Maybe slog should do the same thing.
log.Logger has an optimization where it detects when the writer is io.Discard and does no allocations. Maybe slog should do the same thing.
I'm not sure this would work in this case, since this would be an implementation at the handler level, which would mean it would only apply to the standard lib JSON and KV handlers. It would require the user to take an slog.Handler and write something like:
if config.LoggingHandler == nil {
//default: this JSONHandler does nothing
config.LoggingHandler = slog.NewJSONHandler(io.Discard, nil)
}
// need the handler since slog.New(nil) will panic
logger := slog.New(config.LoggingHandler)
Which still feels pretty awkward.
I wouldn't want to make a nil *Logger meaningful. I think it would lead to hard-to-find errors where forgetting to set a logger resulted in no log output, and that is only detected when you need the log output (like during a production outage).
I think a slog.DiscardHandler variable would make sense, just as we have io.Discard. Or maybe NopHandler is a better name. Does that work for you?
I wouldn't want to make a nil *Logger meaningful. I think it would lead to hard-to-find errors where forgetting to set a logger resulted in no log output, and that is only detected when you need the log output (like during a production outage).
Concur. Either handler name works for me and, while I like NopHandler, I think it probably serves as more of a mnemonic to call it DiscardHandler in keeping with the convention elsewhere.
This would make the fallback story for logging injection something like:
type ClientConfig struct {
Logger: *slog.Logger
}
func NewClient(config ClientConfig) (*Client, error) {
if config.Logger == nil {
config.Logger = slog.New(slog.DiscardHandler)
}
//...
return &Client{
logger: config.Logger,
}, nil
}
Which isn't too bad, and makes a DiscardLogger() *Logger redundant, unless you really care about those 9 characters ;)
The only reason to prefer DiscardLogger() *Logger might be that it could be created fresh on invocation, which would prevent dependencies from mucking about with slog.DiscardHandler, though the flip side is that they can already call slog.SetDefault(), and it could be useful for debugging to explicitly set slog.DiscardHandler in main(). I guess it depends on your attitude towards package-level variables.
The proposal here is to add slog.DiscardHandler as a global variable, like io.Discard, or possibly a function.
It's ready for the proposal committee.
/cc @rsc
I want to cast a vote for this being added. I found myself writing the following today in a test suite because the portion of my application I was testing relies on having a logger instance, but I don't care about logs during the tests:
package main
import "log/slog"
type NullWriter struct{}
func (NullWriter) Write([]byte) (int, error) { return 0, nil }
func main() {
logger := NullLogger()
logger.Info("foo")
}
func NullLogger() *slog.Logger {
return slog.New(slog.NewTextHandler(NullWriter{}, nil))
}
Reading through this thread I learned about io.Discard (I had searched for combination of "null" and "writer" but never would have guessed "Discard"). So that'll remove some code for me. But it would be really nice if we could invoke slog.NewNullLogger() or some such to avoid this sort of boilerplate.
@flowchartsman, can you edit your top post to refer to the actual proposal (https://github.com/golang/go/issues/62005#issuecomment-1697500900)?
Seeing this is not on track for 1.22, what should package authors do in the meantime, in case they want to provide structured logging as an optional feature?
- Implement their own no-op loggers, or
- Require users to pass a *slog.Logger (which isn't harmful per-se but can add undesirable friction)
I'd assume that a gradual ecosystem-wide migration to slog starts with package authors adding support first, enabling downstream projects to flip the switch at some point in the future.
Added https://go-review.googlesource.com/c/go/+/548335 which uses @bcmills example to demonstrate how users can solve this, for now.
Change https://go.dev/cl/548335 mentions this issue: log/slog: add example showing how to discard logs
Change https://go.dev/cl/547956 mentions this issue: log/slog: implement slog.Discard with example
I've gone ahead and submitted change 547956, just to get the ball rolling. It required I rename the internal discardHandler, (which is similar, but also allows for initialization to test allocs) to nopHandler, which is a fitting use for the discarded name, IMO, and I've gone with slog.Discard for brevity and symmetry with io.Discard, but am happy to amend.
It seemed appropriate to make a whole file example demonstrating its usage in type initialization, since the initial purpose of the proposal was to have a sensible fallback handler that types which consume a *slog.Logger could use when none is provided. Hopefully this also helps illustrate that types should accept loggers, rather than generate them.
Hey, I missed this issue while writing my own https://github.com/golang/go/issues/69227. I closed it.
As a workaround, you will see in my issue I used handler = slog.NewTextHandler(io.Discard, nil) . But I will check what @flowchartsman proposition becomes!
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group
In the log package, we talked about adding a special "discard" mode and instead we recognized log.SetOutput(io.Discard).
Here it seems like we should probably do the same thing: recognize slog.NewTextHandler(io.Discard, nil) instead of creating new API.
Thoughts?
log.SetOutput(io.Discard) is optimized to be very fast. But that optimization is fragile; if you wrap io.Discard, you don't get it. The optimization is crucial for TextHandler, because it does a lot of work if it's going to print something.
If we do this for TextHandler, it would be surprising if we didn't do it for JSONHandler as well.
Those are the built-in handlers. Would people expect the same behavior from third-party handlers? It's already hard enough to write a handler.
I think we'll end up with wordy, brittle magic formula. A single new exported symbol is a small price to pay to avoid that.
I agree that overloading TextHandler to have a short circuit for io.Discard is too magical.
For log the output contract is io.Writer so it makes sense to recognize io.Discard. Forslog, the contract is slog.Handler, discards should be more clearly visible at that level.
Prior art:
- https://pkg.go.dev/github.com/go-kit/log#NewNopLogger
- https://pkg.go.dev/go.uber.org/zap#NewNop
- https://pkg.go.dev/github.com/inconshreveable/log15/v3#DiscardHandler
- https://pkg.go.dev/github.com/rs/zerolog#Nop
In the log package, we talked about adding a special "discard" mode and instead we recognized log.SetOutput(io.Discard). Here it seems like we should probably do the same thing: recognize
slog.NewTextHandler(io.Discard, nil)instead of creating new API.Thoughts?
I think it's much more ergonomic for the package to provide this itself. As I alluded to in https://github.com/golang/go/issues/62005#issuecomment-1747630201, it is not at all obvious how to instantiate a no-output logger, particularly to people not intimately familiar with the whole (quite large) standard library. If the package provides a canonical way to do it, then it becomes a lot easier to find; people who know they want to use the logger package, but don't know all of the other things, can see it right inline with the rest of the documentation they are reading around logging and opt to use it.
I also agree with https://github.com/golang/go/issues/62005#issuecomment-2344343003 that providing an optimized no-output option is better for applications that may want logging for debug situations, but don't want to incur the costs associated with building the logs when they are not in such a situation.
My understanding of this proposal is that we want to optimize for the "discard logs" case, and could either achieve that by having every slog handler detect io.Discard, which requires O(n) changes and is potentially fragile, or we can have a specific slog.DiscardHandler and not add any extra logic to each slog handler. Is that understanding right?
If slog.DiscardHandler is a package variable of type slog.Handler, then the compiler can't constant-fold away log operations on it. It may be that that's not practical for other reasons, but I wanted to point it out as a consideration.
is there any scenario where the compiler can constant fold away log operations? all user code will call logging methods on slog.Logger which stores a reference to the handler in an interface field.
imo it's sufficient for the Handler methods to just do no further work with the arguments it receives, code that's concerned about the cost for preparing the fields to log can wrap it in if logger.Enabled(slog.LevelDebug) { ... }
@aclements That's correct.
Logging has to be fast when disabled, but that's fast relative to formatting and I/O. I don't think we need to worry about optimizations at the level of constant folding.
Have all remaining concerns about this proposal been addressed?
The proposal is:
package slog
// DiscardHandler dicards all log output.
var DiscardHandler Handler
Based on the discussion above, this proposal seems like a likely accept.
The proposal is:
package slog
// DiscardHandler dicards all log output.
var DiscardHandler Handler
The only reason to prefer
DiscardLogger() *Loggermight be that it could be created fresh on invocation, which would prevent dependencies from mucking about withslog.DiscardHandler
For the folks paranoid about security out there (like me), is it worth considering adding a constructor to get a clean discard handler constructed from the same unexported type I assume is being created to back var DiscardHandler Handler.
func NewDiscardHandler() Handler
If it's good enough for io.EOF, it's good enough for slog.DiscardHandler.
Less glibly, the threat model of Go does not include having access to the code of the program. If it did, many things about the language would be different. We do understand that writable globals are not great, but we are not going to start addressing that in slog.