From 6fd8c49d50803bbccfcc11b195f9e334a6e835e9 Mon Sep 17 00:00:00 2001 From: Pedro Tanaka Date: Tue, 14 Jan 2025 12:46:59 +0100 Subject: [PATCH] Prevent ENOBUFS errors by skipping UDP send buffer size configuration (#392) The previous implementation attempted to set the UDP socket's send buffer size, which could lead to ENOBUFS errors in some environments. This change: - Overrides setup_socket in UDPConnection to skip buffer size configuration - Improves socket creation by using the correct address family - Updates error message to be more accurate ("Failed to setup socket") This prevents potential buffer-related errors while maintaining the original functionality for other connection types. --- CHANGELOG.md | 4 ++++ Gemfile | 5 +++++ lib/statsd/instrument/connection_behavior.rb | 2 +- lib/statsd/instrument/udp_connection.rb | 9 +++++++-- lib/statsd/instrument/version.rb | 2 +- test/udp_sink_test.rb | 14 -------------- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c6c0c3d..dd6fb003 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ section below. ## Unreleased changes +## Version 3.9.9 + +- [#392](https://github.com/Shopify/statsd-instrument/pull/392) - Prevent ENOBUFS errors when using UDP, by skipping setting socket buffer size. + ## Version 3.9.8 - [#390](https://github.com/Shopify/statsd-instrument/pull/391) - Fixing bug in Environment when using UDS. The max packet size option was not being passed to the diff --git a/Gemfile b/Gemfile index f6ef0d3c..0d053075 100644 --- a/Gemfile +++ b/Gemfile @@ -19,4 +19,9 @@ platform :mri do if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.2") gem "vernier", require: false end + + # From Ruby >= 3.5, logger is not part of the stdlib anymore + if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.5") + gem "logger" + end end diff --git a/lib/statsd/instrument/connection_behavior.rb b/lib/statsd/instrument/connection_behavior.rb index 4a651157..0e8e7fcf 100644 --- a/lib/statsd/instrument/connection_behavior.rb +++ b/lib/statsd/instrument/connection_behavior.rb @@ -42,7 +42,7 @@ def setup_socket(original_socket) original_socket rescue IOError => e StatsD.logger.debug do - "[#{self.class.name}] Failed to create socket: #{e.class}: #{e.message}" + "[#{self.class.name}] Failed to setup socket: #{e.class}: #{e.message}" end nil end diff --git a/lib/statsd/instrument/udp_connection.rb b/lib/statsd/instrument/udp_connection.rb index a291319c..b7f9ff5f 100644 --- a/lib/statsd/instrument/udp_connection.rb +++ b/lib/statsd/instrument/udp_connection.rb @@ -25,11 +25,16 @@ def type private + def setup_socket(original_socket) + original_socket + end + def socket @socket ||= begin - udp_socket = UDPSocket.new + family = Addrinfo.udp(host, port).afamily + udp_socket = UDPSocket.new(family) setup_socket(udp_socket)&.tap do |s| - s.connect(@host, @port) + s.connect(host, port) end end end diff --git a/lib/statsd/instrument/version.rb b/lib/statsd/instrument/version.rb index fb5ec9b5..793dca58 100644 --- a/lib/statsd/instrument/version.rb +++ b/lib/statsd/instrument/version.rb @@ -2,6 +2,6 @@ module StatsD module Instrument - VERSION = "3.9.8" + VERSION = "3.9.9" end end diff --git a/test/udp_sink_test.rb b/test/udp_sink_test.rb index 7fe9c2da..ef3c6059 100644 --- a/test/udp_sink_test.rb +++ b/test/udp_sink_test.rb @@ -151,25 +151,11 @@ def test_socket_error_should_invalidate_socket seq = sequence("connect_fail_connect_succeed") # First attempt - socket.expects(:setsockopt) - .with(Socket::SOL_SOCKET, Socket::SO_SNDBUF, StatsD::Instrument::UdpConnection::DEFAULT_MAX_PACKET_SIZE) - .in_sequence(seq) - socket.expects(:getsockopt) - .with(Socket::SOL_SOCKET, Socket::SO_SNDBUF) - .returns(mock(int: StatsD::Instrument::UdpConnection::DEFAULT_MAX_PACKET_SIZE)) - .in_sequence(seq) socket.expects(:connect).with("localhost", 8125).in_sequence(seq) socket.expects(:send).raises(Errno::EDESTADDRREQ).in_sequence(seq) socket.expects(:close).in_sequence(seq) # Second attempt after error - socket.expects(:setsockopt) - .with(Socket::SOL_SOCKET, Socket::SO_SNDBUF, StatsD::Instrument::UdpConnection::DEFAULT_MAX_PACKET_SIZE) - .in_sequence(seq) - socket.expects(:getsockopt) - .with(Socket::SOL_SOCKET, Socket::SO_SNDBUF) - .returns(mock(int: StatsD::Instrument::UdpConnection::DEFAULT_MAX_PACKET_SIZE)) - .in_sequence(seq) socket.expects(:connect).with("localhost", 8125).in_sequence(seq) socket.expects(:send).twice.returns(1).in_sequence(seq) socket.expects(:close).in_sequence(seq)