diff --git a/ddtrace/tracer/sampler.go b/ddtrace/tracer/sampler.go index b71306c020..57971c63e3 100644 --- a/ddtrace/tracer/sampler.go +++ b/ddtrace/tracer/sampler.go @@ -292,6 +292,7 @@ func (rs *rulesSampler) apply(span *span) bool { var matched bool rate := rs.globalRate for _, rule := range rs.rules { + fmt.Println("try match", rule.exactService, rule.exactName, span.Service, span.Name) if rule.match(span) { matched = true rate = rule.Rate diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index e9b3741cbf..32c78628dd 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -373,7 +373,11 @@ func (s *span) finish(finishTime int64) { // client sampling enabled. This will lead to inexact APM metrics but reduces performance impact. keep = false } - s.context.finish(keep) + if keep { + // a single kept span keeps the whole trace. + s.context.trace.keep() + } + s.context.finish() } // newAggregableSpan creates a new summary for the span s, within an application diff --git a/ddtrace/tracer/spancontext.go b/ddtrace/tracer/spancontext.go index 399a320e3e..228b688644 100644 --- a/ddtrace/tracer/spancontext.go +++ b/ddtrace/tracer/spancontext.go @@ -25,7 +25,7 @@ type spanContext struct { trace *trace // reference to the trace that this span belongs too span *span // reference to the span that hosts this context - drop bool // when true, the span will not be sent to the agent, even when not computing stats in the tracer. + drop bool // reports whether this span was dropped by the rate sampler errors int64 // number of spans with errors in this trace // the below group should propagate cross-process @@ -126,7 +126,7 @@ func (c *spanContext) baggageItem(key string) string { } // finish marks this span as finished in the trace. -func (c *spanContext) finish(keep bool) { c.trace.finishedOne(c.span, keep) } +func (c *spanContext) finish() { c.trace.finishedOne(c.span) } // trace contains shared context information about a trace, such as sampling // priority, the root reference and a buffer of the spans which are part of the @@ -138,7 +138,7 @@ type trace struct { full bool // signifies that the span buffer is full priority *float64 // sampling priority locked bool // specifies if the sampling priority can be altered - keep bool // keep indicates whether to send the trace to the agent or no. + kept bool // kept indicates whether to send the trace to the agent or no. // root specifies the root of the trace, if known; it is nil when a span // context is extracted from a carrier, at which point there are no spans in @@ -180,6 +180,12 @@ func (t *trace) setSamplingPriority(p float64) { t.setSamplingPriorityLocked(p) } +func (t *trace) keep() { + t.mu.Lock() + defer t.mu.Unlock() + t.kept = true +} + func (t *trace) setSamplingPriorityLocked(p float64) { if t.locked { return @@ -226,7 +232,7 @@ func (t *trace) push(sp *span) { // finishedOne aknowledges that another span in the trace has finished, and checks // if the trace is complete, in which case it calls the onFinish function. It uses // the given priority, if non-nil, to mark the root span. -func (t *trace) finishedOne(s *span, keep bool) { +func (t *trace) finishedOne(s *span) { t.mu.Lock() defer t.mu.Unlock() if t.full { @@ -236,9 +242,6 @@ func (t *trace) finishedOne(s *span, keep bool) { // to a race condition where spans can be modified while flushing. return } - if keep { - t.keep = true - } t.finished++ if s == t.root && t.priority != nil { // after the root has finished we lock down the priority; @@ -254,7 +257,7 @@ func (t *trace) finishedOne(s *span, keep bool) { t.spans = nil t.finished = 0 // important, because a buffer can be used for several flushes }() - if !t.keep { + if !t.kept { return } if tr, ok := internal.GetGlobalTracer().(*tracer); ok { diff --git a/ddtrace/tracer/tracer_test.go b/ddtrace/tracer/tracer_test.go index dc38acf501..d60a3aee71 100644 --- a/ddtrace/tracer/tracer_test.go +++ b/ddtrace/tracer/tracer_test.go @@ -236,15 +236,49 @@ func TestTracerStartSpan(t *testing.T) { child := tracer.StartSpan("home/user", Measured(), ChildOf(parent.context)).(*span) assert.Equal(t, 1.0, child.Metrics[keyMeasured]) }) +} - t.Run("sampling_priority", func(t *testing.T) { - tracer := newTracer() +func TestP0Dropping(t *testing.T) { + t.Run("p0 are kept by default", func(t *testing.T) { + tracer, _, _, stop := startTestTracer(t) + defer stop() + tracer.prioritySampling.defaultRate = 0 tracer.config.serviceName = "test_service" - tracer.rulesSampling.rules = append(tracer.rulesSampling.rules, SamplingRule{exactService: "test_service", exactName: "web.request", Rate: 1}) - span := tracer.StartSpan("web.request").(*span) - assert.Equal(t, float64(ext.PriorityAutoKeep), span.Metrics[keySamplingPriority]) - priority, _ := span.context.samplingPriority() - assert.Equal(t, 1, priority) + span := tracer.StartSpan("name_1").(*span) + child := tracer.StartSpan("name_2", ChildOf(span.context)) + child.Finish() + span.Finish() + assert.Equal(t, float64(ext.PriorityAutoReject), span.Metrics[keySamplingPriority]) + assert.True(t, span.context.trace.kept) + }) + + t.Run("p0 are dropped when DropP0s flag is on", func(t *testing.T) { + tracer, _, _, stop := startTestTracer(t) + defer stop() + tracer.features.DropP0s = true + tracer.prioritySampling.defaultRate = 0 + tracer.config.serviceName = "test_service" + span := tracer.StartSpan("name_1").(*span) + child := tracer.StartSpan("name_2", ChildOf(span.context)) + child.Finish() + span.Finish() + assert.Equal(t, float64(ext.PriorityAutoReject), span.Metrics[keySamplingPriority]) + assert.False(t, span.context.trace.kept) + }) + + t.Run("p0 are kept if at least one span should be kept", func(t *testing.T) { + tracer, _, _, stop := startTestTracer(t) + defer stop() + tracer.features.DropP0s = true + tracer.prioritySampling.defaultRate = 0 + tracer.config.serviceName = "test_service" + span := tracer.StartSpan("name_1").(*span) + child := tracer.StartSpan("name_2", ChildOf(span.context)) + child.SetTag(ext.EventSampleRate, 1) + child.Finish() + span.Finish() + assert.Equal(t, float64(ext.PriorityAutoReject), span.Metrics[keySamplingPriority]) + assert.True(t, span.context.trace.kept) }) }