Skip to content

Commit

Permalink
Remove Span.isTransaction attribute (#736)
Browse files Browse the repository at this point in the history
  • Loading branch information
greywolve authored Nov 13, 2023
1 parent 03c6345 commit 032ecc1
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 29 deletions.
20 changes: 7 additions & 13 deletions tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
28 changes: 12 additions & 16 deletions tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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{},
Expand All @@ -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,
},
Expand All @@ -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{
Expand All @@ -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)

Expand Down Expand Up @@ -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,
Expand All @@ -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{
Expand Down Expand Up @@ -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")
}
Expand Down

0 comments on commit 032ecc1

Please sign in to comment.