Skip to content

Commit

Permalink
fix(otelzap): withoptions(zap.fields(...)) will keep fields with logger
Browse files Browse the repository at this point in the history
  • Loading branch information
ezh committed Jan 25, 2022
1 parent baea276 commit 5e91392
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 0 deletions.
39 changes: 39 additions & 0 deletions otelzap/fieldextractor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package otelzap

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

// fieldExtractorCore copy zapcore.Fields from With method to extraFields list
type fieldExtractorCore struct {
extraFields *[]zap.Field
}

var _ zapcore.Core = (*fieldExtractorCore)(nil)

// With adds structured context to the Core.
func (fe *fieldExtractorCore) With(fs []zapcore.Field) zapcore.Core {
*fe.extraFields = append(*fe.extraFields, fs...)
return nil
}

// Check stub
func (*fieldExtractorCore) Check(zapcore.Entry, *zapcore.CheckedEntry) *zapcore.CheckedEntry {
return nil
}

// Write stub
func (*fieldExtractorCore) Write(zapcore.Entry, []zapcore.Field) error {
return nil
}

// Sync stub
func (*fieldExtractorCore) Sync() error {
return nil
}

// Enabled stub
func (*fieldExtractorCore) Enabled(zapcore.Level) bool {
return false
}
16 changes: 16 additions & 0 deletions otelzap/otelzap.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ type Logger struct {

caller bool
stackTrace bool

// extraFields contains a number of zap.Fields that are added to every log entry
extraFields []zap.Field
}

func New(logger *zap.Logger, opts ...Option) *Logger {
Expand All @@ -59,8 +62,13 @@ func New(logger *zap.Logger, opts ...Option) *Logger {
// WithOptions clones the current Logger, applies the supplied Options,
// and returns the resulting Logger. It's safe to use concurrently.
func (l *Logger) WithOptions(opts ...zap.Option) *Logger {
extraFields := []zap.Field{}
// zap.New side effect is extracting fields from .WithOptions(zap.Fields(...))
zap.New(&fieldExtractorCore{extraFields: &extraFields}, opts...)
clone := *l
clone.Logger = l.Logger.WithOptions(opts...)
clone.skipCaller = l.skipCaller.WithOptions(opts...)
clone.extraFields = append(clone.extraFields, extraFields...)
return &clone
}

Expand Down Expand Up @@ -149,6 +157,14 @@ func (l *Logger) logFields(
attrs = appendField(attrs, f)
}

for _, f := range l.extraFields {
if f.Type == zapcore.NamespaceType {
// should this be a prefix?
continue
}
attrs = appendField(attrs, f)
}

l.log(span, lvl, msg, attrs)

if l.withTraceID {
Expand Down
33 changes: 33 additions & 0 deletions otelzap/otelzap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,39 @@ func TestOtelZap(t *testing.T) {
requireCodeAttrs(t, m)
},
},
{
log: func(ctx context.Context, log *Logger) {
log.Ctx(ctx).
WithOptions(zap.Fields(zap.String("baz", "baz1"))).
WithOptions(zap.Fields(zap.String("faz", "faz1"))).
Warn("hello", zap.Strings("foo", []string{"bar1", "bar2", "bar3"}))
},
require: func(t *testing.T, event sdktrace.Event) {
m := attrMap(event.Attributes)

sev, ok := m[logSeverityKey]
require.True(t, ok)
require.Equal(t, "WARN", sev.AsString())

msg, ok := m[logMessageKey]
require.True(t, ok)
require.Equal(t, "hello", msg.AsString())

foo, ok := m["foo"]
require.True(t, ok)
require.Equal(t, []string{"bar1", "bar2", "bar3"}, foo.AsStringSlice())

baz, ok := m["baz"]
require.True(t, ok)
require.Equal(t, "baz1", baz.AsString())

faz, ok := m["faz"]
require.True(t, ok)
require.Equal(t, "faz1", faz.AsString())

requireCodeAttrs(t, m)
},
},
{
log: func(ctx context.Context, log *Logger) {
log.Ctx(ctx).Warn("hello", zap.Durations("foo", []time.Duration{time.Millisecond, time.Second, time.Hour}))
Expand Down

0 comments on commit 5e91392

Please sign in to comment.