ddtrace/tracer: fix regression when flushing based on size (#570)

During a previous change released as part of v1.20.0 in PR #498, the
flush channel was accidentally made blocking. This would cause a
deadlock on occasion when flushing after the payload size became large.

Tests failed to catch this problem because they were not using the
constructor and instead had their own incorrect constructor. This
change also addresses that problem so that tests would now correctly
catch the issue.

ddtrace/tracer/metrics_test.go

@@ -247,13 +247,7 @@ func (tg *testStatsdClient) Wait(n int, d time.Duration) error {
 
 func TestReportRuntimeMetrics(t *testing.T) {
 	var tg testStatsdClient
-	trc := &tracer{
-		stopped:  make(chan struct{}),
-		exitChan: make(chan struct{}),
-		config: &config{
-			statsd: &tg,
-		},
-	}
+	trc := newUnstartedTracer(withStatsdClient(&tg))
 
 	trc.wg.Add(1)
 	go func() {
@@ -274,19 +268,7 @@ func TestReportRuntimeMetrics(t *testing.T) {
 func TestReportHealthMetrics(t *testing.T) {
 	assert := assert.New(t)
 	var tg testStatsdClient
-	trc := &tracer{
-		config: &config{
-			statsd:    &tg,
-			sampler:   NewAllSampler(),
-			transport: newDummyTransport(),
-		},
-		payload:          newPayload(),
-		flushChan:        make(chan chan<- struct{}),
-		exitChan:         make(chan struct{}),
-		payloadChan:      make(chan []*span, payloadQueueSize),
-		stopped:          make(chan struct{}),
-		prioritySampling: newPrioritySampler(),
-	}
+	trc := newUnstartedTracer(withStatsdClient(&tg))
 	internal.SetGlobalTracer(trc)
 	defer internal.SetGlobalTracer(&internal.NoopTracer{})
 

ddtrace/tracer/tracer.go

@@ -130,7 +130,7 @@ func Inject(ctx ddtrace.SpanContext, carrier interface{}) error {
 // payloadQueueSize is the buffer size of the trace channel.
 const payloadQueueSize = 1000
 
-func newTracer(opts ...StartOption) *tracer {
+func newUnstartedTracer(opts ...StartOption) *tracer {
 	c := new(config)
 	defaults(c)
 	for _, fn := range opts {
@@ -157,17 +157,21 @@ func newTracer(opts ...StartOption) *tracer {
 			c.statsd = client
 		}
 	}
-
-	t := &tracer{
+	return &tracer{
 		config:           c,
 		payload:          newPayload(),
-		flushChan:        make(chan chan<- struct{}),
+		flushChan:        make(chan chan<- struct{}, 1),
 		exitChan:         make(chan struct{}),
 		payloadChan:      make(chan []*span, payloadQueueSize),
 		stopped:          make(chan struct{}),
 		prioritySampling: newPrioritySampler(),
 		pid:              strconv.Itoa(os.Getpid()),
 	}
+}
+
+func newTracer(opts ...StartOption) *tracer {
+	t := newUnstartedTracer(opts...)
+	c := t.config
 	t.config.statsd.Incr("datadog.tracer.started", nil, 1)
 	if c.runtimeMetrics {
 		log.Debug("Runtime metrics enabled.")

ddtrace/tracer/tracer_test.go

@@ -867,16 +867,8 @@ func TestWorker(t *testing.T) {
 	}
 }
 
-func newTracerChannels() *tracer {
-	return &tracer{
-		payload:     newPayload(),
-		payloadChan: make(chan []*span, payloadQueueSize),
-		flushChan:   make(chan chan<- struct{}, 1),
-	}
-}
-
 func TestPushPayload(t *testing.T) {
-	tracer := newTracerChannels()
+	tracer := newUnstartedTracer()
 	s := newBasicSpan("3MB")
 	s.Meta["key"] = strings.Repeat("X", payloadSizeLimit/2+10)
 
@@ -896,7 +888,7 @@ func TestPushTrace(t *testing.T) {
 
 	tp := new(testLogger)
 	log.UseLogger(tp)
-	tracer := newTracerChannels()
+	tracer := newUnstartedTracer()
 	trace := []*span{
 		&span{
 			Name:     "pylons.request",

internal/version/version.go

@@ -8,4 +8,4 @@ package version
 // Tag specifies the current release tag. It needs to be manually
 // updated. A test checks that the value of Tag never points to a
 // git tag that is older than HEAD.
-const Tag = "v1.20.0"
+const Tag = "v1.20.1"