From 6ea0cb19f8125644448f17834010c8d62c874949 Mon Sep 17 00:00:00 2001 From: Leo Antunes Date: Wed, 9 Sep 2026 18:18:04 +0200 Subject: [PATCH] fix: propagate Tag and Context fields through Logger.With zapsentry.Tag and zapsentry.Context were only honoured when passed directly to the log call. Fields attached earlier via zap's Logger.With were silently dropped, because core.with() did not carry them into the child core and Write() only scanned the call-site fields. Store tags and the context on the core in with(), and have Write() read them from the already-computed clone. Add regression tests for both fields. --- core.go | 79 ++++++++++++++++++++++++---------------------------- core_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++++++++ field.go | 12 ++++---- 3 files changed, 119 insertions(+), 47 deletions(-) create mode 100644 core_test.go diff --git a/core.go b/core.go index 978baa5..eb3161e 100644 --- a/core.go +++ b/core.go @@ -1,7 +1,9 @@ package zapsentry import ( + "context" "errors" + "maps" "reflect" "time" @@ -76,6 +78,7 @@ func NewCore(cfg Configuration, factory SentryClientFactory) (zapcore.Core, erro }, flushTimeout: flushTimeout, fields: make(map[string]interface{}), + tags: maps.Clone(cfg.Tags), } return &core, nil @@ -110,36 +113,17 @@ func (c *core) Write(ent zapcore.Entry, fs []zapcore.Field) error { } if c.cfg.Level.Enabled(ent.Level) { - tagsCount := len(c.cfg.Tags) - for _, f := range fs { - if f.Type == zapcore.SkipType { - if _, ok := f.Interface.(tagField); ok { - tagsCount++ - } - } - } - var hint *sentry.EventHint + if clone.ctx != nil { + hint = &sentry.EventHint{Context: clone.ctx} + } event := sentry.NewEvent() event.Message = ent.Message event.Timestamp = ent.Time event.Level = sentrySeverity(ent.Level) event.Contexts["Extra"] = clone.fields - event.Tags = make(map[string]string, tagsCount) - for k, v := range c.cfg.Tags { - event.Tags[k] = v - } - for _, f := range fs { - if f.Type == zapcore.SkipType { - switch t := f.Interface.(type) { - case tagField: - event.Tags[t.Key] = t.Value - case ctxField: - hint = &sentry.EventHint{Context: t.Value} - } - } - } + maps.Copy(event.Tags, clone.tags) event.Exception = clone.createExceptions() if event.Exception == nil && !c.cfg.DisableStacktrace && c.client.Options().AttachStacktrace { @@ -266,16 +250,6 @@ func (c *core) scope() *sentry.Scope { return c.hub().Scope() } -func getScope(field zapcore.Field) *sentry.Scope { - if field.Type == zapcore.SkipType { - if scope, ok := field.Interface.(*sentry.Scope); ok && field.Key == zapSentryScopeKey { - return scope - } - } - - return nil -} - func (c *core) Sync() error { c.client.Flush(c.flushTimeout) @@ -292,23 +266,40 @@ func (c *core) with(fs []zapcore.Field) *core { copy(errs, c.errs) fields := make(map[string]interface{}, len(c.fields)+len(fs)) + maps.Copy(fields, c.fields) - for k, v := range c.fields { - fields[k] = v - } - + // tags is shared with the parent until a tag field is actually added. + tags, tagsCloned := c.tags, false + ctx := c.ctx sentryScope := c.sentryScope enc := zapcore.NewMapObjectEncoder() for _, f := range fs { f.AddTo(enc) - if f.Type == zapcore.ErrorType { + switch f.Type { + case zapcore.ErrorType: errs = append(errs, f.Interface.(error)) - } else if errSlice, ok := f.Interface.([]error); ok { - errs = append(errs, errSlice...) - } else if scope := getScope(f); scope != nil { - sentryScope = scope + case zapcore.SkipType: + switch t := f.Interface.(type) { + case *sentry.Scope: + if f.Key == zapSentryScopeKey { + sentryScope = t + } + case tagField: + if !tagsCloned { + tags = make(map[string]string, len(c.tags)+1) + maps.Copy(tags, c.tags) + tagsCloned = true + } + tags[f.Key] = string(t) + case ctxField: + ctx = t.Value + } + default: + if errSlice, ok := f.Interface.([]error); ok { + errs = append(errs, errSlice...) + } } } @@ -322,8 +313,10 @@ func (c *core) with(fs []zapcore.Field) *core { LevelEnabler: c.LevelEnabler, flushTimeout: c.flushTimeout, sentryScope: sentryScope, + ctx: ctx, errs: errs, fields: fields, + tags: tags, } } @@ -338,9 +331,11 @@ type core struct { flushTimeout time.Duration sentryScope *sentry.Scope + ctx context.Context errs []error fields map[string]interface{} + tags map[string]string } // follow same logic with sentry-go to filter unnecessary frames diff --git a/core_test.go b/core_test.go new file mode 100644 index 0000000..bd2a759 --- /dev/null +++ b/core_test.go @@ -0,0 +1,75 @@ +package zapsentry_test + +import ( + "context" + "testing" + + "github.com/TheZeroSlave/zapsentry" + "github.com/getsentry/sentry-go" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +type captured struct { + events []*sentry.Event + hints []*sentry.EventHint +} + +func newTestLogger(t *testing.T) (*zap.Logger, *captured) { + t.Helper() + c := &captured{} + client, err := sentry.NewClient(sentry.ClientOptions{ + Transport: &transport{MockSendEvent: func(e *sentry.Event) { c.events = append(c.events, e) }}, + BeforeSend: func(e *sentry.Event, hint *sentry.EventHint) *sentry.Event { + c.hints = append(c.hints, hint) + return e + }, + }) + if err != nil { + t.Fatal(err) + } + core, err := zapsentry.NewCore( + zapsentry.Configuration{Level: zapcore.ErrorLevel, DisableStacktrace: true, Tags: map[string]string{"from_cfg": "c"}}, + zapsentry.NewSentryClientFromClient(client), + ) + if err != nil { + t.Fatal(err) + } + return zap.New(core), c +} + +func TestTagViaWith(t *testing.T) { + log, c := newTestLogger(t) + + log.With(zapsentry.Tag("from_with", "a")).Error("boom", zapsentry.Tag("from_call", "b")) + + if len(c.events) != 1 { + t.Fatalf("expected 1 event, got %d", len(c.events)) + } + want := map[string]string{"from_cfg": "c", "from_with": "a", "from_call": "b"} + for k, v := range want { + if c.events[0].Tags[k] != v { + t.Errorf("tag %q: got %q, want %q (all: %v)", k, c.events[0].Tags[k], v, c.events[0].Tags) + } + } +} + +func TestContextViaWith(t *testing.T) { + log, c := newTestLogger(t) + + type ctxKey struct{} + ctx := context.WithValue(context.Background(), ctxKey{}, "marker") + + log.With(zapsentry.Context(ctx)).Error("boom") + + if len(c.hints) != 1 { + t.Fatalf("expected 1 hint, got %d", len(c.hints)) + } + hint := c.hints[0] + if hint == nil || hint.Context == nil { + t.Fatalf("With() context missing from hint: %+v", hint) + } + if got := hint.Context.Value(ctxKey{}); got != "marker" { + t.Errorf("unexpected context value: %v", got) + } +} diff --git a/field.go b/field.go index 3039012..d672359 100644 --- a/field.go +++ b/field.go @@ -7,13 +7,12 @@ import ( "go.uber.org/zap/zapcore" ) -type tagField struct { - Key string - Value string -} +type tagField string +// Tag adds a Sentry tag to the event. Tags attached via [zap.Logger.With] are +// carried over to every event logged through the derived logger. func Tag(key string, value string) zap.Field { - return zap.Field{Key: key, Type: zapcore.SkipType, Interface: tagField{key, value}} + return zap.Field{Key: key, Type: zapcore.SkipType, Interface: tagField(value)} } type ctxField struct { @@ -23,6 +22,9 @@ type ctxField struct { // Context adds a context to the logger. // This can be used e.g. to pass trace information to sentry and allow linking events to their respective traces. // +// When attached via [zap.Logger.With], the context is retained for the lifetime of the +// derived logger, so prefer passing request-scoped contexts at the log call site. +// // See also https://docs.sentry.io/platforms/go/performance/instrumentation/opentelemetry/#linking-errors-to-transactions func Context(ctx context.Context) zap.Field { return zap.Field{Key: "context", Type: zapcore.SkipType, Interface: ctxField{ctx}}