Add a "distribution_time" method#248
Conversation
| # @example Report the time (in ms) taken to activate an account | ||
| # $statsd.distribution_time('account.activate') { @account.activate! } | ||
| def distribution_time(stat, opts = EMPTY_OPTIONS) | ||
| opts = { sample_rate: opts } if opts.is_a?(Numeric) |
There was a problem hiding this comment.
I added this while adding specs, to mimic what happens in the timing method below. Is that what you'd like to do here?
There was a problem hiding this comment.
I suspect this is for backward-compatibility, so can probably be omitted here.
There was a problem hiding this comment.
Shall I go ahead and remove this then? cc @remeh
|
Are there other specs besides Also is something wrong w/ the CircleCI specs? They failed due to this error:
|
|
It looks like the patch is based on a version of master that still used circleci (ef41904). A rebase should help. |
To facilitate measuring timing for a yielded block and report it as a distribution metric instead of as a histogram metric.
jordan-brough
force-pushed
the
distribution-time
branch
from
5c82112 to
f71dd79
Compare
jordan-brough
force-pushed
the
distribution-time
branch
from
f71dd79 to
ab46b8a
Compare
|
@djmitche thanks, I've rebased on latest master and force-pushed. |
| # @example Report the time (in ms) taken to activate an account | ||
| # $statsd.distribution_time('account.activate') { @account.activate! } | ||
| def distribution_time(stat, opts = EMPTY_OPTIONS) | ||
| opts = { sample_rate: opts } if opts.is_a?(Numeric) |
There was a problem hiding this comment.
I suspect this is for backward-compatibility, so can probably be omitted here.
|
@remeh does this look ok? |
remeh
left a comment
remeh
left a comment
There was a problem hiding this comment.
This LGTM! Thanks for the added tests and sorry for the late review 🙇
|
Great! I left a follow-up comment above. Anything else before someone can merge this? Thanks again. |
|
Yep absolutely, latest change + the additional tests are great! Sorry for the latency @jordan-brough |
3 participants
Note: New PR for #200 with tests added.
Add a "distribution_time" method to facilitate measuring timing for
a yielded block and report it as a distribution metric instead of as a
histogram metric.