Replace SlogMessageOnlyHandler with attribute replacement scheme
#1065
Conversation
Here, replace with the utility `SlogMessageOnlyHandler` that we use in
example tests with a `ReplaceAttr` implementation instead so that we use
a standard `TextHandler` with a `ReplaceAttr`:
``` diff
- Logger: slog.New(&slogutil.SlogMessageOnlyHandler{Level: slog.LevelWarn}),
+ Logger: slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelWarn, ReplaceAttr: slogutil.NoLevelTimeJobID})),
```
The major benefit of this approach is that in case of an error (where
the error detail gets put in a key of the output log), we don't end up
hiding the detail and making the error difficult to debug without
removing the message-only handler.
It also has the nominal advantage of being a little more standard, i.e.
Uses normal `slog` handlers instead of a custom one, and is a little
more succinct to write.
|
|
||
| riverClient, err := river.NewClient(riverpgxv5.New(dbPool), &river.Config{ | ||
| Logger: slog.New(&slogutil.SlogMessageOnlyHandler{Level: slog.LevelWarn}), | ||
| Logger: slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelWarn, ReplaceAttr: slogutil.NoLevelTime})), |
There was a problem hiding this comment.
I might prefer to still wrap this in a helper to a) avoid potential drift as examples are added/updated and b) allow some space for a comment about why this uses ReplaceAttr which most users won't want/need. Up to you though, I do see the advantage of not obscuring the example code behind more indirection.
There was a problem hiding this comment.
Yeah, let's leave for now. I kinda like how this just shows raw options instead of hiding things in a helper, and it's really only three options it's configuring total, with 2/3 very standard. The comment on why ReplaceAttr is on slogutil.NoLevelTime in case anyone cares to jump-to and read it.
| // We have to use a specialized ReplacedAttr without level or | ||
| // timestamps to make test output reproducible, but in reality | ||
| // this would as simple as something like: | ||
| // | ||
| // return slog.NewJSONHandler(w, nil) |
There was a problem hiding this comment.
yeah this is what I was getting at with context for why we have special attrs passed in, seems helpful here though it adds a few lines
|
Thanks! |
Here, replace with the utility
SlogMessageOnlyHandlerthat we use inexample tests with a
ReplaceAttrimplementation instead so that we usea standard
TextHandlerwith aReplaceAttr:The major benefit of this approach is that in case of an error (where
the error detail gets put in a key of the output log), we don't end up
hiding the detail and making the error difficult to debug without
removing the message-only handler.
It also has the nominal advantage of being a little more standard, i.e.
Uses normal
sloghandlers instead of a custom one, and is a littlemore succinct to write.