go: proposal: log/slog: add an 'error' field to 'Record'

This proposal is to discuss adding an error field to Record, and potentially adding associated methods to slog and Logger.

Example adding error to Record:

type Record struct {
	// The time at which the output method (Log, Info, etc.) was called.
	Time time.Time

	// The log message.
	Message string

	// The level of the event.
	Level Level

	// The error associated with the event, if any.
	Err error // <-- this is new.  This could potentially be an `any` instead of an `error`.

// etc...
}

Possible new methods:

func (l *Logger) WithErr(err error) *Logger {
	// Returns a copy of Logger with the error field filled in
}
// Err returns an Attr for an error.
func Err(v error) Attr {
	return Attr{internalErrorKey, ErrValue(v)}
}

// internalErrorKey is private on purpose, and the string could be any unlikely string.
// When an Attr comes in with this key, the value replaces the Record's Err field instead of being added to the array of regular attributes.
const internalErrorKey = "!ERROR!!"

// ErrValue returns a Value for an error.
func ErrValue(v error) Value {
	return Value{any: v} // Could be optimized?
}

Advantages of doing this proposal:

  • Handlers can choose how to handle and display errors.
  • Handlers can choose whatever key they want, if any, and it will affect all errors.
  • Eliminates confusion for what key to use when adding an error.
  • Eliminates possibility of libraries choosing different keys. Instead there are no keys, and the handler chooses what if any key to use when displaying.

Other comments:

  • Custom error can still implment log.LogValuer, fmt.Stringer, and/or json.Marshaler, and the Handler can still choose how to handle that.
  • Deduplication of message keys can be handled in the exact same was as the existing Time, Level, and Message fields on Record.
  • Errors are just as important and special as messages and timestamps and levels.
  • By doing this proposal, we avoid a situation where 1 single application, that uses multiple libraries, ends up having log output looking like this because each library chose a different key to use for error types:
{
	"msg": "library1",
	"error": "db: bad connection"
}
{
	"msg": "library2",
	"err": "duplicate login detected"
}
{
	"msg": "library3",
	"_ERROR": "divide by zero"
}

With this proposal, the handler would choose what the key is, or if there even is a key. This puts control back where it should be for something as important as errors. If the log aggregation system I am using wants messages or errors to come in with a specific key or in a specific format, I can accomodate that with a custom handler. This lets me have consistent logs, even for libraries and sub-libraries. Like this:

{
	"msg": "library1",
	"err": "db: bad connection"
}
{
	"msg": "library2",
	"err": "duplicate login detected"
}
{
	"msg": "library3",
	"err": "divide by zero"
}

thanks, Chris

https://github.com/golang/go/issues/56345 and @jba

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Reactions: 7
  • Comments: 16 (11 by maintainers)

Most upvoted comments

This would be really nice. After coming from Logrus and Zap, I find myself missing a WithErr or even just a dedicated error attribute type, both for fewer key presses and the desire to make sure errors are always logged and aggregated the same way.

I have currently have a wrapper like this in every project using slog:

func ErrAttr(err error) slog.Attr {
	return slog.Any("error", err)
}

My conclusion from reading the original log/slog proposal is that tying the error to slog.Error has been rejected for fair reasons. But I haven’t been convinced about declining this proposal.

My point is, I think it’s too limiting to have a Record.Err field. There are too many ways to log errors for slog to take a position. That is not the case for the other built-in fields (except for the minor question of what to do with an empty message).

This seems like a valid point, I personally have never had the need to do anything much different than using my own custom ErrAttr function, since usually errors are wrapped and propagated upwards, meaning, it’s almost always a very clear case of how we’re logging the error. My argument is that, considering that error is a first class citizen, and a primitive of the language, and the fact that so many people tried typing slog.Error() as the attribute and then we’re left googling to figure out why this isn’t included in the library. It goes against, at least to me, my mental model of the language. Also, considering many third-party libraries ( that we’re the go-to standard prior to log/slog introduction, notably Zap and Zerolog ), provide this proposal out of the box.

Which brings up a question: how would your proposal handle multiple errors in a log call?

Since the current proposal already leaves you to implement this yourself/ to write your own error attr in some way or form, why wouldn’t the multiple errors be treated the same? I’d argue that the case for multiple errors can be treated like error attribute is treated at the moment, write your own solution for handling them (eg writing your own ErrsAttribute(...error) slog.Attr) if you need special handling, like differentiating them, if not, the default behavior will occur. I haven’t had enough of those situations to say what kind of output I’d expect, but error wrapping is something that people should expect from the concentration of errors (and from 1.20 I believe, wrapping errors became even easier)

Error attribute also does not tie the implementation to any logging level, so you’re free to mix and match.

Having said all of that, I do agree that the error attribute does not have a clear cut solution like the others, but I cannot help but feel like I am hacking and piecing together what I would expect to be part of the library when writing my own error attributes, especially since I have to put it in some module and reach out for it every time, even though I have log/slog already imported in scope.

Please do correct me, and tell me if I am missing something, but this has been nagging me ever since I tried I tried the library some while ago. Can anyone point me to the current recommendation on how to approach error logging within the current iteration of the log/slog, are there any guidelines available?

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group