Skip to content

Commit f057a8d

Browse files
giortzisggiortzisg
authored
fix: ensure correct signal delivery on multi-client setups (#1190)
1 parent 76dd512 commit f057a8d

File tree

4 files changed

+232
-31
lines changed

4 files changed

+232
-31
lines changed

‎client_test.go‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/google/go-cmp/cmp"
1818
"github.com/google/go-cmp/cmp/cmpopts"
1919
pkgErrors "github.com/pkg/errors"
20+
"github.com/stretchr/testify/assert"
2021
"github.com/stretchr/testify/require"
2122
)
2223

@@ -1054,3 +1055,185 @@ func TestClient_SetupTelemetryBuffer_NoDSN(t *testing.T) {
10541055
t.Fatalf("expected noopTransport, got %T", client.Transport)
10551056
}
10561057
}
1058+
1059+
type multiClientEnv struct {
1060+
client1, client2 *Client
1061+
transport1, transport2 *MockTransport
1062+
hub1, hub2 *Hub
1063+
ctx1, ctx2 context.Context
1064+
traceID1, traceID2 TraceID
1065+
}
1066+
1067+
func setupMultiClientEnv(t *testing.T) *multiClientEnv {
1068+
t.Helper()
1069+
mkClient := func(dsn string) (*Client, *MockTransport) {
1070+
tr := &MockTransport{}
1071+
c, err := NewClient(ClientOptions{
1072+
Dsn: dsn,
1073+
Transport: tr,
1074+
EnableLogs: true,
1075+
Integrations: func(_ []Integration) []Integration {
1076+
return []Integration{}
1077+
},
1078+
})
1079+
require.NoError(t, err)
1080+
return c, tr
1081+
}
1082+
1083+
e := &multiClientEnv{}
1084+
e.client1, e.transport1 = mkClient("https://public@example.com/sentry/1")
1085+
e.client2, e.transport2 = mkClient("https://public@example.com/sentry/2")
1086+
e.traceID1 = TraceIDFromHex("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1")
1087+
e.traceID2 = TraceIDFromHex("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb2")
1088+
1089+
scope1 := NewScope()
1090+
scope1.SetPropagationContext(PropagationContext{TraceID: e.traceID1})
1091+
scope2 := NewScope()
1092+
scope2.SetPropagationContext(PropagationContext{TraceID: e.traceID2})
1093+
1094+
e.hub1 = NewHub(e.client1, scope1)
1095+
e.hub2 = NewHub(e.client2, scope2)
1096+
e.ctx1 = SetHubOnContext(context.Background(), e.hub1)
1097+
e.ctx2 = SetHubOnContext(context.Background(), e.hub2)
1098+
1099+
t.Cleanup(func() {
1100+
e.client1.Close()
1101+
e.client2.Close()
1102+
})
1103+
return e
1104+
}
1105+
1106+
func (e *multiClientEnv) resetTransports() {
1107+
e.transport1.mu.Lock()
1108+
e.transport1.events = nil
1109+
e.transport1.mu.Unlock()
1110+
e.transport2.mu.Lock()
1111+
e.transport2.events = nil
1112+
e.transport2.mu.Unlock()
1113+
}
1114+
1115+
func (e *multiClientEnv) flushAll() {
1116+
e.client1.Flush(5 * time.Second)
1117+
e.client2.Flush(5 * time.Second)
1118+
}
1119+
1120+
func eventTraceID(t *testing.T, ev *Event) TraceID {
1121+
t.Helper()
1122+
traceCtx, ok := ev.Contexts["trace"]
1123+
require.True(t, ok, "event should have a trace context")
1124+
tid, ok := traceCtx["trace_id"].(TraceID)
1125+
require.True(t, ok, "trace context should contain a TraceID")
1126+
return tid
1127+
}
1128+
1129+
func TestClient_MultiClientSetup(t *testing.T) {
1130+
t.Run("signals_route_to_correct_client", func(t *testing.T) {
1131+
e := setupMultiClientEnv(t)
1132+
1133+
e.hub1.CaptureMessage("msg-from-client1")
1134+
e.hub2.CaptureMessage("msg-from-client2")
1135+
1136+
require.Len(t, e.transport1.Events(), 1)
1137+
require.Len(t, e.transport2.Events(), 1)
1138+
assert.Equal(t, "msg-from-client1", e.transport1.Events()[0].Message)
1139+
assert.Equal(t, "msg-from-client2", e.transport2.Events()[0].Message)
1140+
assert.Equal(t, e.traceID1, eventTraceID(t, e.transport1.Events()[0]),
1141+
"event on client1 should carry hub1's trace ID")
1142+
assert.Equal(t, e.traceID2, eventTraceID(t, e.transport2.Events()[0]),
1143+
"event on client2 should carry hub2's trace ID")
1144+
e.resetTransports()
1145+
1146+
NewLogger(e.ctx1).Info().WithCtx(e.ctx1).Emit("log-from-client1")
1147+
NewLogger(e.ctx2).Info().WithCtx(e.ctx2).Emit("log-from-client2")
1148+
e.flushAll()
1149+
1150+
require.Len(t, e.transport1.Events(), 1, "client1 transport should have 1 log event")
1151+
require.Len(t, e.transport2.Events(), 1, "client2 transport should have 1 log event")
1152+
require.Len(t, e.transport1.Events()[0].Logs, 1)
1153+
require.Len(t, e.transport2.Events()[0].Logs, 1)
1154+
assert.Equal(t, "log-from-client1", e.transport1.Events()[0].Logs[0].Body)
1155+
assert.Equal(t, "log-from-client2", e.transport2.Events()[0].Logs[0].Body)
1156+
assert.Equal(t, e.traceID1, e.transport1.Events()[0].Logs[0].TraceID,
1157+
"log on client1 should carry hub1's trace ID")
1158+
assert.Equal(t, e.traceID2, e.transport2.Events()[0].Logs[0].TraceID,
1159+
"log on client2 should carry hub2's trace ID")
1160+
e.resetTransports()
1161+
1162+
NewMeter(e.ctx1).Count("counter-from-client1", 1)
1163+
NewMeter(e.ctx2).Count("counter-from-client2", 2)
1164+
e.flushAll()
1165+
1166+
require.Len(t, e.transport1.Events(), 1, "client1 transport should have 1 metric event")
1167+
require.Len(t, e.transport2.Events(), 1, "client2 transport should have 1 metric event")
1168+
require.Len(t, e.transport1.Events()[0].Metrics, 1)
1169+
require.Len(t, e.transport2.Events()[0].Metrics, 1)
1170+
assert.Equal(t, "counter-from-client1", e.transport1.Events()[0].Metrics[0].Name)
1171+
assert.Equal(t, "counter-from-client2", e.transport2.Events()[0].Metrics[0].Name)
1172+
})
1173+
1174+
t.Run("signals_respect_emit_context_client", func(t *testing.T) {
1175+
e := setupMultiClientEnv(t)
1176+
logger := NewLogger(e.ctx1)
1177+
meter := NewMeter(e.ctx1)
1178+
logger.Info().WithCtx(e.ctx2).Emit("cross-context-log")
1179+
meter.WithCtx(e.ctx2).Count("cross-context-count", 1)
1180+
e.flushAll()
1181+
1182+
assert.Empty(t, e.transport1.Events(),
1183+
"creation-time client should NOT receive the log when emit context points elsewhere")
1184+
require.Len(t, e.transport2.Events(), 2,
1185+
"emit-context client should receive the signals")
1186+
require.Len(t, e.transport2.Events()[0].Logs, 1)
1187+
require.Len(t, e.transport2.Events()[1].Metrics, 1)
1188+
assert.Equal(t, "cross-context-log", e.transport2.Events()[0].Logs[0].Body)
1189+
assert.Equal(t, "cross-context-count", e.transport2.Events()[1].Metrics[0].Name)
1190+
assert.Equal(t, e.traceID2, e.transport2.Events()[0].Logs[0].TraceID,
1191+
"trace ID should come from the emit context's hub, not the creation context")
1192+
})
1193+
1194+
t.Run("signals_follow_bind_client", func(t *testing.T) {
1195+
e := setupMultiClientEnv(t)
1196+
1197+
traceID := TraceIDFromHex("cccccccccccccccccccccccccccccccc")
1198+
scope := NewScope()
1199+
scope.SetPropagationContext(PropagationContext{TraceID: traceID})
1200+
hub := NewHub(e.client1, scope)
1201+
ctx := SetHubOnContext(context.Background(), hub)
1202+
logger := NewLogger(ctx)
1203+
meter := NewMeter(ctx)
1204+
1205+
hub.BindClient(e.client2)
1206+
1207+
hub.CaptureMessage("event-after-rebind")
1208+
logger.Info().WithCtx(ctx).Emit("log-after-rebind")
1209+
meter.WithCtx(ctx).Count("count-after-rebind", 1)
1210+
e.flushAll()
1211+
1212+
assert.Empty(t, e.transport1.Events(),
1213+
"old client should not receive any signals after BindClient")
1214+
require.Len(t, e.transport2.Events(), 3,
1215+
"new client should receive all signals")
1216+
1217+
var gotEvent, gotLog, gotMetric bool
1218+
for _, ev := range e.transport2.Events() {
1219+
if ev.Message == "event-after-rebind" {
1220+
gotEvent = true
1221+
assert.Equal(t, traceID, eventTraceID(t, ev),
1222+
"event should carry the hub's trace ID")
1223+
}
1224+
if len(ev.Logs) == 1 && ev.Logs[0].Body == "log-after-rebind" {
1225+
gotLog = true
1226+
assert.Equal(t, traceID, ev.Logs[0].TraceID,
1227+
"log should carry the hub's trace ID")
1228+
}
1229+
if len(ev.Metrics) == 1 && ev.Metrics[0].Name == "count-after-rebind" {
1230+
gotMetric = true
1231+
assert.Equal(t, traceID, ev.Metrics[0].TraceID,
1232+
"count should carry the hub's trace ID")
1233+
}
1234+
}
1235+
assert.True(t, gotEvent, "event should arrive at new client")
1236+
assert.True(t, gotLog, "log should arrive at new client")
1237+
assert.True(t, gotMetric, "count should arrive at new client")
1238+
})
1239+
}

