Skip to content

Commit

Permalink
zapcore: Ignore nil Entry.Time in encoders (#1369)
Browse files Browse the repository at this point in the history
Per the comment on `Encoder.EncodeEntry`, any fields that are empty
including fields on the `Entry` type should be omitted. Omit the `Time`
field when we have empty time.
This also aligns with slog.Handler contract.

Refs #1334
Discovered by #1335

---------

Co-authored-by: Abhinav Gupta <mail@abhinavg.net>
  • Loading branch information
justinhwang and abhinav committed Oct 5, 2023
1 parent 54430cb commit 87577d8
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 2 deletions.
2 changes: 1 addition & 1 deletion zapcore/console_encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (c consoleEncoder) EncodeEntry(ent Entry, fields []Field) (*buffer.Buffer,
// If this ever becomes a performance bottleneck, we can implement
// ArrayEncoder for our plain-text format.
arr := getSliceEncoder()
if c.TimeKey != "" && c.EncodeTime != nil {
if c.TimeKey != "" && c.EncodeTime != nil && !ent.Time.IsZero() {
c.EncodeTime(ent.Time, arr)
}
if c.LevelKey != "" && c.EncodeLevel != nil {
Expand Down
45 changes: 45 additions & 0 deletions zapcore/console_encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package zapcore_test

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
. "go.uber.org/zap/zapcore"
Expand All @@ -35,6 +36,50 @@ var testEntry = Entry{
Caller: EntryCaller{Defined: true, File: "foo.go", Line: 42, Function: "foo.Foo"},
}

func TestConsoleEncodeEntry(t *testing.T) {
tests := []struct {
desc string
expected string
ent Entry
fields []Field
}{
{
desc: "info no fields",
expected: "2018-06-19T16:33:42Z\tinfo\tbob\tlob law\n",
ent: Entry{
Level: InfoLevel,
Time: time.Date(2018, 6, 19, 16, 33, 42, 99, time.UTC),
LoggerName: "bob",
Message: "lob law",
},
},
{
desc: "zero_time_omitted",
expected: "info\tname\tmessage\n",
ent: Entry{
Level: InfoLevel,
Time: time.Time{},
LoggerName: "name",
Message: "message",
},
},
}

cfg := testEncoderConfig()
cfg.EncodeTime = RFC3339TimeEncoder
enc := NewConsoleEncoder(cfg)

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
buf, err := enc.EncodeEntry(tt.ent, tt.fields)
if assert.NoError(t, err, "Unexpected console encoding error.") {
assert.Equal(t, tt.expected, buf.String(), "Incorrect encoded entry.")
}
buf.Free()
})
}
}

func TestConsoleSeparator(t *testing.T) {
tests := []struct {
desc string
Expand Down
2 changes: 1 addition & 1 deletion zapcore/json_encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func (enc *jsonEncoder) EncodeEntry(ent Entry, fields []Field) (*buffer.Buffer,
final.AppendString(ent.Level.String())
}
}
if final.TimeKey != "" {
if final.TimeKey != "" && !ent.Time.IsZero() {
final.AddTime(final.TimeKey, ent.Time)
}
if ent.LoggerName != "" && final.NameKey != "" {
Expand Down
14 changes: 14 additions & 0 deletions zapcore/json_encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ func TestJSONEncodeEntry(t *testing.T) {
}),
},
},
{
desc: "zero_time_omitted",
expected: `{
"L": "info",
"N": "name",
"M": "message"
}`,
ent: zapcore.Entry{
Level: zapcore.InfoLevel,
Time: time.Time{},
LoggerName: "name",
Message: "message",
},
},
}

enc := zapcore.NewJSONEncoder(zapcore.EncoderConfig{
Expand Down

0 comments on commit 87577d8

Please sign in to comment.