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

fix: built-in attributes are not grouped #30

Merged
merged 2 commits into from
Aug 21, 2023
Merged

fix: built-in attributes are not grouped #30

merged 2 commits into from
Aug 21, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Aug 21, 2023

Built-in attributes are not considered part of a logger's group
as specified by WithGroup.
Doing so means that if a user makes a log statement like the following,

log.WithGroup("foo").Info("bar")

ReplaceAttr will be called with groups = [foo] for all built-in keys,
and therefore the included code sample for creating a logger
without the time key will not work.

This change fixes invocations of ReplaceAttr to call it with
an empty groups slice for built-in attributes.

This matches the behavior of log/slog's default handlers
which set groups = nil for the built-in attributes (src)
before restoring it for non-built-ins (src).

Includes tests for omitting all built-in attributes.

abhinav and others added 2 commits August 21, 2023 05:48
Built-in attributes are not considered part of a logger's group
as specified by `WithGroup`.
Doing so means that if a user makes a log statement like the following,

    log.WithGroup("foo").Info("bar")

ReplaceAttr will be called with `groups = [foo]` for all built-in keys,
and therefore the [included code sample][1] for creating a logger
without the time key will not work.

  [1]: https://github.com/lmittmann/tint#customize-attributes

This change fixes invocations of ReplaceAttr to call it with
an empty groups slice for built-in attributes.

This matches the behavior of `log/slog`'s default handlers
which set `groups = nil` for the built-in attributes ([src][2])
before restoring it for non-built-ins ([src][3]).

  [2]: https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/log/slog/handler.go;l=265-266
  [3]: https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/log/slog/handler.go;l=300-301

Includes tests for omitting all built-in attributes.
@lmittmann lmittmann merged commit 8c0ec83 into lmittmann:main Aug 21, 2023
@lmittmann
Copy link
Owner

Thanks for the fix!

@abhinav abhinav deleted the pre-built-ungrouped branch August 21, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants