Add Objects and ObjectValues field constructors #1071
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a new Zap field constructor that encodes an array of zero or more
Zap-marshalable objects into a Zap array.
Usage:
// Given,
//
// func (*User) MarshalLogObject(zapcore.ObjectEncoder) error
zap.Objects("foo", []*User{u1, u2, u3})
Before this, users would have to resort to constructing a `[]*User`
wrapper, and use Zap's `Array` constructor:
// Before:
type users []*User
func (uu users) MarshalLogArray(enc zapcore.ArrayEncoder) error a
for _, u := range uu {
if err := enc.AppendObject(u); err != nil {
return err
}
}
return nil
}
zap.Array("foo", users{u1, u2, u3})
Credit to ztstewart for the suggestion.
Refs GO-618
Reviewed-At: D7294727
Reviewed-By: ztstewart
Codecov Report
@@ Coverage Diff @@
## master #1071 +/- ##
==========================================
+ Coverage 98.19% 98.30% +0.10%
==========================================
Files 48 49 +1
Lines 2111 2120 +9
==========================================
+ Hits 2073 2084 +11
+ Misses 29 28 -1
+ Partials 9 8 -1
Continue to review full report at Codecov.
|
In the previous change, we added a Objects field constructor to support
logging arrays of any object supported by zap.Object.
func (*User) MarshalLogObject(...) error { ... }
var users []*User = ...
zap.Object("users", users)
That solution is incomplete because it does not cover the case when,
instead of a `[]*User`, we have a `[]User` despite the method being on
`*User`.
This change adds a similar ObjectValues field constructor that handles the
case where we have a `[]T` but the MarshalLogObject is on `*T`.
This works by declaring a new type constraint objectMarshalerPtr.
`objectMarshalerPtr[T]` is true for a type that is a pointer to `T`,
and implements zapcore.ObjectMarshaler.
Note that we still do not cover the case where the method is on the value
receiver (`func (User) MarshalLogObject(..) error`) and we have a
`[]*User`. We can add that when it becomes necessary.
Reviewed-At: D7295739
Reviewed-By: ztstewart
abhinav
force-pushed
the
abg/objects
branch
2 times, most recently
from
bb9b132
to
a648660
Compare
sywhang
approved these changes
Mar 22, 2022
Minor optimization to avoid copying the struct to a variable. Refs GO-618 Reviewed-At: D7297933 Reviewed-By: abhinav
staticcheck is expected to have support for generics-based code in a few weeks. Disable it until then.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds two new field constructors for slices, parameterized
over the types of the values in the slices:
This turns a slice of any Zap marshalable object into a Zap field.
This is a variant of Objects for the case where
*Timplementszapcore.ObjectMarshalerbut we have a[]T.Together, these two field constructors obviate the need for writing
ArrayMarshalerimplementations for slices of objects:Backwards compatibility
Both new field constructors are hidden behind a build tag, but use of type
parameters requires us to bump the
godirective in thego.modfile to 1.18.Note that this does not break Zap on Go 1.17.
Per the documentation for the
godirective,It only breaks our ability to run
go mod tidyinside Zap from Go 1.17,which is okay because with the
godirective, we're stating that we'redeveloping Zap while on Go 1.18.
Linting
staticcheck support for type parameters is expected in a few weeks.
Meanwhile, this disables staticcheck for the
make linttarget.(I considered switching linting to Go 1.17 until then, but that isn't
enough because the formatting linter expects valid Go code--which type
parameters aren't in 1.17.)