Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Stringers field constructor for array of objects implementing fmt.Stringer method #1155

Merged
merged 1 commit into from Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 33 additions & 1 deletion array_go118.go
Expand Up @@ -23,7 +23,11 @@

package zap

import "go.uber.org/zap/zapcore"
import (
"fmt"

"go.uber.org/zap/zapcore"
)

// Objects constructs a field with the given key, holding a list of the
// provided objects that can be marshaled by Zap.
Expand Down Expand Up @@ -122,3 +126,31 @@ func (os objectValues[T, P]) MarshalLogArray(arr zapcore.ArrayEncoder) error {
}
return nil
}

// Stringers constructs a field with the given key, holding a list of the
// output provided by the value's String method
//
// Given an object that implements String on the value receiver, you
Comment on lines +130 to +133
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same caveat as Objects applies here. We should add a comment about it:

// Note that these objects must implement fmt.Stringer directly.
// That is, if you're trying to marshal a []Request, the String method
// must be declared on the Request type, not its pointer (*Request).

In a future change, we can add an ObjectValues equivalent if necessary that will let the String method be on *Request for a []Request.

// can log a slice of those objects with Objects like so:
//
// type Request struct{ ... }
// func (a Request) String() string
//
// var requests []Request = ...
// logger.Info("sending requests", zap.Stringers("requests", requests))
//
// Note that these objects must implement fmt.Stringer directly.
// That is, if you're trying to marshal a []Request, the String method
// must be declared on the Request type, not its pointer (*Request).
func Stringers[T fmt.Stringer](key string, values []T) Field {
return Array(key, stringers[T](values))
}

type stringers[T fmt.Stringer] []T

func (os stringers[T]) MarshalLogArray(arr zapcore.ArrayEncoder) error {
for _, o := range os {
arr.AppendString(o.String())
}
return nil
}
59 changes: 59 additions & 0 deletions array_go118_test.go
Expand Up @@ -25,6 +25,7 @@ package zap

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -179,3 +180,61 @@ func TestObjectsAndObjectValues_marshalError(t *testing.T) {
})
}
}

type stringerObject struct {
value string
}

func (s stringerObject) String() string {
return s.value
}

func TestStringers(t *testing.T) {
t.Parallel()

tests := []struct {
desc string
give Field
want []any
}{
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add a test for a []fmt.Stringer? That should also work.

desc: "Stringers",
give: Stringers("", []stringerObject{
{value: "foo"},
{value: "bar"},
{value: "baz"},
}),
want: []any{
"foo",
"bar",
"baz",
},
},
{
desc: "Stringers with []fmt.Stringer",
give: Stringers("", []fmt.Stringer{
stringerObject{value: "foo"},
stringerObject{value: "bar"},
stringerObject{value: "baz"},
}),
want: []any{
"foo",
"bar",
"baz",
},
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.desc, func(t *testing.T) {
t.Parallel()

tt.give.Key = "k"

enc := zapcore.NewMapObjectEncoder()
tt.give.AddTo(enc)
assert.Equal(t, tt.want, enc.Fields["k"])
})
}
}