From de2f13176288d8ff79d2dd61090f4dcee560aa27 Mon Sep 17 00:00:00 2001 From: Remy Mathieu Date: Tue, 27 Jun 2023 17:58:32 +0200 Subject: [PATCH 1/3] connection_cfg: add support for `DD_DOGSTATSD_URL` environment variable. --- lib/datadog/statsd/connection_cfg.rb | 66 +++++++++++++++++-- spec/statsd/connection_cfg_spec.rb | 98 +++++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 10 deletions(-) diff --git a/lib/datadog/statsd/connection_cfg.rb b/lib/datadog/statsd/connection_cfg.rb index c52b5a25..53e7bddd 100644 --- a/lib/datadog/statsd/connection_cfg.rb +++ b/lib/datadog/statsd/connection_cfg.rb @@ -23,12 +23,24 @@ def make_connection(**params) private + ERROR_MESSAGE = "Valid environment variables combination for connection configuration:\n" + + " - DD_DOGSTATSD_URL for UDP or UDS connection.\n" + + " Example for UDP: DD_DOGSTATSD_URL='udp://localhost:8125'\n" + + " Example for UDS: DD_DOGSTATSD_URL='unix:///path/to/unix.sock'\n" + + " or\n" + + " - DD_AGENT_HOST and DD_DOGSTATSD_PORT for an UDP connection. E.g. DD_AGENT_HOST='localhost' DD_DOGSTATSD_PORT=8125\n" + + " or\n" + + " - DD_DOGSTATSD_SOCKET for an UDS connection: E.g. DD_DOGSTATSD_SOCKET='/path/to/unix.sock'\n" + DEFAULT_HOST = '127.0.0.1' DEFAULT_PORT = 8125 + UDP_PREFIX = 'udp://' + UDS_PREFIX = 'unix://' + def initialize_with_constructor_args(host: nil, port: nil, socket_path: nil) try_initialize_with(host: host, port: port, socket_path: socket_path, - not_both_error_message: + error_message: "Both UDP: (host/port #{host}:#{port}) and UDS (socket_path #{socket_path}) " + "constructor arguments were given. Use only one or the other.", ) @@ -36,13 +48,11 @@ def initialize_with_constructor_args(host: nil, port: nil, socket_path: nil) def initialize_with_env_vars() try_initialize_with( + dogstatsd_url: ENV['DD_DOGSTATSD_URL'], host: ENV['DD_AGENT_HOST'], port: ENV['DD_DOGSTATSD_PORT'] && ENV['DD_DOGSTATSD_PORT'].to_i, socket_path: ENV['DD_DOGSTATSD_SOCKET'], - not_both_error_message: - "Both UDP (DD_AGENT_HOST/DD_DOGSTATSD_PORT #{ENV['DD_AGENT_HOST']}:#{ENV['DD_DOGSTATSD_PORT']}) " + - "and UDS (DD_DOGSTATSD_SOCKET #{ENV['DD_DOGSTATSD_SOCKET']}) environment variables are set. " + - "Set only one or the other.", + error_message: ERROR_MESSAGE, ) end @@ -50,9 +60,17 @@ def initialize_with_defaults() try_initialize_with(host: DEFAULT_HOST, port: DEFAULT_PORT) end - def try_initialize_with(host: nil, port: nil, socket_path: nil, not_both_error_message: "") + def try_initialize_with(dogstatsd_url: nil, host: nil, port: nil, socket_path: nil, error_message: ERROR_MESSAGE) + if dogstatsd_url && (host || port || socket_path) + raise ArgumentError, error_message + end + + if dogstatsd_url + host, port, socket_path = parse_dogstatsd_url(str: dogstatsd_url.to_s) + end + if (host || port) && socket_path - raise ArgumentError, not_both_error_message + raise ArgumentError, error_message end if host || port @@ -71,6 +89,40 @@ def try_initialize_with(host: nil, port: nil, socket_path: nil, not_both_error_m return false end + + def parse_dogstatsd_url(str:) + # udp socket connection + + if str.start_with?(UDP_PREFIX) + dogstatsd_url = str[UDP_PREFIX.size..] + host = nil + port = nil + + if dogstatsd_url.include?(":") + parts = dogstatsd_url.split(":") + if parts.size > 2 + raise ArgumentError, "Error: DD_DOGSTATSD_URL wrong format for an UDP connection. E.g. 'udp://localhost:8125'" + end + + host = parts[0] + port = parts[1].to_i + else + host = dogstatsd_url + end + + return host, port, nil + end + + # unix socket connection + + if str.start_with?(UDS_PREFIX) + return nil, nil, str[UDS_PREFIX.size..] + end + + # malformed value + + raise ArgumentError, "Error: DD_DOGSTATSD_URL has been provided but is not starting with 'udp://' nor 'unix://'" + end end end end diff --git a/spec/statsd/connection_cfg_spec.rb b/spec/statsd/connection_cfg_spec.rb index 9b7726a0..3e603f28 100644 --- a/spec/statsd/connection_cfg_spec.rb +++ b/spec/statsd/connection_cfg_spec.rb @@ -8,6 +8,7 @@ around do |example| ClimateControl.modify( 'DD_AGENT_HOST' => dd_agent_host, + 'DD_DOGSTATSD_URL' => dd_dogstatsd_url, 'DD_DOGSTATSD_PORT' => dd_dogstatsd_port, 'DD_DOGSTATSD_SOCKET' => dd_dogstatsd_socket, ) do @@ -19,6 +20,7 @@ let(:port) { nil } let(:socket_path) { nil } let(:dd_agent_host) { nil } + let(:dd_dogstatsd_url) { nil } let(:dd_dogstatsd_port) { nil } let(:dd_dogstatsd_socket) { nil } @@ -29,6 +31,7 @@ let(:dd_agent_host) { 'unused' } let(:dd_dogstatsd_port) { '999' } let(:dd_dogstatsd_socket) { '/un/used' } + let(:dd_dogstatsd_url) { 'unix://unused.sock' } it 'creates a UDP connection' do expect(subject.transport_type).to eq :udp @@ -144,6 +147,75 @@ end end + context 'with no args and a malformed DD_DOGSTATSD_URL' do + let(:dd_dogstatsd_url) { 'tcppp://somehost' } + it 'raises an exception' do + expect do + subject.new(dogstatsd_url: dogstatsd_url, host: host, port: port, socket_path: socket_path) + end.to raise_error(ArgumentError) + end + end + + context 'with no args and DD_DOGSTATSD_URL set for UDP connection without port' do + let(:dd_dogstatsd_url) { 'udp://somehost' } + + it 'creates an UDP connection' do + expect(subject.transport_type).to eq :udp + end + + it 'sets host' do + expect(subject.host).to eq 'somehost' + end + + it 'sets port to default port' do + expect(subject.port).to eq 8125 + end + + it 'sets socket_path to nil' do + expect(subject.socket_path).to eq nil + end + end + + context 'with no args and DD_DOGSTATSD_URL set for UDP connection with a port' do + let(:dd_dogstatsd_url) { 'udp://somehost:1111' } + + it 'creates an UDP connection' do + expect(subject.transport_type).to eq :udp + end + + it 'sets host' do + expect(subject.host).to eq 'somehost' + end + + it 'sets port' do + expect(subject.port).to eq 1111 + end + + it 'sets socket_path to nil' do + expect(subject.socket_path).to eq nil + end + end + + context 'with no args and DD_DOGSTATSD_URL set for UDS connection' do + let(:dd_dogstatsd_url) { 'unix:///path/to/some-unix.sock' } + + it 'creates an UDS connection' do + expect(subject.transport_type).to eq :uds + end + + it 'sets host to nil' do + expect(subject.host).to eq nil + end + + it 'sets port to nil' do + expect(subject.port).to eq nil + end + + it 'sets socket_path' do + expect(subject.socket_path).to eq '/path/to/some-unix.sock' + end + end + context 'with both DD_AGENT_HOST and DD_DOGSTATSD_SOCKET set' do let(:dd_agent_host) { 'some-host' } let(:dd_dogstatsd_socket) { '/some/socket' } @@ -151,9 +223,29 @@ it 'raises an exception' do expect do subject.new(host: host, port: port, socket_path: socket_path) - end.to raise_error( - ArgumentError, - 'Both UDP (DD_AGENT_HOST/DD_DOGSTATSD_PORT some-host:) and UDS (DD_DOGSTATSD_SOCKET /some/socket) environment variables are set. Set only one or the other.') + end.to raise_error(ArgumentError) + end + end + + context 'with both DD_DOGSTATSD_URL and DD_AGENT_HOST' do + let(:dd_agent_host) { 'some-host' } + let(:dd_dogstatsd_url) { 'udp://localhost' } + + it 'raises an exception' do + expect do + subject.new(dogstatsd_url: dogstatsd_url, host: host, port: port, socket_path: socket_path) + end.to raise_error(ArgumentError) + end + end + + context 'with both DD_DOGSTATSD_URL and DD_DOGSTATSD_SOCKET' do + let(:dd_agent_host) { 'some-host' } + let(:dd_dogstatsd_socket) { 'statsd.sock' } + + it 'raises an exception' do + expect do + subject.new(dogstatsd_url: dogstatsd_url, host: host, port: port, socket_path: socket_path) + end.to raise_error(ArgumentError) end end From f02ff3a76b53d341e322b41df0c79206af338b59 Mon Sep 17 00:00:00 2001 From: Remy Mathieu Date: Wed, 28 Jun 2023 10:47:05 +0200 Subject: [PATCH 2/3] Support older rubies version. --- lib/datadog/statsd/connection_cfg.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/datadog/statsd/connection_cfg.rb b/lib/datadog/statsd/connection_cfg.rb index 53e7bddd..b8274180 100644 --- a/lib/datadog/statsd/connection_cfg.rb +++ b/lib/datadog/statsd/connection_cfg.rb @@ -94,7 +94,7 @@ def parse_dogstatsd_url(str:) # udp socket connection if str.start_with?(UDP_PREFIX) - dogstatsd_url = str[UDP_PREFIX.size..] + dogstatsd_url = str[UDP_PREFIX.size..str.size] host = nil port = nil @@ -116,7 +116,7 @@ def parse_dogstatsd_url(str:) # unix socket connection if str.start_with?(UDS_PREFIX) - return nil, nil, str[UDS_PREFIX.size..] + return nil, nil, str[UDS_PREFIX.size..str.size] end # malformed value From 8ffee4a83ffbd6688e010a064000eb3867ff74f6 Mon Sep 17 00:00:00 2001 From: Remy Mathieu Date: Wed, 28 Jun 2023 11:39:10 +0200 Subject: [PATCH 3/3] connection_cfg: DD_DOGSTATSD_URL has priority on other environment vars. --- lib/datadog/statsd/connection_cfg.rb | 9 ++-- spec/statsd/connection_cfg_spec.rb | 78 ++++++++++++++++++++++++---- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/lib/datadog/statsd/connection_cfg.rb b/lib/datadog/statsd/connection_cfg.rb index b8274180..0259041c 100644 --- a/lib/datadog/statsd/connection_cfg.rb +++ b/lib/datadog/statsd/connection_cfg.rb @@ -30,7 +30,8 @@ def make_connection(**params) " or\n" + " - DD_AGENT_HOST and DD_DOGSTATSD_PORT for an UDP connection. E.g. DD_AGENT_HOST='localhost' DD_DOGSTATSD_PORT=8125\n" + " or\n" + - " - DD_DOGSTATSD_SOCKET for an UDS connection: E.g. DD_DOGSTATSD_SOCKET='/path/to/unix.sock'\n" + " - DD_DOGSTATSD_SOCKET for an UDS connection: E.g. DD_DOGSTATSD_SOCKET='/path/to/unix.sock'\n" + + " Note that DD_DOGSTATSD_URL has priority on other environment variables." DEFAULT_HOST = '127.0.0.1' DEFAULT_PORT = 8125 @@ -61,7 +62,7 @@ def initialize_with_defaults() end def try_initialize_with(dogstatsd_url: nil, host: nil, port: nil, socket_path: nil, error_message: ERROR_MESSAGE) - if dogstatsd_url && (host || port || socket_path) + if (host || port) && socket_path raise ArgumentError, error_message end @@ -69,10 +70,6 @@ def try_initialize_with(dogstatsd_url: nil, host: nil, port: nil, socket_path: n host, port, socket_path = parse_dogstatsd_url(str: dogstatsd_url.to_s) end - if (host || port) && socket_path - raise ArgumentError, error_message - end - if host || port @host = host || DEFAULT_HOST @port = port || DEFAULT_PORT diff --git a/spec/statsd/connection_cfg_spec.rb b/spec/statsd/connection_cfg_spec.rb index 3e603f28..bc110a59 100644 --- a/spec/statsd/connection_cfg_spec.rb +++ b/spec/statsd/connection_cfg_spec.rb @@ -227,25 +227,81 @@ end end - context 'with both DD_DOGSTATSD_URL and DD_AGENT_HOST' do + context 'with both DD_DOGSTATSD_URL and DD_AGENT_HOST, udp variant' do let(:dd_agent_host) { 'some-host' } + let(:dd_dogstatsd_port) { '1111' } let(:dd_dogstatsd_url) { 'udp://localhost' } - it 'raises an exception' do - expect do - subject.new(dogstatsd_url: dogstatsd_url, host: host, port: port, socket_path: socket_path) - end.to raise_error(ArgumentError) + # DD_DOGSTATSD_URL has priority + + it 'sets host' do + expect(subject.host).to eq 'localhost' + end + + it 'sets port' do + expect(subject.port).to eq 8125 + end + + it 'sets socket_path' do + expect(subject.socket_path).to eq nil end end - context 'with both DD_DOGSTATSD_URL and DD_DOGSTATSD_SOCKET' do + context 'with both DD_DOGSTATSD_URL and DD_DOGSTATSD_SOCKET, udp variant' do + let(:dd_dogstatsd_socket) { '/not/used.sock' } + let(:dd_dogstatsd_url) { 'udp://localhost:2222' } + + # DD_DOGSTATSD_URL has priority + + it 'sets host' do + expect(subject.host).to eq 'localhost' + end + + it 'sets port' do + expect(subject.port).to eq 2222 + end + + it 'sets socket_path' do + expect(subject.socket_path).to eq nil + end + end + + context 'with both DD_DOGSTATSD_URL and DD_AGENT_HOST, uds variant' do let(:dd_agent_host) { 'some-host' } - let(:dd_dogstatsd_socket) { 'statsd.sock' } + let(:dd_dogstatsd_port) { '1111' } + let(:dd_dogstatsd_url) { 'unix:///path/to/unix.sock' } - it 'raises an exception' do - expect do - subject.new(dogstatsd_url: dogstatsd_url, host: host, port: port, socket_path: socket_path) - end.to raise_error(ArgumentError) + # DD_DOGSTATSD_URL has priority + + it 'sets host to nil' do + expect(subject.host).to eq nil + end + + it 'sets port to nil' do + expect(subject.port).to eq nil + end + + it 'sets socket_path' do + expect(subject.socket_path).to eq '/path/to/unix.sock' + end + end + + context 'with both DD_DOGSTATSD_URL and DD_DOGSTATSD_SOCKET, uds variant' do + let(:dd_dogstatsd_socket) { '/not/used.sock' } + let(:dd_dogstatsd_url) { 'unix:///path/to/unix.sock' } + + # DD_DOGSTATSD_URL has priority + + it 'sets host to nil' do + expect(subject.host).to eq nil + end + + it 'sets port to nil' do + expect(subject.port).to eq nil + end + + it 'sets socket_path' do + expect(subject.socket_path).to eq '/path/to/unix.sock' end end