Sample rate config for dogstatsd #739
Conversation
GeorgeHahn
force-pushed
the
george/sampling-config
branch
from
599f503 to
bacbed6
Compare
GeorgeHahn
force-pushed
the
george/sampling-config
branch
from
bacbed6 to
fdf81e4
Compare
| type Error = ZeroToOneError; | ||
|
|
||
| fn try_from(value: f32) -> Result<Self, Self::Error> { | ||
| if (value - 1.0).abs() < f32::EPSILON { |
There was a problem hiding this comment.
The value 1.0 is exactly representable in IEEE-754-conformant 32-bit floating-point arithmetic, so the epsilon check here should be unnecessary, though I appreciate the intent because it is usually good practice.
| let prob: f32 = OpenClosed01.sample(&mut rng); | ||
| let sample_rate = if prob < self.sampling_probability { |
There was a problem hiding this comment.
Minor pedantic nit: because floating-point arithmetic quantizes the range [0, 1], omitting zero here does have a small effect on the probability logic that would be material for small values of self.sampling_probability (e.g., f64::MIN_POSITIVE). Does this actually matter in practice? Probably not, unless our users are pedantic masochists.
There was a problem hiding this comment.
(To be clear, I don't think the range needs to be changed unless we think someone would actually fuzz our system with an outrageously small sampling probability.)
There was a problem hiding this comment.
Merged, but I'd still like to understand this better. Would one of the other provided distributions be more correct? https://docs.rs/rand/latest/rand/distributions/struct.Standard.html#floating-point-implementation The Standard distribution stands out to me, but it seems like it would have the same issue on the opposite side.
Co-authored-by: Scott Opell <[email protected]>
Co-authored-by: Scott Opell <[email protected]>
Co-authored-by: Scott Opell <[email protected]>
What does this PR do?
Add sample rate & sample rate probability configs to the Dogstatsd payload generator.
Motivation
This parameter was previously randomly generated between
0..=1, and we'd like more control over it.Related issues
SMPTNG-72