From 032ecc1861700897888aec5644403a3d0a435007 Mon Sep 17 00:00:00 2001 From: Oliver Powell Date: Mon, 13 Nov 2023 12:40:36 +0200 Subject: [PATCH] Remove Span.isTransaction attribute (#736) --- tracing.go | 20 +++++++------------- tracing_test.go | 28 ++++++++++++---------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/tracing.go b/tracing.go index cc68374ff..793bd28e2 100644 --- a/tracing.go +++ b/tracing.go @@ -49,11 +49,6 @@ type Span struct { //nolint: maligned // prefer readability over optimal memory // parent refers to the immediate local parent span. A remote parent span is // only referenced by setting ParentSpanID. parent *Span - // isTransaction is true only for the root span of a local span tree. The - // root span is the first span started in a context. Note that a local root - // span may have a remote parent belonging to the same trace, therefore - // isTransaction depends on ctx and not on parent. - isTransaction bool // recorder stores all spans in a transaction. Guaranteed to be non-nil. recorder *spanRecorder // span context, can only be set on transactions @@ -114,9 +109,8 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp StartTime: time.Now(), Sampled: SampledUndefined, - ctx: context.WithValue(ctx, spanContextKey{}, &span), - parent: parent, - isTransaction: !hasParent, + ctx: context.WithValue(ctx, spanContextKey{}, &span), + parent: parent, } if hasParent { @@ -257,7 +251,7 @@ func (s *Span) SetContext(key string, value Context) { // IsTransaction checks if the given span is a transaction. func (s *Span) IsTransaction() bool { - return s.isTransaction + return s.parent == nil } // GetTransaction returns the transaction that contains this span. @@ -326,7 +320,7 @@ func (s *Span) ToBaggage() string { // SetDynamicSamplingContext sets the given dynamic sampling context on the // current transaction. func (s *Span) SetDynamicSamplingContext(dsc DynamicSamplingContext) { - if s.isTransaction { + if s.IsTransaction() { s.dynamicSamplingContext = dsc } } @@ -391,7 +385,7 @@ func (s *Span) updateFromSentryTrace(header []byte) (updated bool) { } func (s *Span) updateFromBaggage(header []byte) { - if s.isTransaction { + if s.IsTransaction() { dsc, err := DynamicSamplingContextFromHeader(header) if err != nil { return @@ -452,7 +446,7 @@ func (s *Span) sample() Sampled { // Note: non-transaction should always have a parent, but we check both // conditions anyway -- the first for semantic meaning, the second to // avoid a nil pointer dereference. - if !s.isTransaction && s.parent != nil { + if !s.IsTransaction() && s.parent != nil { return s.parent.Sampled } @@ -516,7 +510,7 @@ func (s *Span) toEvent() *Event { s.mu.Lock() defer s.mu.Unlock() - if !s.isTransaction { + if !s.IsTransaction() { return nil // only transactions can be transformed into events } diff --git a/tracing_test.go b/tracing_test.go index 885f173ec..874f72170 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -488,8 +488,7 @@ func TestContinueTransactionFromHeaders(t *testing.T) { traceStr: "", baggageStr: "", wantSpan: &Span{ - isTransaction: true, - Sampled: 0, + Sampled: 0, dynamicSamplingContext: DynamicSamplingContext{ Frozen: false, Entries: nil, @@ -501,8 +500,7 @@ func TestContinueTransactionFromHeaders(t *testing.T) { traceStr: "", baggageStr: "other-vendor-key1=value1;value2, other-vendor-key2=value3", wantSpan: &Span{ - isTransaction: true, - Sampled: 0, + Sampled: 0, dynamicSamplingContext: DynamicSamplingContext{ Frozen: false, Entries: map[string]string{}, @@ -515,10 +513,9 @@ func TestContinueTransactionFromHeaders(t *testing.T) { traceStr: "bc6d53f15eb88f4320054569b8c553d4-b72fa28504b07285-1", baggageStr: "", wantSpan: &Span{ - isTransaction: true, - TraceID: TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"), - ParentSpanID: SpanIDFromHex("b72fa28504b07285"), - Sampled: 1, + TraceID: TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"), + ParentSpanID: SpanIDFromHex("b72fa28504b07285"), + Sampled: 1, dynamicSamplingContext: DynamicSamplingContext{ Frozen: true, }, @@ -529,10 +526,9 @@ func TestContinueTransactionFromHeaders(t *testing.T) { traceStr: "bc6d53f15eb88f4320054569b8c553d4-b72fa28504b07285-1", baggageStr: "sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03,sentry-public_key=public,sentry-sample_rate=1", wantSpan: &Span{ - isTransaction: true, - TraceID: TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"), - ParentSpanID: SpanIDFromHex("b72fa28504b07285"), - Sampled: 1, + TraceID: TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4"), + ParentSpanID: SpanIDFromHex("b72fa28504b07285"), + Sampled: 1, dynamicSamplingContext: DynamicSamplingContext{ Frozen: true, Entries: map[string]string{ @@ -546,7 +542,7 @@ func TestContinueTransactionFromHeaders(t *testing.T) { } for _, tt := range tests { - s := &Span{isTransaction: true} + s := &Span{} spanOption := ContinueFromHeaders(tt.traceStr, tt.baggageStr) spanOption(s) @@ -702,9 +698,9 @@ func TestDoesNotCrashWithEmptyContext(t *testing.T) { func TestSetDynamicSamplingContextWorksOnTransaction(t *testing.T) { s := Span{ - isTransaction: true, dynamicSamplingContext: DynamicSamplingContext{Frozen: false}, } + newDsc := DynamicSamplingContext{ Entries: map[string]string{"environment": "dev"}, Frozen: true, @@ -720,7 +716,7 @@ func TestSetDynamicSamplingContextWorksOnTransaction(t *testing.T) { func TestSetDynamicSamplingContextDoesNothingOnSpan(t *testing.T) { // SetDynamicSamplingContext should do nothing on non-transaction spans s := Span{ - isTransaction: false, + parent: &Span{}, dynamicSamplingContext: DynamicSamplingContext{}, } newDsc := DynamicSamplingContext{ @@ -812,7 +808,7 @@ func TestGetTransactionReturnsNilOnManuallyCreatedSpans(t *testing.T) { t.Errorf("GetTransaction() should return nil on manually created Spans") } - span2 := Span{isTransaction: true} + span2 := Span{} if span2.GetTransaction() != nil { t.Errorf("GetTransaction() should return nil on manually created Spans") }