Add ContextMap method to LoggedEntry which returns fields as a map #490
Conversation
Tests often care about specific fields, or want fields without worrying
about order, or marshalling objects etc. It makes more sense for tests
to compare a fields `map[string]interface{}` rather than `[]zap.Field`
prashantv
force-pushed
the
observer_fields
branch
from
40b15b3 to
83a3834
Compare
Codecov Report
@@ Coverage Diff @@
## master #490 +/- ##
=======================================
+ Coverage 96.9% 97% +0.1%
=======================================
Files 36 37 +1
Lines 2197 2240 +43
=======================================
+ Hits 2129 2173 +44
+ Misses 59 58 -1
Partials 9 9
Continue to review full report at Codecov.
|
zaptest/observer/fields.go
Outdated
| f.AddTo(encoder) | ||
| } | ||
| return encoder.Fields | ||
| } |
There was a problem hiding this comment.
Love the implementation. Would this be more ergonomic as a ContextMap method on LoggedEntry?
There was a problem hiding this comment.
I'd prefer to keep this separate from LoggedEntry since it doesn't have the message or timestamp.
I'm open to ContextMap, however any underlying nested objects would use map[string]interface{}. If we're OK with that, I can make this a new ContextMap type.
There was a problem hiding this comment.
Clarified offline - since the []zapcore.Field usually comes from LoggedEntry.Context, we're going to start by exposing this as LoggedEntry.ContextMap().
There was a problem hiding this comment.
Misunderstood the comment, moved this method to be a function on LoggedEntry
prashantv
changed the title
Tests often care about specific fields, or want fields without worrying
about order, or marshalling objects etc. It makes more sense for tests
to compare a fields
map[string]interface{}rather than[]zap.Field