Updated statsd client to allow less than 1 ms timing #63
Conversation
| @@ -82,7 +82,7 @@ def timer(self, stat, rate=1): | |||
|
|
|||
| def timing(self, stat, delta, rate=1): | |||
| """Send new timing information. `delta` is in milliseconds.""" | |||
| self._send_stat(stat, '%d|ms' % delta, rate) | |||
| self._send_stat(stat, '%f|ms' % delta, rate) | |||
There was a problem hiding this comment.
Would it be worth being more specific about the format here, to make the output more predictable, especially in the tests?
| ('gauge', 'g'), | ||
| ('incr', 'c'), | ||
| ('timing', 'ms'), | ||
| ('gauge', 'g', 'foo:1234568901234|%s'), |
There was a problem hiding this comment.
might as well just drop the suffix part and make the result the complete thing (along with adjusting the _check method to skip the result % suffix line, which is the only place suffix gets used.
|
Made a couple of comments. Would you mind hitting those, adding a line to the changelog under 3.2, and squashing it all into one commit? Thanks! |
|
@jsocol Awesome. I wanted to get to this yesterday, but ran out of time. I will get the updates made. How do you prefer squashing commits? |
|
@scbunn All squashed down into one commit, so code and tests changes are together, is perfect. Thanks so much! |
| @@ -50,7 +50,7 @@ def stop(self, send=True): | |||
| if self._start_time is None: | |||
| raise RuntimeError('Timer has not started.') | |||
| dt = time.time() - self._start_time | |||
| self.ms = int(round(1000 * dt)) # Convert to milliseconds. | |||
| self.ms = float(1000 * dt) # Convert to milliseconds. | |||
There was a problem hiding this comment.
@scbunn The float() call is not necessary, dt is already a float and 1000 * dt will be a float too.
|
This would be useful to have, I'm happy to apply the required changes so that this can be merged. |
|
@jstasiak cool. I have been wanted to get back to this but have been too busy with other work projects. If you could wrap this up it would be appreciated. |
|
@scbunn @jsocol Please see smarkets@a001224 ( One thing I left unchanged is the |
|
@jstasiak your branch looks good to me. As far as %f formatter I would think that not limiting it would be best. Its possible that somebody might be timing something that needs a very high precision. |
|
@jsocol Is there a chance this is merged soon? Thanks in advance. |
|
@jstasiak Can you open a PR from the smarkets branch? Thanks for addressing the comments there. The only thing I'd ask is if we can specify the decimals on the float formatter, something like 4 or 6 decimal points, because I don't know that that behavior is guaranteed not to change on a given platform or Python version. |
Currently working on a project that requires the timing of functions that take less than 1 ms to run. Updated to allow for floating point values so timings of less than 1 ms can be represented.