From f785f2a6d5908bc261ad7b86b08a8a52647dd4ac Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 14 Mar 2017 09:44:07 -0700 Subject: [PATCH] Add a shorter EncodeCaller variant --- benchmarks/zap_test.go | 3 +-- config.go | 4 ++-- zapcore/encoder.go | 20 +++++++++++++++----- zapcore/encoder_test.go | 8 +++++--- zapcore/entry.go | 39 ++++++++++++++++++++++++++++++++++++--- zapcore/entry_test.go | 29 +++++++++++++++++++++-------- 6 files changed, 80 insertions(+), 23 deletions(-) diff --git a/benchmarks/zap_test.go b/benchmarks/zap_test.go index c42667f0b..e0f2b08e6 100644 --- a/benchmarks/zap_test.go +++ b/benchmarks/zap_test.go @@ -69,8 +69,7 @@ var _jane = user{ func newZapLogger(lvl zapcore.Level) *zap.Logger { // use the canned production encoder configuration - cfg := zap.NewProductionConfig() - enc := zapcore.NewJSONEncoder(cfg.EncoderConfig) + enc := zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig()) return zap.New(zapcore.NewCore( enc, &zaptest.Discarder{}, diff --git a/config.go b/config.go index 1382b1b7f..a198f16ea 100644 --- a/config.go +++ b/config.go @@ -89,7 +89,7 @@ func NewProductionEncoderConfig() zapcore.EncoderConfig { EncodeLevel: zapcore.LowercaseLevelEncoder, EncodeTime: zapcore.EpochTimeEncoder, EncodeDuration: zapcore.SecondsDurationEncoder, - EncodeCaller: zapcore.FullPathCallerEncoder, + EncodeCaller: zapcore.ShortCallerEncoder, } } @@ -127,7 +127,7 @@ func NewDevelopmentEncoderConfig() zapcore.EncoderConfig { EncodeLevel: zapcore.CapitalLevelEncoder, EncodeTime: zapcore.ISO8601TimeEncoder, EncodeDuration: zapcore.StringDurationEncoder, - EncodeCaller: zapcore.FullPathCallerEncoder, + EncodeCaller: zapcore.ShortCallerEncoder, } } diff --git a/zapcore/encoder.go b/zapcore/encoder.go index 34a40dd2d..54a814423 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -165,18 +165,28 @@ func (e *DurationEncoder) UnmarshalText(text []byte) error { // A CallerEncoder serializes an EntryCaller to a primitive type. type CallerEncoder func(EntryCaller, PrimitiveArrayEncoder) -// FullPathCallerEncoder serializes a caller in /full/path/file:line format. -func FullPathCallerEncoder(caller EntryCaller, enc PrimitiveArrayEncoder) { +// FullCallerEncoder serializes a caller in /full/path/to/package/file:line +// format. +func FullCallerEncoder(caller EntryCaller, enc PrimitiveArrayEncoder) { // TODO: consider using a byte-oriented API to save an allocation. enc.AppendString(caller.String()) } -// UnmarshalText unmarshals text to a CallerEncoder. -// FIXME: Support more options. +// ShortCallerEncoder serializes a caller in package/file:line format, trimming +// all but the final directory from the full path. +func ShortCallerEncoder(caller EntryCaller, enc PrimitiveArrayEncoder) { + // TODO: consider using a byte-oriented API to save an allocation. + enc.AppendString(caller.TrimmedPath()) +} + +// UnmarshalText unmarshals text to a CallerEncoder. "full" is unmarshaled to +// FullCallerEncoder and anything else is unmarshaled to ShortCallerEncoder. func (e *CallerEncoder) UnmarshalText(text []byte) error { switch string(text) { + case "full": + *e = FullCallerEncoder default: - *e = FullPathCallerEncoder + *e = ShortCallerEncoder } return nil } diff --git a/zapcore/encoder_test.go b/zapcore/encoder_test.go index 8ef7277f3..adb9f1fe3 100644 --- a/zapcore/encoder_test.go +++ b/zapcore/encoder_test.go @@ -53,7 +53,7 @@ func testEncoderConfig() EncoderConfig { EncodeTime: EpochTimeEncoder, EncodeLevel: LowercaseLevelEncoder, EncodeDuration: SecondsDurationEncoder, - EncodeCaller: FullPathCallerEncoder, + EncodeCaller: ShortCallerEncoder, } } @@ -490,8 +490,10 @@ func TestCallerEncoders(t *testing.T) { name string expected interface{} // output of serializing caller }{ - {"", "/home/jack/src/github.com/foo/foo.go:42"}, - {"something-random", "/home/jack/src/github.com/foo/foo.go:42"}, + {"", "foo/foo.go:42"}, + {"something-random", "foo/foo.go:42"}, + {"short", "foo/foo.go:42"}, + {"full", "/home/jack/src/github.com/foo/foo.go:42"}, } for _, tt := range tests { diff --git a/zapcore/entry.go b/zapcore/entry.go index b580141ab..9bab935b2 100644 --- a/zapcore/entry.go +++ b/zapcore/entry.go @@ -22,6 +22,8 @@ package zapcore import ( "fmt" + "os" + "strings" "sync" "time" @@ -74,11 +76,16 @@ type EntryCaller struct { Line int } -// String returns a "file:line" string if the EntryCaller is Defined, and the -// empty string otherwise. +// String returns the full path and line number of the caller. func (ec EntryCaller) String() string { + return ec.FullPath() +} + +// FullPath returns a /full/path/to/package/file:line description of the +// caller. +func (ec EntryCaller) FullPath() string { if !ec.Defined { - return "" + return "undefined" } buf := bufferpool.Get() buf.AppendString(ec.File) @@ -89,6 +96,32 @@ func (ec EntryCaller) String() string { return caller } +// TrimmedPath returns a package/file:line description of the caller, +// preserving only the leaf directory name and file name. +func (ec EntryCaller) TrimmedPath() string { + if !ec.Defined { + return "undefined" + } + // Find the last separator. + idx := strings.LastIndexByte(ec.File, os.PathSeparator) + if idx == -1 { + return ec.FullPath() + } + // Find the penultimate separator. + idx = strings.LastIndexByte(ec.File[:idx], os.PathSeparator) + if idx == -1 { + return ec.FullPath() + } + buf := bufferpool.Get() + // Keep everything after the penultimate separator. + buf.AppendString(ec.File[idx+1:]) + buf.AppendByte(':') + buf.AppendInt(int64(ec.Line)) + caller := buf.String() + bufferpool.Put(buf) + return caller +} + // An Entry represents a complete log message. The entry's structured context // is already serialized, but the log level, time, message, and call site // information are available for inspection and modification. diff --git a/zapcore/entry_test.go b/zapcore/entry_test.go index 1198b5db9..569c4e1e0 100644 --- a/zapcore/entry_test.go +++ b/zapcore/entry_test.go @@ -59,18 +59,31 @@ func TestPutNilEntry(t *testing.T) { func TestEntryCaller(t *testing.T) { tests := []struct { - ok bool - want EntryCaller - str string + caller EntryCaller + full string + short string }{ - {true, EntryCaller{PC: 100, Defined: true, File: "foo.go", Line: 42}, "foo.go:42"}, - {false, EntryCaller{}, ""}, + { + caller: NewEntryCaller(100, "/path/to/foo.go", 42, false), + full: "undefined", + short: "undefined", + }, + { + caller: NewEntryCaller(100, "/path/to/foo.go", 42, true), + full: "/path/to/foo.go:42", + short: "to/foo.go:42", + }, + { + caller: NewEntryCaller(100, "to/foo.go", 42, true), + full: "to/foo.go:42", + short: "to/foo.go:42", + }, } for _, tt := range tests { - caller := NewEntryCaller(100, "foo.go", 42, tt.ok) - assert.Equal(t, tt.want, caller, "Unexpected output from NewEntryCaller.") - assert.Equal(t, tt.str, caller.String(), "Unexpected string output from EntryCaller") + assert.Equal(t, tt.full, tt.caller.String(), "Unexpected string from EntryCaller.") + assert.Equal(t, tt.full, tt.caller.FullPath(), "Unexpected FullPath from EntryCaller.") + assert.Equal(t, tt.short, tt.caller.TrimmedPath(), "Unexpected TrimmedPath from EntryCaller.") } }