Bunch of enhancements... #21
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Makes attribute values and keys a little darker to make the messages pop more. Addresses issue phsym#9.
Uses fmt.Fprintf with "%+v" to print errors. Useful with some error packages which render additional information like stacktraces. Addresses phsym#7
Sort multiline values to the end, and print the key on a separate line
HandlerOptions.ReplaceAttr support. When ReplaceAttr is set, there will be more allocations. There is one allocation happening even when ReplaceAttr is nil, around passing the slice of groups to ReplaceAttr. Still needs to be optimized away.
Encoders are now the disposable state struct for each Handle call. A sync pool of encoders replaces the sync pool of buffers. This allows the encoder to store additional state per Handle() call, including a slice of currently open groups, which is needed to call ReplaceAttr. Benchmarking shows 0 allocations, regardless of whether ReplaceAttr is set or not. Setting a ReplaceAttr does add some overhead, similar to the slog.TextHandler. Benchmarking also shows a slight performance improvement over the original code (using the buf pool), when ReplaceAttrs is not set.
If the error doesn't implement fmt.Formatter, as most won't, its faster to use err.Error() than always use fmt to print the error.
Only the caller knows the appropriate style to use for the current context in the log line.
Blue looks good with some terminal themes, but not with the default macos theme. Cyan generally looks fine on more themes I think.
Only make the source file path relative to the current working directly if the source file is a child path of the current working directory. Typically, the source file path will only be a child of the CWD when the code is being run from its own project folder. But if the executable moved somewhere else first, or packaged in a container, or compiled with the -trimpath option, then this relative path logic doesn't work. If the file path isn't related to the CWT, then trim to just one path element above the file name, which is typically the go package name, e.g. "io/reader.go" Addresses phsym#16
Uses simple approach, allocating each header exactly opts.HeaderWidth (truncating or padding as needed). Also moves source out of the header section.
Also always treat the source as just another attribute. It can be put in the headers with opts.Headers
Also print the headers before the log level. Makes it easier to scan.
Some io.Writers may be expecting each log to equate to a single call
Replacing Headers with a formatting syntax, which lets the user compose the header section of the log like however they like.
Encoder methods beginning with "encode" write things to known buffers in the encoder, and do not take a buffer as an arg. "write" methods are lower level, and always take a buffer as an argument.
- the header field now has to reference the full path to the attr, like "group1.group2.attrKey" - header attributes are captured after being resolved, and after handling groups - header attribute capturing is now handled entirely inside the encoder - the "source" attr is now always added without a group
Spaces are still not collapsing correctly though.
Still not handling cases like `[%t %m]`, you end up with `[t ]`, `[ m]`, or `[ ]`. I'd really like it so that a space between two fields is elided if either field is elided.
Instead just modified the default theme.
For the most part, we're just using this as a byte slice, not like a bytes.Buffer(), so we don't really need to emulate that API. Just treat it as a byte slice where possible.
This was referenced Feb 17, 2025
Closed
Closed
Just generally favor trimming space over preserving it, since this handler is primarily concerned with readability
%a verb inserts the attributes (these were implicitly added to the end of the log line before, but now must be explicit) %s verb inserts the source. If it is not present in the format, source is just treated like another attribute.
Styles any fixed strings inside the group. defaults to the header style
It's just a simple struct with fields now. Much easier for consumers to create their own themes.
Handlers don't necessarily *need* to synchronize writes, as the underlying writer itself may handle this, but in practice, most of the slog handlers are doing this, include the builtin handlers. Particularly since this handler isn't performance focussed and isn't intended to be used in production, better safe than sorry.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I kept going in circles with the figuring out the optimal combination of headers, fixed size headers, and how to handle the source field.
So I decided to instead implement an alternative option which lets the developer design their own header format using a format syntax.
This merge request is the tip of a fork I'm using, which either includes or supercedes some of the other pull requests I've opened. Here's a summary of what this fork does, relative to the base main branch of this project:
HeaderFormatoption, which allows for configuration the layout of the entire header section of the log line (everything before the attrs are printed). Also supports fixed width and right-aligned headers. The syntax of the format is similar to fmt.Printf()By default, the HeaderFormat is configured to exactly mimic the standard layout of this project. The default format is expressed like this:
That reads like:
The "group" stuff allows strings (like
>) to be omitted if all the headers in that group are also omitted.The new HeaderFormat allows for many customizations:
The "source" attr can be presented as a header (as in the standard layout), or as an attribute
Other attrs can be pulled out of the attrs list and injected into the headers, just like the source, e.g.
The order of the headers (including the timestamp, level, and message) can be completely reconfigured by the user, e.g.
The
>separator can be swapped for other characters, or moved, headers can be enclosed in brackets, etc., e.g.For details, see the docs for
HandlerOptions.HeaderFormat.All this flexibility does come at a cost:
Theme.Source()toTheme.Header(), since that style is used for all header values.