ddtrace/tracer: send payloads asynchronously

[knusbaum]

This patch moves the flushing of payloads to the agent into its own goroutine.
This avoids blocking the rest of the tracer, decreasing the liklihood of dropping traces
due to network latency.

[knusbaum]

Hmmm. Dependencies are failing. I'll check this out tomorrow.

[gbbr]

Personally I don't think we need a different worker routine. With i/o, it's fine to have many goroutines. I already implemented this as part of the experimental tracer I am working on and I was hoping you'd use the same technique: https://github.com/DataDog/dd-trace-go/blob/v0.10.0/ddtrace/tracer/tracer.go#L320-L336. Can you maybe see what you think? The technique I've used is similar to what we've been doing in the agent and has been working very well with many repetitive flushes and high traffic.

[gbbr]

Sorry for the triple review, more came out :)

[knusbaum]

@gbbr

Personally I don't think we need a different worker routine. With i/o, it's fine to have many goroutines.

Sounds good to me. I see you're limiting the goroutines with a channel, so I don't see a reason to have a separate worker.

Edited: deleted long description of why we should limit the number of goroutines we're spawning. I missed the fact that that is already implemented here:

dd-trace-go/ddtrace/tracer/tracer.go

Lines 320 to 336 in 8961ccb

	t.climit <- struct{}{}
	t.wg.Add(1)
	go func(p *payload) {
	defer func() {
	<-t.climit
	t.wg.Done()
	}()
	size, count := p.size(), p.itemCount()
	log.Debug("Sending payload: size: %d spans: %d\n", size, count)
	rc, err := t.config.transport.send(p)
	if err != nil {
	log.Error("lost %d spans: %v", count, err)
	}
	if err == nil {
	t.prioritySampling.readRatesJSON(rc) // TODO: handle error?
	}
	}(t.payload)

[gbbr]

Thanks Kyle. I think this is looking good. I would like us to push a bit further and clean up that confirm channel and change forceFlush a bit. It will result in a better overall testing framework and cleaner code.

FWIW, if it helps, this is what I did in an experiment I ran https://github.com/DataDog/dd-trace-go/blob/v2-alpha/ddtrace/tracer/tracer_test.go#L983-L1005

[gbbr]

Thanks Kyle. I really like how this turned out. Tests are much more readable and code has also turned out cleaner. The async flushing will also make a big difference to memory usage and data integrity in many cases.

Please bear with me while I raise some questions and make some suggestions.

[gbbr]

Ace! 👌

[gbbr]

🎉

[gbbr]

Link #475