Dogstatds: Enable consolidating different metrics in a single payload

[nappairam]

Changes:

Length prefix is only needed in case of Unix Stream and not for Datagram whether it Unix or UDP sockets.

Reference:
https://github.com/DataDog/datadog-go/blob/bae3560e5c664d64b71c0e8ca89326afa362e12a/statsd/uds.go#L62

Earlier, it was assumed that RemoteAddr::Unixgram has to prefix length which made the code complex padding length, to avoid copy before sending.

Since length prefix is not needed for UDP sockets, made the code simple by pushing the length padding role until the end of send.

This allows me to send different metrics in a same UDP packet

TODO:

• Pending testing
• Add tests for consolidating metrics

[tobz]

So before you get too far, a few things...

First, I don't want to have two write calls for the length prefix and the payload buffer when we can simply add the length header in the payload writer. I intentionally wrote the current code in this way to avoid that.

Secondly, the Datadog Agent can accept multiple metrics bundled in a single length-delimited frame, so long as they're newline-delimited so that they can be split apart. This is, overall, more efficient than trying to make each one length-delimited, since we only pay the 4 byte length delimiter cost once per overall buffer instead of for each metric.

[nappairam]

Thanks @tobz for early comments.

First, I don't want to have two write calls for the length prefix and the payload buffer when we can simply add the length header in the payload writer. I intentionally wrote the current code in this way to avoid that.

Yes, I understood it was intentional. But I was thinking, since we had length_prefix enabled for UDS Datagram, we did not want to memmove or memcopy before writing.
Cause we cannot split and send one frame with two write calls in case of Datagram. And it makes sense to use the PayloadWriter to have length_prefix as part of it.

But with Streams, IMHO there is no need for that, since we can write twice which is still fine.

And we use write_all for Stream, which can still call write system call multiple times, in case of partial success. I did not yet fully understand, what is the concern to use two Stream write here.

If we want single write, it is still possible to use length_prefix in PayloadWriter. But we had to constantly do memmove/memcopy, since we do not know when will the max_payload_len will be full and add length prefix. So I opted for two writes for Stream alone

Secondly, the Datadog Agent can accept multiple metrics bundled in a single length-delimited frame, so long as they're newline-delimited so that they can be split apart. This is, overall, more efficient than trying to make each one length-delimited, since we only pay the 4 byte length delimiter cost once per overall buffer instead of for each metric.

Yes correct. This is what I am trying to do.

• Remove length prefix from the PayloadWriter
• Aggregate/consolidate different metrics in a single payload
• Send length of payload (with multiple metrics inside) ONLY in case of Unix Stream
• And then send Payload fully

Please let me know if I miss something. Would be happy to refactor based on comments.

[tobz]

Yes, I understood it was intentional. But I was thinking, since we had length_prefix enabled for UDS Datagram, we did not want to memmove or memcopy before writing. Cause we cannot split and send one frame with two write calls in case of Datagram. And it makes sense to use the PayloadWriter to have length_prefix as part of it.

Actually, now that I look closer... I mistakenly had is_length_prefixed returning true for UDS datagram, when it should be false.

And we use write_all for Stream, which can still call write system call multiple times, in case of partial success. I did not yet fully understand, what is the concern to use two Stream write here.

That's true, but in the optimal case, it uses a single write syscall. If we write the length prefix in one call, and then the rest of the payload in another call... then we're always making two write syscalls. This is what we want to avoid: always making at least two, instead of sometimes making two.

If we want single write, it is still possible to use length_prefix in PayloadWriter. But we had to constantly do memmove/memcopy, since we do not know when will the max_payload_len will be full and add length prefix. So I opted for two writes for Stream alone

We don't have to move or copy anything, as PayloadWriter already tracks most of this state. We just need to add a little more state (current payload size) so that subsequent writes can determine if they can append to the current payload or if a new payload must be marked.