Skip to content

Commit

Permalink
ddtrace/tracer: fix concurrent map writes when applying trace samplin…
Browse files Browse the repository at this point in the history
…g rules and setting tags concurrently
  • Loading branch information
darccio committed Jun 4, 2024
1 parent 6126fb6 commit 9b132d7
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
11 changes: 7 additions & 4 deletions ddtrace/tracer/rules_sampler.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,20 +498,23 @@ func (rs *traceRulesSampler) sampleRules(span *span) bool {
}

func (rs *traceRulesSampler) applyRate(span *span, rate float64, now time.Time, sampler samplernames.SamplerName) {
span.Lock()
defer span.Unlock()

span.setMetric(keyRulesSamplerAppliedRate, rate)
delete(span.Metrics, keySamplingPriorityRate)
if !sampledByRate(span.TraceID, rate) {
span.setSamplingPriority(ext.PriorityUserReject, sampler)
span.setSamplingPriorityLocked(ext.PriorityUserReject, sampler)
return
}

sampled, rate := rs.limiter.allowOne(now)
if sampled {
span.setSamplingPriority(ext.PriorityUserKeep, sampler)
span.setSamplingPriorityLocked(ext.PriorityUserKeep, sampler)
} else {
span.setSamplingPriority(ext.PriorityUserReject, sampler)
span.setSamplingPriorityLocked(ext.PriorityUserReject, sampler)
}
span.SetTag(keyRulesSamplerLimiterRate, rate)
span.setMetric(keyRulesSamplerLimiterRate, rate)
}

// limit returns the rate limit set in the rules sampler, controlled by DD_TRACE_RATE_LIMIT, and
Expand Down
27 changes: 27 additions & 0 deletions ddtrace/tracer/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1076,3 +1076,30 @@ type stringer struct{}
func (s *stringer) String() string {
return "string"
}

// TestConcurrentSpanSetTag tests that setting tags concurrently on a span directly or
// not (through tracer.Inject when trace sampling rules are in place) does not cause
// concurrent map writes. It seems to only be consistently reproduced with the -count=100
// flag when running go test, but it's a good test to have.
func TestConcurrentSpanSetTag(t *testing.T) {
tracer, _, _, stop := startTestTracer(t, WithSamplingRules([]SamplingRule{NameRule("root", 1.0)}))
defer stop()

span := tracer.StartSpan("root")
defer span.Finish()

const n = 100
wg := sync.WaitGroup{}
wg.Add(n * 2)
for i := 0; i < n; i++ {
go func() {
span.SetTag("key", "value")
wg.Done()
}()
go func() {
tracer.Inject(span.Context(), TextMapCarrier(map[string]string{}))
wg.Done()
}()
}
wg.Wait()
}

0 comments on commit 9b132d7

Please sign in to comment.