Add custom error serialization support and provide sane defaults #78
Conversation
| @@ -18,6 +18,11 @@ var eventPool = &sync.Pool{ | |||
| }, | |||
| } | |||
|
|
|||
| // ErrorMarshalFunc allows customization of global error marshaling | |||
| var ErrorMarshalFunc = func (err error) interface{} { | |||
There was a problem hiding this comment.
You could have a nil default and skip the type assertion in the default state. It might improve performance when people not using this feature (to be tested though).
There was a problem hiding this comment.
I'll write a benchmark and post results.
There was a problem hiding this comment.
So here's the benchmark https://gist.github.com/DusanKasan/6eb7ef0b4e784bbd26a32df0073ec9cc.
Results on my machine:
BenchmarkCalling-4 20000000 66.7 ns/op 23 B/op 1 allocs/op
BenchmarkCheckingAndCalling-4 20000000 67.5 ns/op 23 B/op 1 allocs/op
BenchmarkCheckingAndNotCalling-4 20000000 65.8 ns/op 23 B/op 1 allocs/op
Nil handling would be a marginal speedup in default cases, however if you change the value from nil to something else you'll pay the price of that if that does the checking (again it's marginal so it shouldn't bother no one).
I don't know how to judge this really. However having a nil value as a default may be helpful also because user can't shoot himself in the foot now. In the original design if you assign nil to ErrorMarshalFunc it would panic on invocation.
Thoughts?
| for _, err := range errs { | ||
| marshaled := ErrorMarshalFunc(err) | ||
| switch m := marshaled.(type) { | ||
| case LogObjectMarshaler: |
There was a problem hiding this comment.
Probably not, my reasoning is that nil will be handled by the default case in which it is serialized using the interface method (which will serialize it as "null").
|
@rs what is needed/missing for this to be merged? Would be nice to roll this out asap :) |
As per #9 and to offer a different approach from #11 and #35 this PR introduces custom error serialization with sane defaults without breaking the existing APIs.
This is just a first draft and is missing tests. Also, a bit of code duplication which I feel could be reduced but it serves to get the idea across.
It provides global error marshalling by exposing a
var ErrorMarshalFunc func(error) interface{}in zerolog package that by default is a function that returns the passed argument. It should be overriden if you require custom error marshalling.Then in every function that accept error or array of errors
ErrorMarshalFuncis called on the error and then the result of it is processed like this:LogObjectMarshaler, serialize it as an objectError()The side effect of this change is that the encoders don't need the
AppendError/smethods anymore, as the errors are serialized directly to other types.