golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.94k stars 17.53k forks source link

log/slog: documentation passes nil contexts, which is discouraged #61219

Closed dominikh closed 1 year ago

dominikh commented 1 year ago

The documentation of the slog package passes nil contexts, for example:

logger.LogAttrs(nil, slog.LevelInfo, "hello", slog.Int("count", 3))

However, the context package discourages this:

Do not pass a nil Context, even if a function permits it. Pass context.TODO if you are unsure about which Context to use.

slog internally turns nils into context.Background when calling methods on slog.Handler, which is probably good for resilience, but probably isn't something users should rely on.

In particular, such calls will currently trigger Staticcheck's SA1012 and I'm not sure slog warrants an exception.

I see from https://github.com/golang/exp/commit/f0f767cdffd6c3fe9fa17c685f39b1cf8e866c0c that this is intentional, so this issue might be contentious. However, I'm not sure the "make them easier to write" argument is as important now, considering there are helpers for pre-defined log levels that don't take context arguments, and it would be unfortunate for the standard library to ignore its own rules.

/cc @jba

gopherbot commented 1 year ago

Change https://go.dev/cl/508437 mentions this issue: log/slog: replace nil contexts with context.Background()