fix: report the right number of dropped bytes#927
fix: report the right number of dropped bytes#927KowalskiThomas wants to merge 4 commits intomasterfrom
Conversation
KowalskiThomas
added
the
changelog/Fixed
datadog/dogstatsd/base.py
Outdated
| except queue.Full: | ||
| self.packets_dropped_queue += 1 | ||
| self.bytes_dropped_queue += 1 | ||
| self.bytes_dropped_queue += len(packet) + 1 |
There was a problem hiding this comment.
I'm curious, why add 1 to the packet length?
Also, would it be possible to add a test for this?
There was a problem hiding this comment.
why add 1 to the packet length?
I believe the packet length would be the size of the string in characters + one null byte the new line we add in the message (sorry I got things mixed up here), right? (If not then we shouldn't add 1)
There was a problem hiding this comment.
would it be possible to add a test for this?
Sure :)
There was a problem hiding this comment.
That's what I was looking, I think the tests for this part are in https://github.com/DataDog/datadogpy/blob/a323241b79c2b17638fc980e57353537d866a9b8/tests/integration/dogstatsd/test_statsd_sender.py ?
There was a problem hiding this comment.
Just added a test
There was a problem hiding this comment.
Non-ascii characters may not be supported in metric names, but they are supported in tags, and this is being used. If we say that we count bytes, we should count bytes.
There was a problem hiding this comment.
Interesting, that's a good point, and to be honest I didn't know about that. The docs aren't super clear about it I think 😅
May contain alphanumerics, underscores, minuses, colons, periods, and slashes. Other characters are converted to underscores.
...
Tags can be up to 200 characters long (including both key and value) and support Unicode. Additional characters beyond this limit are truncated.
Which I guess means you can submit Unicode characters in tags but they'll be converted to underscores if they're not part of the supported character classes?
Anyway, I'll update the way we compute the value.
There was a problem hiding this comment.
Just updated it, please have another look 🙇
There was a problem hiding this comment.
Which I guess means you can submit Unicode characters in tags but they'll be converted to underscores if they're not part of the supported character classes?
No, we definitely support non-ascii text in tags in backend. Coincidentally, docs are being updated to make this more explicit.
There was a problem hiding this comment.
Sounds good, noted.
Kaycell
left a comment
Kaycell
left a comment
There was a problem hiding this comment.
No issue on the principle but need a test to clarify behavior :)
datadog/dogstatsd/base.py
Outdated
| except queue.Full: | ||
| self.packets_dropped_queue += 1 | ||
| self.bytes_dropped_queue += 1 | ||
| self.bytes_dropped_queue += len(packet) + 1 |
There was a problem hiding this comment.
That's what I was looking, I think the tests for this part are in https://github.com/DataDog/datadogpy/blob/a323241b79c2b17638fc980e57353537d866a9b8/tests/integration/dogstatsd/test_statsd_sender.py ?
KowalskiThomas
force-pushed
the
kowalski/fix-report-the-right-number-of-dropped-bytes
branch
from
7e16b3c to
3edfc4d
Compare
3 participants
What is this?
This PR updates the dropped bytes reporting logic to actually report the number of bytes dropped and not the number of packets dropped.