dropwizard/metrics

SlidingTimeWindowMovingAverages returns counts instead of rates

bgK opened this issue · 9 comments

bgK commented

When using Meters or Timers backed by the SlidingTimeWindowMovingAverages implementation, the reported OneMinuteRate, FiveMinuteRate and FifteenMinuteRate values are the number of events that occurred during the time window.
I would have expected the average number of events per second during the time window to be reported so that the moving average values are consistent with MeanRate and with the output of the ExponentialMovingAverages implementation.

Fixing the issue is simply a matter of dividing the number of events by the number of seconds in the time window. However it would be a breaking change as the reported values would not have the same meaning as with the current version. Would it be acceptable to submit a pull request with such a fix?

bgK commented

Yes indeed, this article explains very well the difference between the exponential moving average and the sliding time window moving average. What looks like a bug to me is that Dropwizard's sliding time window moving average implementation returns the number of events that occurred in the time window instead of the rate at which the events occurred. That is to say in the last experiment of the article with the simulated workload of 100 events per second, I would have expected the graph to peak at 100 (events / seconds) instead of 3000 (events).
A consequence of this behavior is a poor interaction with ScheduledReporter's rate unit conversion feature. If the reporter is configured to export the rates in events/minute instead of the default events/second, then the output of SlidingTimeWindowMovingAverages is multiplied by 60 which doesn't make a lot of sense.

This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.

bgK commented

This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.

Commenting to remove the stale label as this issue has not been resolved.

bgK commented

This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.

Commenting to remove the stale label as this issue has not been resolved.

This took me by surprise as well. All the meter reports now (when using the SlidingTimeWindowMovingAverages) is the number of events that occurred in the 1, 5 and 15 minute window and not the rate.

This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.

bgK commented

This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.

Commenting to remove the stale label as this issue has not been resolved.

This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.