Skip to content

slog: rewrite level synchronization with Logrus - #21

Open
thaJeztah wants to merge 5 commits into
containerd:mainfrom
thaJeztah:rewrite_slog
Open

slog: rewrite level synchronization with Logrus#21
thaJeztah wants to merge 5 commits into
containerd:mainfrom
thaJeztah:rewrite_slog

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Aug 13, 2026

Copy link
Copy Markdown
Member

slog: rewrite level synchronization with Logrus

Use the Logrus slog level adapter as a dynamic slog.Leveler instead of
maintaining a separate slog.LevelVar and duplicating the Logrus-to-slog
level mapping.

This keeps the Logrus logger as the single source of truth for the configured
level, so changes made through SetLevel or directly on L.Logger are reflected
by slog handlers automatically.

Also avoid formatting Logrus entries that are forwarded to slog and then
discarded.

slog: remove duplicate level mapping test

Remove TestLogrusToSlogLevel now that containerd uses the level adapter from
logrus/hooks/slog directly.

The mapping itself is covered by the Logrus package, while containerd's
integration tests verify that forwarded records use the expected slog levels
and that filtering follows the configured Logrus level.

slog: improve bridge test coverage

Verify that Logrus records are forwarded with the expected slog levels, and
that slog filtering follows changes to the Logrus logger level.

Cover both SetLevel and direct changes to L.Logger to ensure the dynamic level
adapter remains in sync without separate level state.

log: accept slog-compatible levels in SetLevel

Extend SetLevel to accept slog.Level and the level adapter types provided
by logrus/hooks/slog, in addition to strings and Level values.

Keep logrus.Level as the canonical representation. Map slog-backed levels
through logrus/hooks/slog, while accepting Logrus-backed levels directly.

Add coverage for all accepted level representations across the supported
log levels.

Comment thread slog.go
Comment on lines 36 to 42
slogOnce.Do(func() {
L.Logger.SetNoLock()
L.Logger.AddHook(slogHook{})
L.Logger.SetFormatter(discardFormatter{})
slogOut = L.Logger.Out
L.Logger.SetOutput(io.Discard)
slogLevel.Set(logrusToSlogLevel(L.Logger.GetLevel()))
})

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially this could also do the reverse, and use the Handler that logrus has if we want to keep logrus for the formatting;

slog.SetDefault(slog.New(
	logrusslog.NewHandler(L.Logger, nil),
))

@thaJeztah
thaJeztah force-pushed the rewrite_slog branch 3 times, most recently from ce8c72f to 7fd77f3 Compare August 14, 2026 17:45
@thaJeztah
thaJeztah marked this pull request as ready for review August 14, 2026 17:46
Comment thread slog.go Outdated
slogOnce.Do(func() {
L.Logger.SetNoLock()
L.Logger.AddHook(slogHook{})
L.Logger.SetFormatter(discardFormatter{})

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that we logrus was still configured to do the formatting, even it it wasn't used.

@thaJeztah
thaJeztah force-pushed the rewrite_slog branch 2 times, most recently from 5a292b7 to 683ef7d Compare August 14, 2026 18:18
Use the Logrus slog level adapter as a dynamic slog.Leveler instead of
maintaining a separate slog.LevelVar and duplicating the Logrus-to-slog
level mapping.

This keeps the Logrus logger as the single source of truth for the configured
level, so changes made through SetLevel or directly on L.Logger are reflected
by slog handlers automatically.

Also avoid formatting Logrus entries that are forwarded to slog and then
discarded.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When slog output is enabled, SetFormat reconfigures the slog handler but
could leave logrus formatting or output enabled if either had been changed
after UseSlog.

Reset the logrus formatter and output when updating the slog format so
logrus remains only the forwarding path and does not duplicate work or
output.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove TestLogrusToSlogLevel now that containerd uses the level adapter from
logrus/hooks/slog directly.

The mapping itself is covered by the Logrus package, while containerd's
integration tests verify that forwarded records use the expected slog levels
and that filtering follows the configured Logrus level.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Verify that Logrus records are forwarded with the expected slog levels, and
that slog filtering follows changes to the Logrus logger level.

Cover both SetLevel and direct changes to L.Logger to ensure the dynamic level
adapter remains in sync without separate level state.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Extend SetLevel to accept slog.Level and the level adapter types provided
by logrus/hooks/slog, in addition to strings and Level values.

Keep logrus.Level as the canonical representation. Map slog-backed levels
through logrus/hooks/slog, while accepting Logrus-backed levels directly.

Add coverage for all accepted level representations across the supported
log levels.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment thread context.go
// underlying Logrus level.
//
// SetLevel returns an error if a string level is not supported.
func SetLevel[T levelValue](level T) error {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated after #22 was merged; this now accepts both logrus and slog levels, as well as strings;

type levelValue interface {
    string | Level | slog.Level | lslog.Level | lslog.SlogLevel
}

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