Move FutureMetrics to main source tree about metrics-scala HOT 5 CLOSED

Should have seen that when we splitted the code in two: ActorMetrics and FutureMetrics!
Yes, we should definitely move that!

Are you willing to create a pull request?

from metrics-scala.

This was done because:

• The implementation of FutureMetrics is only written against the akka-style of futures.
• scala futures and akka futures were separate before 2.10
• We needed to support scala versions prior to 2.10

Thus the 2.10+ code is in its own source tree.

I'd agree this is no longer necessary, it should be a fairly quick fix. If a PR doesn't show up soon-ish, I had an enhancement for the FutureMetrics that covers the parallel case of tacking on a timer listener onto an externally built Future that I am wrapping up and I can create a PR.

from metrics-scala.

Brian, you are clearly more awake then me this week :)

I am also thinking about wrapping the executor. We have something that does this but is currently too specific. It mostly useful to see if tasks need to wait long before they can get executed.

from metrics-scala.

Here's what i'm thinking of adding. Basically timing a an API that already produces a future by leveraging the onComplete listener. The current timer we have for Futures really only works with either a synchronous API or in code that the developer controls, making it restrictive. This would cover the parallel case, and although it's measuring something different, as long as that's understood, it can be useful.

To your point this measures (The time before the future is scheduled+The time the task takes to run+The time the listener takes to be scheduled), so it's not exact. I don't know that this is a problem as long as the user is aware of it and treats it as such.

  def timing[A](metricName: String)(block: => Future[A])(implicit context: ExecutionContext): Future[A] = {
    val timer = metrics.timer(metricName)
    val ctx = timer.timerContext
    block.onComplete(_ => ctx.stop())
    block
  }

from metrics-scala.

@erikvanoosten, @scullxbones Thanks, guys! Such a quick turnaround 👍

from metrics-scala.