‎log.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ var mapTypesToStr = map[attribute.Type]AttrType{
4343

4444
type sentryLogger struct {
4545
ctx context.Context
46-
client *Client
46+
hub *Hub
4747
attributes map[string]Attribute
4848
defaultAttributes map[string]Attribute
4949
mu sync.RWMutex
@@ -91,7 +91,7 @@ func NewLogger(ctx context.Context) Logger { // nolint: dupl
9191

9292
return &sentryLogger{
9393
ctx: ctx,
94-
client: client,
94+
hub: hub,
9595
attributes: make(map[string]Attribute),
9696
defaultAttributes: defaultAttrs,
9797
mu: sync.RWMutex{},
@@ -113,7 +113,17 @@ func (l *sentryLogger) log(ctx context.Context, level LogLevel, severity int, me
113113
return
114114
}
115115

116-
scope, traceID, spanID := resolveScopeAndTrace(ctx, l.ctx)
116+
hub := hubFromContexts(ctx, l.ctx)
117+
if hub == nil {
118+
hub = l.hub
119+
}
120+
client := hub.Client()
121+
if client == nil {
122+
return
123+
}
124+
125+
scope := hub.Scope()
126+
traceID, spanID := resolveTrace(scope, ctx, l.ctx)
117127

118128
// Pre-allocate with capacity hint to avoid map growth reallocations
119129
estimatedCap := len(l.defaultAttributes) + len(entryAttrs) + len(args) + 8 // scope ~3 + instance ~5
@@ -156,9 +166,8 @@ func (l *sentryLogger) log(ctx context.Context, level LogLevel, severity int, me
156166
Attributes: attrs,
157167
}
158168

159-
l.client.captureLog(log, scope)
160-
161-
if l.client.options.Debug {
169+
client.captureLog(log, scope)
170+
if client.options.Debug {
162171
debuglog.Printf(message, args...)
163172
}
164173
}

‎metrics.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func NewMeter(ctx context.Context) Meter {
7979

8080
return &sentryMeter{
8181
ctx: ctx,
82-
client: client,
82+
hub: hub,
8383
attributes: make(map[string]Attribute),
8484
defaultAttributes: defaultAttrs,
8585
mu: sync.RWMutex{},
@@ -92,7 +92,7 @@ func NewMeter(ctx context.Context) Meter {
9292

9393
type sentryMeter struct {
9494
ctx context.Context
95-
client *Client
95+
hub *Hub
9696
attributes map[string]Attribute
9797
defaultAttributes map[string]Attribute
9898
mu sync.RWMutex
@@ -104,10 +104,21 @@ func (m *sentryMeter) emit(ctx context.Context, metricType MetricType, name stri
104104
return
105105
}
106106

107-
scope, traceID, spanID := resolveScopeAndTrace(ctx, m.ctx)
107+
hub := hubFromContexts(ctx, m.ctx)
108+
if hub == nil {
109+
hub = m.hub
110+
}
111+
112+
client := hub.Client()
113+
if client == nil {
114+
return
115+
}
116+
117+
scope := hub.Scope()
108118
if customScope != nil {
109119
scope = customScope
110120
}
121+
traceID, spanID := resolveTrace(scope, ctx, m.ctx)
111122

112123
// Pre-allocate with capacity hint to avoid map growth reallocations
113124
estimatedCap := len(m.defaultAttributes) + len(attributes) + 8 // scope ~3 + call-specific ~5
@@ -140,7 +151,7 @@ func (m *sentryMeter) emit(ctx context.Context, metricType MetricType, name stri
140151
Attributes: attrs,
141152
}
142153

143-
if m.client.captureMetric(metric, scope) && m.client.options.Debug {
154+
if client.captureMetric(metric, scope) && client.options.Debug {
144155
debuglog.Printf("Metric %s [%s]: %v %s", metricType, name, value.AsInterface(), unit)
145156
}
146157
}
@@ -153,7 +164,7 @@ func (m *sentryMeter) WithCtx(ctx context.Context) Meter {
153164

154165
return &sentryMeter{
155166
ctx: ctx,
156-
client: m.client,
167+
hub: m.hub,
157168
attributes: attrsCopy,
158169
defaultAttributes: m.defaultAttributes,
159170
mu: sync.RWMutex{},

‎scope.go‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -523,21 +523,32 @@ func (scope *Scope) populateAttrs(attrs map[string]Attribute) {
523523
// }
524524
}
525525

526-
// resolveScopeAndTrace resolves scope, trace ID, and span ID from the given contexts.
526+
// hubFromContexts is a helper to return the first hub found in the given contexts.
527+
func hubFromContexts(ctxs ...context.Context) *Hub {
528+
for _, ctx := range ctxs {
529+
if ctx == nil {
530+
continue
531+
}
532+
if hub := GetHubFromContext(ctx); hub != nil {
533+
return hub
534+
}
535+
}
536+
return nil
537+
}
538+
539+
// resolveTrace resolves trace ID and span ID from the given scope and contexts.
527540
//
528541
// The resolution order follows a most-specific-to-least-specific pattern:
529542
// 1. Check for span directly in contexts (SpanFromContext) - this is the most specific
530543
// source as it represents a span explicitly attached to the current operation's context
531-
// 2. Check for hub in contexts (GetHubFromContext) - provides access to scope which may
532-
// contain a span, but this is less specific than a directly attached span
533-
// 3. Fall back to CurrentHub() - the global hub as a last resort
544+
// 2. Check scope's span - provides access to span set on the hub's scope
545+
// 3. Fall back to scope's propagation context trace ID
534546
//
535547
// This ordering ensures we always use the most contextually relevant tracing information.
536548
// For example, if a specific span is active for an operation, we use that span's trace/span IDs
537549
// rather than accidentally using a different span that might be set on the hub's scope.
538-
func resolveScopeAndTrace(ctxs ...context.Context) (scope *Scope, traceID TraceID, spanID SpanID) {
550+
func resolveTrace(scope *Scope, ctxs ...context.Context) (traceID TraceID, spanID SpanID) {
539551
var span *Span
540-
var hub *Hub
541552

542553
for _, ctx := range ctxs {
543554
if ctx == nil {
@@ -548,19 +559,6 @@ func resolveScopeAndTrace(ctxs ...context.Context) (scope *Scope, traceID TraceI
548559
}
549560
}
550561

551-
for _, ctx := range ctxs {
552-
if ctx == nil {
553-
continue
554-
}
555-
if hub = GetHubFromContext(ctx); hub != nil {
556-
break
557-
}
558-
}
559-
if hub == nil {
560-
hub = CurrentHub()
561-
}
562-
563-
scope = hub.Scope()
564562
if scope != nil {
565563
scope.mu.RLock()
566564
if span == nil {
@@ -575,5 +573,5 @@ func resolveScopeAndTrace(ctxs ...context.Context) (scope *Scope, traceID TraceI
575573
scope.mu.RUnlock()
576574
}
577575

578-
return scope, traceID, spanID
576+
return traceID, spanID
579577
}

0 commit comments

Comments
 (0)