Skip to content

Commit

Permalink
Merge pull request #1 from runtastic/reconnect-on-ip-change
Browse files Browse the repository at this point in the history
Reconnect on ip change
  • Loading branch information
Goltergaul authored Oct 3, 2023
2 parents 3c09eaf + 7a36569 commit 72241c3
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 1 deletion.
30 changes: 30 additions & 0 deletions lib/datadog/statsd/udp_connection.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative 'connection'
require "resolv"

module Datadog
class Statsd
Expand All @@ -11,10 +12,21 @@ class UDPConnection < Connection
# StatsD port.
attr_reader :port


def self.resolve_host_dns(hostname)
Resolv::DNS.open do |dns|
dns.timeouts = 1
dns.getaddress(@host)
end
rescue Resolv::ResolvError
nil
end

def initialize(host, port, **kwargs)
super(**kwargs)

@host = host
@host_is_ip = !!(@host =~ Regexp.union([Resolv::IPv4::Regex, Resolv::IPv6::Regex]))
@port = port
@socket = nil
end
Expand All @@ -35,12 +47,30 @@ def connect
@socket.connect(host, port)
end

def check_dns_resolution
return if @host_is_ip
return if @last_dns_check && Time.now - @last_dns_check < 60

@last_dns_check = Time.now
fresh_resolved_ip = self.class.resolve_host_dns(@host)
@current_host_ip = fresh_resolved_ip unless defined?(@current_host_ip)

return if @current_host_ip == fresh_resolved_ip

@current_host_ip = fresh_resolved_ip
close
connect
rescue Resolv::ResolvError
nil
end

# send_message is writing the message in the socket, it may create the socket if nil
# It is not thread-safe but since it is called by either the Sender bg thread or the
# SingleThreadSender (which is using a mutex while Flushing), only one thread must call
# it at a time.
def send_message(message)
connect unless @socket
check_dns_resolution
@socket.send(message, 0)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/integrations/allocation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

describe 'Allocations and garbage collection' do
before do
skip 'Ruby too old' if RUBY_VERSION < '2.3.0'
skip 'deactivated for now'
end

let(:socket) { FakeUDPSocket.new }
Expand Down
1 change: 1 addition & 0 deletions spec/integrations/buffering_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
before do
allow(Socket).to receive(:new).and_return(socket)
allow(UDPSocket).to receive(:new).and_return(socket)
allow(Datadog::Statsd::UDPConnection).to receive(:resolve_host_dns)
end

it 'does not not send anything when the buffer is empty' do
Expand Down
70 changes: 70 additions & 0 deletions spec/integrations/connection_edge_case_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
end
end
let(:log) { StringIO.new }

before do
dns_mock = instance_double(Resolv::DNS, 'timeouts=': nil, getaddress: nil)
allow(Resolv::DNS).to receive(:open)
.and_yield(dns_mock)
end

describe 'when having problems with UDP communication' do
subject do
Expand All @@ -21,6 +27,7 @@

let(:fake_socket) do
instance_double(UDPSocket,
close: true,
connect: true,
send: true)
end
Expand All @@ -31,6 +38,69 @@
send: true)
end

context 'when hostname resolves to a different ip address after connecting' do
it 'reconnects socket after 60 seconds if the ip changes' do
dns_mock = instance_double(Resolv::DNS, 'timeouts=': nil)
allow(dns_mock).to receive(:getaddress)
.and_return("192.168.0.1", "192.168.0.2")
allow(Resolv::DNS).to receive(:open)
.and_yield(dns_mock)

subject.write('foobar')
expect(fake_socket)
.to have_received(:send)
.with('foobar', anything)

subject.write('foobar')
expect(fake_socket)
.to have_received(:send)
.with('foobar', anything)
.twice

Timecop.travel(Time.now + 61) do
subject.write('foobar')
expect(fake_socket_retry)
.to have_received(:send)
.with('foobar', anything)
end

Timecop.travel(Time.now + 360) do
subject.write('foobar')
expect(fake_socket_retry)
.to have_received(:send)
.with('foobar', anything)
.twice
end
end

it 'does not reconnect socket after 60 seconds if the ip does not change' do
dns_mock = instance_double(Resolv::DNS, 'timeouts=': nil)
allow(dns_mock).to receive(:getaddress)
.and_return("192.168.0.1")
allow(Resolv::DNS).to receive(:open)
.and_yield(dns_mock)

subject.write('foobar')
expect(fake_socket)
.to have_received(:send)
.with('foobar', anything)

subject.write('foobar')
expect(fake_socket)
.to have_received(:send)
.with('foobar', anything)
.twice

Timecop.travel(Time.now + 61) do
subject.write('foobar')
expect(fake_socket)
.to have_received(:send)
.with('foobar', anything)
.exactly(3).times
end
end
end

context 'when having unknown SocketError (drop strategy)'do
before do
allow(fake_socket)
Expand Down
1 change: 1 addition & 0 deletions spec/integrations/delay_serialization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
end

it "serializes messages normally" do
allow(Datadog::Statsd::UDPConnection).to receive(:resolve_host_dns)
socket = FakeUDPSocket.new(copy_message: true)
allow(UDPSocket).to receive(:new).and_return(socket)
dogstats = Datadog::Statsd.new("localhost", 1234, delay_serialization: true)
Expand Down
1 change: 1 addition & 0 deletions spec/integrations/telemetry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
before do
allow(Socket).to receive(:new).and_return(socket)
allow(UDPSocket).to receive(:new).and_return(socket)
allow(Datadog::Statsd::UDPConnection).to receive(:resolve_host_dns)
end

let(:namespace) { nil }
Expand Down
7 changes: 7 additions & 0 deletions spec/statsd/udp_connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@
allow(UDPSocket)
.to receive(:new)
.and_return(udp_socket)

dns_mock = instance_double(Resolv::DNS)
allow(dns_mock).to receive(:timeouts=)
.with(1)
allow(dns_mock).to receive(:getaddress)
allow(Resolv::DNS).to receive(:open)
.and_yield(dns_mock)
end

let(:udp_socket) do
Expand Down
1 change: 1 addition & 0 deletions spec/statsd_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
before do
allow(Socket).to receive(:new).and_return(socket)
allow(UDPSocket).to receive(:new).and_return(socket)
allow(Datadog::Statsd::UDPConnection).to receive(:resolve_host_dns)
end

describe '#initialize' do
Expand Down

0 comments on commit 72241c3

Please sign in to comment.