feat: slog-native Handler with context field injection and typed attrs#27
Merged
feat: slog-native Handler with context field injection and typed attrs#27
Conversation
Replace BaseLogger abstraction with a custom slog.Handler as the extension point. Native slog.LogAttrs calls now automatically get ColdBrew context fields (trace ID, gRPC method, etc.) after SetDefault wires the Handler. Key changes: - New Handler implementing slog.Handler with context field injection, caller caching, per-request level override, composable WithAttrs/WithGroup - SetDefault(h) sets global handler + slog.SetDefault for native slog support - AddAttrsToContext for typed slog.Attr context fields (zero interface boxing) - toAttr handles slog.Attr and slog.Value to prevent double-wrapping - Exported ToSlogLevel/FromSlogLevel for cross-package use - Deprecated: BaseLogger, SetLogger, NewLogger (backward compat kept) - Deleted: gokit, logrus, zap, stdlog, slog backends; gokitwrap bridge - Removed direct deps: go-kit/log, logrus, zap Performance: 16B/1alloc per log call (down from 80B/3allocs), native slog path adds ~30ns overhead over raw slog.
📝 WalkthroughWalkthroughReplaces multiple legacy backend implementations with a new exported Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant slog as slog.Logger
participant Handler as log.Handler
participant Context as loggers.Context
participant Inner as slog.Handler (JSON/Text)
participant Output
App->>slog: slog.InfoContext(ctx, "msg", attrs...)
slog->>Handler: Handle(ctx, record)
Handler->>Context: loggers.FromContext(ctx)
Context-->>Handler: context fields (slog.Attr)
Handler->>Handler: convert attrs, inject caller if enabled
Handler->>Inner: Handle(ctx, enriched record)
Inner->>Output: write JSON/text log entry
sequenceDiagram
participant App
participant API as AddAttrsToContext
participant Context as context.Context
participant slog as slog.Logger
participant Handler as log.Handler
participant Output
App->>API: AddAttrsToContext(ctx, slog.String(...))
API->>Context: store typed attrs
App->>slog: slog.InfoContext(ctx, "msg", more attrs)
slog->>Handler: Handle(ctx, record)
Handler->>Context: retrieve stored attrs
Handler->>Output: emit record containing both stored and per-call attrs
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ankurs
added a commit
to go-coldbrew/docs.coldbrew.cloud
that referenced
this pull request
Apr 11, 2026
Update all logging references to reflect the slog-native Handler: - howto/Log.md: rewrite backends section (SetDefault/NewHandler/NewHandlerWithInner), update context-aware logs to show AddAttrsToContext + slog.LogAttrs, update OverrideLogLevel example with slog - Packages.md: rewrite log package description for slog-native - Index.md: update feature table and package table - FAQ.md: SetLogger -> SetDefault - howto/production.md: fix wrong function name (AddToLogContext -> AddToContext) Ref: go-coldbrew/log#27
3 tasks
There was a problem hiding this comment.
Pull request overview
Introduces a slog-native logging architecture where a custom log.Handler (implementing slog.Handler) becomes the primary extension point, enabling automatic context field injection for native slog calls and adding a typed-slog.Attr context API.
Changes:
- Add new
log.Handler(composable wrapper around anyslog.Handler) and wire global logging vialog.SetDefault. - Update legacy slog bridge (
wrap/slogwrap) to use exported level mapping and deprecate bridge helpers in favor of the new handler. - Remove unused legacy backends (gokit/logrus/zap/stdlog/slog backend) and update benchmarks/tests/docs accordingly.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| wrap/slogwrap.go | Updates legacy slog bridge; deprecates ToSlogHandler/ToSlogLogger guidance in favor of handler-native setup. |
| wrap/slogwrap_test.go | Adjusts re-entry guard integration test to the new handler-based pipeline. |
| wrap/gokitwrap.go | Removes go-kit wrapper bridge. |
| types.go | Updates Logger interface docs; keeps BaseLogger embedding for backward compatibility. |
| loggers/zap/zap.go | Removes zap backend implementation. |
| loggers/zap/README.md | Removes generated docs for zap backend. |
| loggers/zap/example_test.go | Removes zap backend example. |
| loggers/stdlog/README.md | Removes generated docs for stdlog backend. |
| loggers/stdlog/log.go | Removes stdlog backend implementation. |
| loggers/stdlog/example_test.go | Removes stdlog backend example. |
| loggers/slog/slog.go | Removes former slog BaseLogger backend implementation. |
| loggers/slog/slog_test.go | Removes tests for former slog backend. |
| loggers/slog/README.md | Removes generated docs for former slog backend. |
| loggers/slog/example_test.go | Removes former slog backend examples. |
| loggers/logrus/README.md | Removes generated docs for logrus backend. |
| loggers/logrus/logrus.go | Removes logrus backend implementation. |
| loggers/loggers.go | Deprecates BaseLogger in favor of slog.Handler composition. |
| loggers/gokit/README.md | Removes generated docs for gokit backend. |
| loggers/gokit/gokit.go | Removes gokit backend implementation. |
| loggers/gokit/example_test.go | Removes gokit backend example. |
| log.go | Reworks global logging around *Handler, adds SetDefault/GetHandler, keeps legacy compatibility shims. |
| handler.go | Adds new log.Handler implementation with context injection, caller caching, and level mapping helpers. |
| handler_test.go | Adds tests for native slog context injection, overrides, composability, and typed attrs behavior. |
| go.mod | Drops direct deps on removed backends; keeps only required module(s). |
| go.sum | Updates sums to reflect dependency graph after backend removals. |
| example_test.go | Adds example for typed attrs usage with slog.LogAttrs. |
| documentation.go | Updates package docs to describe slog-native handler approach and typed attrs API. |
| bench_test.go | Updates benchmarks to focus on handler + native slog paths and removes deleted backend benchmarks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ankurs
added a commit
to go-coldbrew/cookiecutter-coldbrew
that referenced
this pull request
Apr 11, 2026
Replace all go-coldbrew/log calls with native slog.LogAttrs using typed attribute constructors. Demonstrates the zero-boxing high-performance logging path enabled by the slog-native Handler (go-coldbrew/log#27). - service/service.go: slog.LogAttrs + AddAttrsToContext example in Echo handler - service/auth/auth.go: slog.LogAttrs with slog.String/slog.Any - main.go: slog.LogAttrs for service exit error - config/config.go: slog.LogAttrs for config load error No go-coldbrew/log import needed except in service.go for AddAttrsToContext. Ref: go-coldbrew/log#27
1 task
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@documentation.go`:
- Around line 19-21: The examples mutate and use ctx before it's defined;
initialize a base context (e.g., via context.Background()) into a variable named
ctx before calling log.AddToContext or log.AddAttrsToContext, then reassign the
returned context; update all occurrences around log.AddToContext,
log.AddAttrsToContext, and subsequent slog.InfoContext/slog.Info calls so they
read from the newly assigned ctx rather than an undefined variable.
In `@handler_test.go`:
- Around line 28-32: Before calling SetDefault(h) in the test, capture the
previous global slog default (e.g., prev := slog.Default()), then in the
t.Cleanup restore it (slog.SetDefault(prev)) and clear the package-level
defaultHandler (defaultHandler.Store(nil)); update the cleanup used after
NewHandlerWithInner/SetDefault so it both restores slog.Default() and clears
defaultHandler to avoid leaking this test's handler into later tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2854a85-8c98-4b79-b6d6-482ed8ee0747
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (27)
bench_test.godocumentation.goexample_test.gogo.modhandler.gohandler_test.golog.gologgers/gokit/README.mdloggers/gokit/example_test.gologgers/gokit/gokit.gologgers/loggers.gologgers/logrus/README.mdloggers/logrus/logrus.gologgers/slog/README.mdloggers/slog/example_test.gologgers/slog/slog.gologgers/slog/slog_test.gologgers/stdlog/README.mdloggers/stdlog/example_test.gologgers/stdlog/log.gologgers/zap/README.mdloggers/zap/example_test.gologgers/zap/zap.gotypes.gowrap/gokitwrap.gowrap/slogwrap.gowrap/slogwrap_test.go
💤 Files with no reviewable changes (16)
- loggers/zap/example_test.go
- loggers/logrus/README.md
- loggers/gokit/README.md
- loggers/slog/README.md
- loggers/stdlog/example_test.go
- loggers/zap/README.md
- wrap/gokitwrap.go
- loggers/stdlog/README.md
- loggers/slog/example_test.go
- loggers/logrus/logrus.go
- loggers/stdlog/log.go
- loggers/gokit/gokit.go
- loggers/slog/slog_test.go
- loggers/zap/zap.go
- loggers/slog/slog.go
- loggers/gokit/example_test.go
👮 Files not reviewed due to content moderation or server errors (8)
- loggers/loggers.go
- types.go
- go.mod
- wrap/slogwrap.go
- handler.go
- log.go
- wrap/slogwrap_test.go
- example_test.go
- Fix NewHandler creating double levelVar (Copilot) — construct Handler directly instead of calling NewHandlerWithInner - Fix SetLevel not propagating to BaseLogger adapter (Copilot) — add type assertion to forward level changes to inner handler - Fix AddAttrsToContext doc overstating zero-boxing (Copilot) — clarify that context storage still goes through any, boxing is avoided at log emission time via toAttr type switch - Fix tests leaking slog.Default global state (Copilot + CodeRabbit) — extract setDefaultForTest helper that saves/restores both defaultHandler and slog.Default in t.Cleanup - Fix ExampleAddAttrsToContext missing SetDefault call (Copilot) - Fix documentation.go using undefined ctx variable (CodeRabbit) - Fix wrap test not restoring slog.Default (Copilot)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
wrap/slogwrap_test.go (1)
460-463:⚠️ Potential issue | 🟠 MajorRestore the previous
slog.Default()after resetting the ColdBrew handler.The cleanup currently restores
prevSlogand then immediately overwrites it again vialog.SetDefault(prevHandler). That leaves later tests running withslog.New(prevHandler)instead of the original logger.Suggested fix
t.Cleanup(func() { - slog.SetDefault(prevSlog) - log.SetDefault(prevHandler) + log.SetDefault(prevHandler) + slog.SetDefault(prevSlog) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wrap/slogwrap_test.go` around lines 460 - 463, The cleanup restores slog.SetDefault(prevSlog) before resetting the ColdBrew handler which causes slog.Default() to end up built from prevHandler; swap the order so you call log.SetDefault(prevHandler) first and then slog.SetDefault(prevSlog) in the t.Cleanup closure (referencing prevHandler, prevSlog, log.SetDefault and slog.SetDefault) so the original slog.Default is fully restored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@log.go`:
- Around line 228-236: The adapter currently ignores errors from the legacy
backend; in baseLoggerAdapter.Handle capture the result of a.bl.Log(ctx,
FromSlogLevel(record.Level), 0, args...) and return that error instead of always
returning nil so failures on the backward-compatibility path propagate to the
slog.Handler.Handle caller; update Handle to call a.bl.Log(...), assign to err,
and return err.
---
Duplicate comments:
In `@wrap/slogwrap_test.go`:
- Around line 460-463: The cleanup restores slog.SetDefault(prevSlog) before
resetting the ColdBrew handler which causes slog.Default() to end up built from
prevHandler; swap the order so you call log.SetDefault(prevHandler) first and
then slog.SetDefault(prevSlog) in the t.Cleanup closure (referencing
prevHandler, prevSlog, log.SetDefault and slog.SetDefault) so the original
slog.Default is fully restored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb9195f8-6ed9-4b28-8493-11c716a418bc
📒 Files selected for processing (6)
documentation.goexample_test.gohandler.gohandler_test.golog.gowrap/slogwrap_test.go
✅ Files skipped from review due to trivial changes (1)
- documentation.go
🚧 Files skipped from review as they are similar to previous changes (2)
- example_test.go
- handler.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ankurs
added a commit
to go-coldbrew/docs.coldbrew.cloud
that referenced
this pull request
Apr 12, 2026
* docs: update logging docs for slog-native architecture Update all logging references to reflect the slog-native Handler: - howto/Log.md: rewrite backends section (SetDefault/NewHandler/NewHandlerWithInner), update context-aware logs to show AddAttrsToContext + slog.LogAttrs, update OverrideLogLevel example with slog - Packages.md: rewrite log package description for slog-native - Index.md: update feature table and package table - FAQ.md: SetLogger -> SetDefault - howto/production.md: fix wrong function name (AddToLogContext -> AddToContext) Ref: go-coldbrew/log#27 * docs: add handler composability examples to Log howto Add concrete examples showing how to compose ColdBrew's Handler with custom inner handlers, slog-multi fan-out, and external middleware wrapping. All done through the log package via NewHandlerWithInner. * fix: add missing imports to code snippets in Log howto Add context, net/http, os imports to code examples so they compile when copy-pasted. Addresses Copilot review comments. * fix: correct tracing description — stats handlers, not interceptors * fix: error handling in file handler example, define logFile in fan-out * fix: clarify sampling handler is a placeholder * fix: add return to example, clarify SetDefault in wrapping pattern
ankurs
added a commit
to go-coldbrew/core
that referenced
this pull request
Apr 12, 2026
Replace BaseLogger-based setup with log.SetDefault(log.NewHandler(...)) which also calls slog.SetDefault, enabling native slog.LogAttrs calls to automatically get ColdBrew context fields. - Remove cbslog (loggers/slog) import — backend logic now in log.Handler - Bump log to v0.4.0 - SetupLogger parses level first, passes it to NewHandler in one call Ref: go-coldbrew/log#27
3 tasks
ankurs
added a commit
to go-coldbrew/core
that referenced
this pull request
Apr 12, 2026
* feat: switch SetupLogger to slog-native log.SetDefault Replace BaseLogger-based setup with log.SetDefault(log.NewHandler(...)) which also calls slog.SetDefault, enabling native slog.LogAttrs calls to automatically get ColdBrew context fields. - Remove cbslog (loggers/slog) import — backend logic now in log.Handler - Bump log to v0.4.0 - SetupLogger parses level first, passes it to NewHandler in one call Ref: go-coldbrew/log#27 * chore: update go.sum for log v0.4.0 * fix: correct error log fields in SetupLogger, add slog.Default wiring test - Fix misordered log fields: use "msg" for message, "err" for the error - Add assertion that SetupLogger wires slog.Default to *log.Handler * fix: restore ColdBrew default handler in test cleanup * feat: respect user-configured handler, bump log to v0.4.1 SetupLogger now checks log.DefaultIsSet() before overwriting — if the user called log.SetDefault in init(), core only updates the log level. - Bump log to v0.4.1 (adds DefaultIsSet) - Fix error log field ordering in SetupLogger - Add TestSetupLogger_RespectsExistingHandler * fix: restore log.SetDefault before slog.SetDefault in test cleanup
ankurs
added a commit
to go-coldbrew/cookiecutter-coldbrew
that referenced
this pull request
Apr 12, 2026
* feat: switch template logging to native slog.LogAttrs Replace all go-coldbrew/log calls with native slog.LogAttrs using typed attribute constructors. Demonstrates the zero-boxing high-performance logging path enabled by the slog-native Handler (go-coldbrew/log#27). - service/service.go: slog.LogAttrs + AddAttrsToContext example in Echo handler - service/auth/auth.go: slog.LogAttrs with slog.String/slog.Any - main.go: slog.LogAttrs for service exit error - config/config.go: slog.LogAttrs for config load error No go-coldbrew/log import needed except in service.go for AddAttrsToContext. Ref: go-coldbrew/log#27 * fix: address PR review comments - Log message length instead of raw user input in Echo handler (Copilot) - Update AGENTS.md to mention slog.LogAttrs and AddAttrsToContext (Copilot) * fix: import grouping for gofmt, clarify cblog alias in AGENTS.md * chore: bump log to v0.4.0 (slog-native Handler) * chore: bump all coldbrew deps to latest - core v0.1.45 → v0.1.51 (slog-native SetupLogger, DefaultIsSet) - log v0.3.1 → v0.4.1 (slog-native Handler, AddAttrsToContext, DefaultIsSet) - errors v0.2.13 → v0.2.15 - interceptors v0.1.20 → v0.1.25 - tracing v0.2.1 → v0.2.2 * fix: add go mod tidy -e before make mock in post-gen hook * fix: step counter 1/4 → 1/5 * fix: update hook comment to match step order, log pre-mock tidy warnings
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
BaseLoggerabstraction with a customslog.Handleras the extension pointslog.LogAttrscalls now automatically get ColdBrew context fields afterSetDefaultwires the HandlerAddAttrsToContextfor typedslog.Attrcontext fields — zero interface boxingslog.Handler, can be wrapped by slog-multi or other middlewareWithAttrs/WithGroupreturn new*Handlerinstances preserving context injectionOverrideLogLevelworks with native slog calls (previously broken)ToSlogLevel/FromSlogLevelfor cross-package usego-kit/log,logrus,zapBaseLogger,SetLogger,NewLoggerPerformance
Test plan
make test— all pass with-racemake lint— 0 issues, 0 vulnerabilitiesmake bench— performance improvedSummary by CodeRabbit