Optionally measure block even when exception occurs

[dnlserrano]

Hey guys (Shopify and contributors)!

First of all thanks so much for the statsd-instrument gem. It helps us scale our system at Onfido.

This pull request introduces a feature that we found could be useful for certain projects. Maybe this is not the right way to achieve it. I'd like your feedback on it. 😅

We've ran into timeouts that yielded exceptions in wrapped StatsD.measure calls. This lead to timeouts not being reported to Datadog, which effectively resulted in biased dashboards and flawed monitoring. What we'd like to know, for better data collection and analysis, is how much time something took even if that something fails.

I've added a global option, StatsD.measure_on_exception that tells statsd-instrument to keep collecting time metrics even if we get an exception (i.e., StatsD.measure will still measure even if the code it wraps fails with a StandardError).

Default behaviour is retro-compatible.

To test it, I've added unit tests and used the following prototypes to do some integration testing:

• Dumb server that times out:

require "sinatra"

get "/*" do
  sleep 10

  [200, {"Content-Type" => "application/json"}, JSON.dump({})]
end

• Client reporting metrics:

require "httpclient"
require "statsd-instrument"

StatsD.measure_on_exception = true

class Test
  def call
    client = HTTPClient.new
    client.receive_timeout = 2
    client.get("http://localhost:4567")
  end

  extend StatsD::Instrument
  statsd_measure :call, "test.timeout"
  statsd_count_success :call, "test.timeout"
end

Before this change, we did not measure on exception:

2.3.1 :001 > Test.new.call
I, [2017-11-02T11:12:32.427451 #4726]  INFO -- : [StatsD] increment test.timeout.failure:1
HTTPClient::ReceiveTimeoutError: execution expired

After adding this, we measure on exception:

2.3.1 :001 > Test.new.call
I, [2017-11-02T11:12:06.369905 #4703]  INFO -- : [StatsD] measure test.timeout:2005.029140971601
I, [2017-11-02T11:12:06.370001 #4703]  INFO -- : [StatsD] increment test.timeout.failure:1
HTTPClient::ReceiveTimeoutError: execution expired

Thanks to my colleague and friend @jcmfernandes who paired with me on this. 😊

What do you guys think?

[dnlserrano]

Closing this in detriment of the supra-referenced issues.