Skip to content

Conversation

@MarcoPolo
Copy link
Collaborator

@MarcoPolo MarcoPolo commented Aug 9, 2025

closes #3362

This is the first step in a larger refactor to be able to pass in a custom slog down to all components and get rid of the global logger.

Also removes the ipfs/go-log dependency with a gologshim. It makes slog behave close enough to ipfs/go-log with respect to environment variables. Users who are parsing the text version will notice differences, but the JSON version is identical except the caller source location key.

Users can use gologshim from go-libp2p to remove their dependency on ipfs/go-log and migrate to slog as well.

The patch is straightforward if not large. The general rules are:

  • If we were doing some form of logf, change the text to the structured key-value pairs.
  • logw becomes just log
  • Try to normalize the log attribute keys.

The most interesting thing to review here is likely the shim itself. But a once over on the log changes would be appreciated.

Comment on lines +31 to +44
ReplaceAttr: func(_ []string, a slog.Attr) slog.Attr {
if a.Key == slog.TimeKey {
// ipfs go-log uses "ts" for time
a.Key = "ts"
} else if a.Key == slog.LevelKey {
// ipfs go-log uses lowercase level names
if lvl, ok := a.Value.Any().(slog.Level); ok {
if s, ok := lvlToLower[lvl]; ok {
a.Value = s
}
}
}
return a
},
Copy link
Member

Choose a reason for hiding this comment

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

Should we just remove this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What is this? The change in key and values? I want to keep this the same if users are already processing the logs (I expect some ipfs users are)

@@ -1,25 +1,54 @@
package canonicallog
Copy link
Member

Choose a reason for hiding this comment

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

Not needed in this PR, but should we just remove this package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe. I'm not sure if anyone is using this. The original idea behind this was to allow connecting libp2p applications to things like fail2ban via the logger. I'd leave it unless there's a good reason to remove it.

@MarcoPolo MarcoPolo force-pushed the marco/structured-logging branch from bb6736f to 947b0a8 Compare August 21, 2025 21:18
@MarcoPolo MarcoPolo merged commit da810a1 into master Aug 25, 2025
10 of 11 checks passed
lidel added a commit that referenced this pull request Oct 25, 2025
#3364 migrated from zap to slog
but accidentally changed the log level for http.Server.ErrorLog from
implicit INFO to explicit ERROR. This caused client EOF and TLS handshake
errors to spam error logs and stdout in apps which log only ERROR by
default.

These http.Server errors (client EOFs, TLS handshake failures from clients
with naive TLS implementations, connection timeouts from clients that abort
early) are normal operational noise, not actual server errors. Using
LevelDebug:
- matches semantic meaning (similar to existing connection timeout logs)
- respects user's configured threshold (default ERROR filters them out)
- allows users to enable for debugging via log level configuration

Fixes ipfs/kubo#11027
Fixes ipfs/kubo#11033
lidel added a commit that referenced this pull request Oct 27, 2025
The migration to slog in #3364
broke go-log's ability to adjust subsystem log levels at runtime via
SetLogLevel() and control output formatting (colors, json). This was
a key debugging feature that allowed changing log verbosity without
restarting daemons.

This fix detects go-log's slog bridge via duck typing and uses it when
available, restoring dynamic level control and unified formatting.
When go-log isn't present, gologshim falls back to standalone slog
handlers as before.

The lazy handler pattern solves initialization order issues where
package-level loggers are created before go-log's bridge is installed.

Users just need to update to this version of go-libp2p and go-log with
ipfs/go-log#176 - no code changes or init()
hacks required.

Prerequisite for addressing ipfs/kubo#11035
lidel added a commit to ipfs/kubo that referenced this pull request Oct 28, 2025
update go-log to v2.8.3-0.20251027235339-5fe47a4191be (feat/slog-interop)
update go-libp2p to v0.44.1-0.20251027235033-ea2c010ece2d (feat/gologshim-use-slog-default)

these changes restore dynamic log level control and tail for go-libp2p
subsystems after the migration to slog, fixing the regression introduced
in libp2p/go-libp2p#3364

Fixes #11035
MarcoPolo pushed a commit that referenced this pull request Oct 28, 2025
#3364 migrated from zap to slog
but accidentally changed the log level for http.Server.ErrorLog from
implicit INFO to explicit ERROR. This caused client EOF and TLS handshake
errors to spam error logs and stdout in apps which log only ERROR by
default.

These http.Server errors (client EOFs, TLS handshake failures from clients
with naive TLS implementations, connection timeouts from clients that abort
early) are normal operational noise, not actual server errors. Using
LevelDebug:
- matches semantic meaning (similar to existing connection timeout logs)
- respects user's configured threshold (default ERROR filters them out)
- allows users to enable for debugging via log level configuration

Fixes ipfs/kubo#11027
Fixes ipfs/kubo#11033
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.

Migrate to slog

3 participants