Fix incomplete flush in the sender when flushing the client

[hush-hush]

In some case, the sender 'sendLoop' would pop a buffer from its queue
as we are calling its 'flush' method. Since the input queue is empty it
would return instantly without waiting for the current buffer own by the
sender to be sent. This would result in partial flush.

We now synchronize 'flush' and 'sendLoop' to make sure flushing the
client will send everything to the wire. The side effect of this is that
the workers are lock for longer as we keep them locked when flushing the
sender.

[olivielpeau]

This should address most cases of incomplete flushes, but in ChannelMode I don't think it guarantees that, when Flush is called, a previous metric submission call (e.g. Gauge()) has gone as far as a worker's writeMetricUnsafe call here

A larger refactoring might be necessary so that the pipeline of client -> workers -> sender can be fully flushed in order, by passing a "flush" message down from the client to the workers and ultimately to the sender, each step confirming that it has processed all data that was available before the flush started.

[hush-hush]

Indeed the ChannelMode will not be fully flushed. The goal of it being to decouple sampling metrics and receiving it in the workers so we never block the app. Right now, we have no way to fully flush the input channel without blocking the calling app, which would defeat the main purpose of ChannelMode. I think the current behavior is expected but we might want to improve the Flush documentation to make it clear.

For the flush message idea, I don't think it would really work with multiple workers as we don't control which one will pull the message from the queue. I'm not sure the overhead of synchronizing the workers is worth it just for ChannelMode.

[olivielpeau]

ok, for now in ChannelMode it sounds reasonable to document that the Flush function doesn't offer guarantees that everything that was submitted is actually flushed.

Left a suggestion inline

[truthbk]

📦