search

liuggio / statsd-php-client

Statsd (Object Oriented) client library for PHP
MIT License
115 stars 32 forks source link

Fixed default sampling function. #54

Open tomec-martin opened 8 years ago

tomec-martin commented 8 years ago

For sampling rate 0.5 function gets arguments (0,2) and should return only 0 or 1 (not 2). Still, this is not correct for sampling rates like 0.4, (because of floor in appendToBuffer) but fix could break BC...

liuggio commented 8 years ago

Hi thanks a lot but this will affect everybody this is a big BC what do you think? Which is your suggestion?

tomec-martin commented 8 years ago

I think that this patch is the best compromise - because it does not afect API for sampling function. Less breaking workaround - you can mention in documentation that the default sampling function is wrong and users should replace it. But the API for sampling function should be specified and corrected. Floor in appendToBuffer is really wrong and should be removed (maybe in 2.0?): https://github.com/liuggio/statsd-php-client/blob/master/src/Liuggio/StatsdClient/Service/StatsdService.php#L175

liuggio commented 8 years ago

Thanks labelled as 2.0