-
Notifications
You must be signed in to change notification settings - Fork 24
Description
We had an issue that was ultimately caused by a bad slog call that panicked. Panicking on a log call is pretty severe and unfriendly---and generally should be reserved for cases where the system is left in an inconsistent or dangerous state.
The panicking call was like
logger.Error(ctx, "something bad happened: %v", err)
This is not the correct calling format for slog, but is very similar to return fmt.Errorf and how other logging libraries work. With the rise of LLM-based code generation, this kind of thing is increasingly easy to get wrong.
Slog's logging calls take ...any in the arguments, so there is nothing the Go compiler can do to ensure you're calling it right.
One thing we could consider is making the calls take ...Field. This would allow the compiler to catch errors like the one above, but would be breaking change and we'd loose support for calling like
logger.Info(ctx, "dug a hole", "depth", depth, "diameter", diameter)
This form is quite rare in github.com/coder/coder so would be a cheap migration to clean up.
Alternatively, we could modify the logging so that it doesn't panic, and instead emits a CRITICAL log noting the bad logging call instead. This keeps backward compatibility, but at the cost of much less visibility --- problems can only be caught at runtime, and only if you are paying attention to the logs.