Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support IP changes of statsd hostname #284

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

[//]: # (comment: Don't forget to update lib/datadog/statsd/version.rb:DogStatsd::Statsd::VERSION when releasing a new version)

## 5.7.1
* [BUGFIX] UPD connection ip tracking

## 5.7.0

* [FEATURE] UDP connection is tracking IP changes of the STATSD_HOST and reconnecting the socket when the ip changes

## 5.6.1 / 2023.09.07

* [IMPROVEMENT] Add support for IPv6 UDP connection. [#280][] by [@kazwolfe][]
Expand Down
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(hostname)
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 lib/datadog/statsd/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

module Datadog
class Statsd
VERSION = '5.6.1'
VERSION = '5.7.1'
end
end
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
71 changes: 71 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,70 @@
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)
.with("localhost")
.and_return(Resolv::IPv4.create("192.168.0.1"), Resolv::IPv4.create("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