Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Catch error conditions that can result even when using UDP #6

Merged
merged 1 commit into from

2 participants

@betamatt

There's a bunch of issues that can result from the DNS request which happens before packets start going out over UDP. This patch ensures that any such network errors are logged instead of bubbling up and that requests which black-hole will timeout quickly.

@jstorimer
Collaborator

Scary to think that StatsD could block the request. Nice fix. And +1 on your usage of #fetch for a default value :)

@jstorimer jstorimer merged commit 171c0de into Shopify:master
@betamatt

Thanks. If you don't have anything else in the pipe, mind cutting a point release?

@jstorimer
Collaborator

Done. v1.1.2 is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 52 additions and 1 deletion.
  1. +11 −1 lib/statsd/instrument.rb
  2. +41 −0 test/statsd-instrument_test.rb
View
12 lib/statsd/instrument.rb
@@ -1,5 +1,6 @@
require 'socket'
require 'benchmark'
+require 'timeout'
class << Benchmark
def ms
@@ -7,6 +8,7 @@ def ms
end
end
+
module StatsD
class << self
attr_accessor :host, :port, :mode, :logger, :enabled, :default_sample_rate
@@ -14,6 +16,8 @@ class << self
self.enabled = true
self.default_sample_rate = 1
+ Timeout = defined?(::SystemTimer) ? ::SystemTimer : ::Timeout
+
# StatsD.server = 'localhost:1234'
def self.server=(conn)
self.host, port = conn.split(':')
@@ -130,10 +134,16 @@ def self.write(k,v,op, sample_rate = default_sample_rate)
command << "|@#{sample_rate}" if sample_rate < 1
if mode == :production
- socket.send(command, 0, host, port)
+ socket_wrapper { socket.send(command, 0, host, port) }
else
logger.info "[StatsD] #{command}"
end
end
+
+ def self.socket_wrapper(options = {})
+ Timeout.timeout(options.fetch(:timeout, 0.1)) { yield }
+ rescue Timeout::Error, SocketError, IOError, SystemCallError => e
+ logger.error e
+ end
end
View
41 test/statsd-instrument_test.rb
@@ -153,4 +153,45 @@ def test_statsd_measure_with_explicit_value
StatsD.measure('values.foobar', 42)
end
+
+ def test_socket_error_should_not_raise
+ StatsD.mode = :production
+ UDPSocket.any_instance.expects(:send).raises(SocketError)
+ StatsD.measure('values.foobar', 42)
+ StatsD.mode = :test
+ end
+
+ def test_timeout_error_should_not_raise
+ StatsD.mode = :production
+ UDPSocket.any_instance.expects(:send).raises(Timeout::Error)
+ StatsD.measure('values.foobar', 42)
+ StatsD.mode = :test
+ end
+
+ def test_system_call_error_should_not_raise
+ StatsD.mode = :production
+ UDPSocket.any_instance.expects(:send).raises(Errno::ETIMEDOUT)
+ StatsD.measure('values.foobar', 42)
+ StatsD.mode = :test
+ end
+
+ def test_io_error_should_not_raise
+ StatsD.mode = :production
+ UDPSocket.any_instance.expects(:send).raises(IOError)
+ StatsD.measure('values.foobar', 42)
+ StatsD.mode = :test
+ end
+
+ def test_long_request_should_timeout
+ StatsD.mode = :production
+ UDPSocket.any_instance.expects(:send).yields do
+ begin
+ Timeout.timeout(0.5) { sleep 1 }
+ rescue Timeout::Error
+ raise "Allowed long running request"
+ end
+ end
+ StatsD.measure('values.foobar', 42)
+ StatsD.mode = :test
+ end
end
Something went wrong with that request. Please try again.