Size-based packetization for dogstatsd batched metrics

[prognant]

What does this PR do?

It improves the packetization while sending batched metrics with packet size adjusted to prevent fragmentation while maximizing payload usage. Telemetry is sent in separate packets, once every 10 seconds won't impact overall performance. API is backward compatible, max_buffer_size has been deprecated but not removed, warning are logged if the parameter is used when building Dogstatsd objects.

Description of the Change

It adds sending heuristic based on current buffer length. Some approximation regarding the actual bytes sent on wire include unicode character encoding (note that the telemetry is not accounting for that either).

Alternate Designs

Better optimal packet size detection could have been considered but it seems overkill, moreover the settings can be overridden.

Possible Drawbacks

Verification Process

Unit tests, integration tests, manual tests, packet capture.

Additional Notes

Future rework of the client is planned and will include better multithreading awareness and optimal packet size management without the need to use batched metrics pattern.

Release Notes

Review checklist (to be filled by reviewers)

• Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
• PR title must be written as a CHANGELOG entry (see why)
• Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
• PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
• PR should not have do-not-merge/ label attached.
• If Applicable, issue must have kind/ and severity/ labels attached at least.

[hush-hush]

I think we should deprecate the old behavior. If a user set a number of metrics per buffer we respect and don't check for size.
By default max_buffer_size would be zero and we would use the actual byte size.

[hush-hush]

Overall it looks good. Since the API for DogStatsd changed we should bump the major, but I'm not sure if we can do it since the whole repo contains multiple project. We should sync with ing-tool-libs on this.

If bumping the major is not possible, we could still receive max_buffer_size in the API but not use it and print a warning ?

[prognant]

Overall it looks good. Since the API for DogStatsd changed we should bump the major, but I'm not sure if we can do it since the whole repo contains multiple project. We should sync with ing-tool-libs on this.

If bumping the major is not possible, we could still receive max_buffer_size in the API but not use it and print a warning ?

I will sync & assess the situation with ing-tool-libs

[jd]

This breaks the current API FWIW.
I'm not against it, but if you're going to break the API, it'd be better to provide a new one that e.g. fixes #439

[prognant]

I had a quick discussion about that, we agreed not to break the API, pending upcoming rework (I keep in mind the redis pipeline idea thanks again BTW).