From 80d8c30ee8c483568a6a2be174048b435181caab Mon Sep 17 00:00:00 2001 From: Jacek Olszak Date: Wed, 2 Feb 2022 16:24:47 +0100 Subject: [PATCH] Remove Entry.With method This method is not needed because the end result is the same as built-in append function. --- adapter/glogadapter/glog_test.go | 3 ++- adapter/internal/adaptertest/adaptertest.go | 11 ++++++---- logger/_examples/caller/main.go | 4 ++-- logger/_examples/tags/main.go | 7 +++---- logger/adapter.go | 17 +++------------ logger/global.go | 3 ++- logger/logger.go | 2 +- logger/logger_test.go | 23 --------------------- 8 files changed, 20 insertions(+), 50 deletions(-) diff --git a/adapter/glogadapter/glog_test.go b/adapter/glogadapter/glog_test.go index 9b5e282..81e5f67 100644 --- a/adapter/glogadapter/glog_test.go +++ b/adapter/glogadapter/glog_test.go @@ -59,7 +59,8 @@ func TestAdapter_Log(t *testing.T) { entry := logger.Entry{ Level: logger.ErrorLevel, Message: message, - }.With(logger.Field{ + } + entry.Fields = append(entry.Fields, logger.Field{ Key: "k", Value: "v", }) diff --git a/adapter/internal/adaptertest/adaptertest.go b/adapter/internal/adaptertest/adaptertest.go index 4edc6a0..124daee 100644 --- a/adapter/internal/adaptertest/adaptertest.go +++ b/adapter/internal/adaptertest/adaptertest.go @@ -154,7 +154,8 @@ func Run(t *testing.T, subject Subject) { // nolint for name, field := range fields { var builder strings.Builder adapter := subject.NewAdapter(&builder) - entryWithField := entry.With( + entryWithField := entry + entryWithField.Fields = append(entryWithField.Fields, logger.Field{ Key: name, Value: field.value, @@ -175,13 +176,14 @@ func Run(t *testing.T, subject Subject) { // nolint stringFieldValue = "string" intFieldValue = 2 ) - entryWithFields := entry.With( + entryWithFields := entry + entryWithFields.Fields = append(entryWithFields.Fields, logger.Field{ Key: "StringField", Value: stringFieldValue, }, ) - entryWithFields = entryWithFields.With( + entryWithFields.Fields = append(entryWithFields.Fields, logger.Field{ Key: "IntField", Value: intFieldValue, @@ -202,7 +204,8 @@ func Run(t *testing.T, subject Subject) { // nolint stringFieldValue = "string" err = "err" ) - entryWithFieldAndError := entry.With( + entryWithFieldAndError := entry + entryWithFieldAndError.Fields = append(entryWithFieldAndError.Fields, logger.Field{ Key: "StringField", Value: stringFieldValue, diff --git a/logger/_examples/caller/main.go b/logger/_examples/caller/main.go index 7147817..5d848b9 100644 --- a/logger/_examples/caller/main.go +++ b/logger/_examples/caller/main.go @@ -43,8 +43,8 @@ func (a ReportCallerAdapter) Log(ctx context.Context, entry logger.Entry) { entry.SkippedCallerFrames++ // each middleware adapter must additionally skip one frame (at least) if _, file, line, ok := runtime.Caller(entry.SkippedCallerFrames); ok { - entry = entry.With(logger.Field{Key: "file", Value: file}) - entry = entry.With(logger.Field{Key: "line", Value: line}) + entry.Fields = append(entry.Fields, logger.Field{Key: "file", Value: file}) + entry.Fields = append(entry.Fields, logger.Field{Key: "line", Value: line}) } a.NextAdapter.Log(ctx, entry) diff --git a/logger/_examples/tags/main.go b/logger/_examples/tags/main.go index 6b55f2e..dda6c6d 100644 --- a/logger/_examples/tags/main.go +++ b/logger/_examples/tags/main.go @@ -34,13 +34,12 @@ type AddFieldFromContextAdapter struct { } func (a AddFieldFromContextAdapter) Log(ctx context.Context, entry logger.Entry) { - // entry.With creates an entry and adds a new field to it - newEntry := entry.With( + entry.Fields = append(entry.Fields, logger.Field{ Key: tag, Value: ctx.Value(tag), }, ) - newEntry.SkippedCallerFrames++ // each middleware adapter must additionally skip one frame - a.NextAdapter.Log(ctx, newEntry) + entry.SkippedCallerFrames++ // each middleware adapter must additionally skip one frame + a.NextAdapter.Log(ctx, entry) } diff --git a/logger/adapter.go b/logger/adapter.go index 350fc11..649cb56 100644 --- a/logger/adapter.go +++ b/logger/adapter.go @@ -20,9 +20,9 @@ type Entry struct { // Fields contains all accumulated fields, in the order they were appended. // - // Please do not modify the slice directly as other go-routines may still read it. To append a new field - // please use With method. It will create a new entry with copy of all fields, plus the new one. To remove a field, - // please create a new slice and copy remaining fields. + // Please do not update fields in the slice as other go-routines may still read them. To add a new field + // please use append built-in function. To remove a field, please create a new slice and copy remaining fields. + // To update field first remove it and then add a new one. // // Fields can be nil. Fields []Field @@ -32,17 +32,6 @@ type Entry struct { SkippedCallerFrames int } -// With creates a new entry with additional field. -func (e Entry) With(field Field) Entry { - newLen := len(e.Fields) + 1 - fields := make([]Field, newLen) - copy(fields, e.Fields) - e.Fields = fields - e.Fields[newLen-1] = field - - return e -} - // Level is a severity level of message. Use Level.MoreSevereThan to compare two levels. type Level int8 diff --git a/logger/global.go b/logger/global.go index 4b4863b..b31ce2e 100644 --- a/logger/global.go +++ b/logger/global.go @@ -94,7 +94,8 @@ func (g *Global) Error(ctx context.Context, msg string) { // With creates a new child logger with field. func (g *Global) With(key string, value interface{}) *Global { - newEntry := g.entry.With(Field{Key: key, Value: value}) + newEntry := g.entry + newEntry.Fields = append(newEntry.Fields, Field{Key: key, Value: value}) return &Global{ entry: newEntry, diff --git a/logger/logger.go b/logger/logger.go index 7c53391..076757d 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -59,7 +59,7 @@ func (l Logger) Error(ctx context.Context, msg string) { // With creates a new logger with field. func (l Logger) With(key string, value interface{}) Logger { - l.entry = l.entry.With(Field{key, value}) + l.entry.Fields = append(l.entry.Fields, Field{Key: key, Value: value}) return l } diff --git a/logger/logger_test.go b/logger/logger_test.go index 6f54030..9769768 100644 --- a/logger/logger_test.go +++ b/logger/logger_test.go @@ -311,29 +311,6 @@ func TestGlobal_WithError(t *testing.T) { } } -func TestEntry_With(t *testing.T) { - field1 := logger.Field{Key: "k1", Value: "v1"} - field2 := logger.Field{Key: "k2", Value: "v2"} - - t.Run("should add field to empty entry", func(t *testing.T) { - entry := logger.Entry{} - newEntry := entry.With(field1) - assert.Empty(t, entry.Fields) - require.Len(t, newEntry.Fields, 1) - assert.Equal(t, newEntry.Fields[0], field1) - }) - - t.Run("should add field to entry with one field", func(t *testing.T) { - entry := logger.Entry{}.With(field1) - newEntry := entry.With(field2) - require.Len(t, entry.Fields, 1) - require.Len(t, newEntry.Fields, 2) - assert.Equal(t, entry.Fields[0], field1) - assert.Equal(t, newEntry.Fields[0], field1) - assert.Equal(t, newEntry.Fields[1], field2) - }) -} - func TestLevel_MoreSevereThan(t *testing.T) { t.Run("should return true", func(t *testing.T) { assert.True(t, logger.InfoLevel.MoreSevereThan(logger.DebugLevel))