Support non-standard, rich error types #325
Conversation
|
@akshayjshah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @imkira, @peter-edge and @osamingo to be potential reviewers. |
|
LGTM |
There was a problem hiding this comment.
While this is a dependency free way to use pkg/errors, I'm not sure we should do this since the error and stacktrace are under a single key, which will make it harder to filter on a single error when using something like ELK.
If anything, I think the error key should always be just the error message, and we might have a second field added if the type implements formatter, and possibly only if the formatted error message is different.
|
Certainly don't mind doing it that way. I'll update. |
zapcore/field.go
Outdated
| val := f.Interface.(error) | ||
| if fancy, ok := val.(fmt.Formatter); ok { | ||
| // This is a rich error type, like those produced by | ||
| // github.com/pkg/errors. |
There was a problem hiding this comment.
Suggest verifying that the verbose error is actually different from the error message before adding.
It won't cost much extra (since the string length check will probably fail first)
There was a problem hiding this comment.
Sure, done - forgot that you suggested that in the first round.
This necessitates updating all dependencies.
Users who take the time (and performance hit) of using Dave Cheney's errors package should get rich, annotated errors and stacktraces in their logs. However, that package is only one of a number of similar packages hasn't yet committed to a stable API, so we don't want to take a direct dependency on it. To support these sorts of packages, serialize errors that implement `fmt.Formatter` differently from those that don't.
Users who take the time (and performance hit) of using Dave Cheney's errors
package should get rich, annotated errors and stacktraces in their logs.
However, that package is only one of a number of similar packages and hasn't yet
committed to a stable API, so we don't want to take a direct dependency on it.
To support these sorts of packages, serialize errors that implement
fmt.Formatterdifferently from those that don't. This is necessarily slower thanserializing standard errors, but users of these packages have already committed
to a more expensive error-handling path.
@sectioneight, this should make UberFX's logs nicer.
Fixes #303.