From fb6183a1783b14ece132b343e812ceb9259e9e6b Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 24 Oct 2024 10:05:08 -0400 Subject: [PATCH 1/7] use uds --- .../configuration/agent_settings_resolver.rb | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/lib/datadog/core/configuration/agent_settings_resolver.rb b/lib/datadog/core/configuration/agent_settings_resolver.rb index 057f5285d14..93592404380 100644 --- a/lib/datadog/core/configuration/agent_settings_resolver.rb +++ b/lib/datadog/core/configuration/agent_settings_resolver.rb @@ -232,7 +232,26 @@ def uds_fallback end def should_use_uds? - can_use_uds? && !mixed_http_and_uds? + # When we have mixed settings for http/https and uds, we print a warning + # and use the uds settings. + unless defined?(@mixed_http_and_uds) + @mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds? + if @mixed_http_and_uds + warn_if_configuration_mismatch( + [ + DetectedConfiguration.new( + friendly_name: 'configuration of hostname/port for http/https use', + value: "hostname: '#{hostname}', port: '#{port}'", + ), + DetectedConfiguration.new( + friendly_name: 'configuration for unix domain socket', + value: parsed_url.to_s, + ), + ] + ) + end + end + can_use_uds? end def can_use_uds? @@ -307,30 +326,6 @@ def unix_scheme?(uri) uri.scheme == 'unix' end - # When we have mixed settings for http/https and uds, we print a warning and ignore the uds settings - def mixed_http_and_uds? - return @mixed_http_and_uds if defined?(@mixed_http_and_uds) - - @mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds? - - if @mixed_http_and_uds - warn_if_configuration_mismatch( - [ - DetectedConfiguration.new( - friendly_name: 'configuration of hostname/port for http/https use', - value: "hostname: '#{hostname}', port: '#{port}'", - ), - DetectedConfiguration.new( - friendly_name: 'configuration for unix domain socket', - value: parsed_url.to_s, - ), - ] - ) - end - - @mixed_http_and_uds - end - # Represents a given configuration value and where we got it from class DetectedConfiguration attr_reader :friendly_name, :value From 8a873afc69747abb3f171f92263ce03a06d022a6 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 24 Oct 2024 11:44:13 -0400 Subject: [PATCH 2/7] update tests to match this new world order --- .../configuration/agent_settings_resolver.rb | 8 +-- .../agent_settings_resolver_spec.rb | 70 ++++++++----------- 2 files changed, 33 insertions(+), 45 deletions(-) diff --git a/lib/datadog/core/configuration/agent_settings_resolver.rb b/lib/datadog/core/configuration/agent_settings_resolver.rb index 93592404380..480c286419e 100644 --- a/lib/datadog/core/configuration/agent_settings_resolver.rb +++ b/lib/datadog/core/configuration/agent_settings_resolver.rb @@ -239,14 +239,14 @@ def should_use_uds? if @mixed_http_and_uds warn_if_configuration_mismatch( [ - DetectedConfiguration.new( - friendly_name: 'configuration of hostname/port for http/https use', - value: "hostname: '#{hostname}', port: '#{port}'", - ), DetectedConfiguration.new( friendly_name: 'configuration for unix domain socket', value: parsed_url.to_s, ), + DetectedConfiguration.new( + friendly_name: 'configuration of hostname/port for http/https use', + value: "hostname: '#{hostname}', port: '#{port}'", + ), ] ) end diff --git a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb index 33d40c71368..329df3a5ac8 100644 --- a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb +++ b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb @@ -170,32 +170,29 @@ context 'when there is a hostname specified along with uds configuration' do let(:with_agent_host) { 'custom-hostname' } - it 'prioritizes the http configuration' do - expect(resolver).to have_attributes(hostname: 'custom-hostname', adapter: :net_http) + it 'prioritizes the uds configuration' do + expect(resolver).to have_attributes(adapter: :unix) end - it 'logs a warning including the uds path' do + it 'logs a warning including the mismatching hostname' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*configuration for unix domain socket \("unix:.*/some/path"\)}) + .with(/Configuration mismatch:.*hostname: 'custom-hostname'.*/) resolver end - it 'does not include a uds_path in the configuration' do - expect(resolver).to have_attributes(uds_path: nil) + it 'includes a uds_path in the configuration' do + expect(resolver).to have_attributes(uds_path: '/some/path') end context 'when there is no port specified' do - it 'prioritizes the http configuration and uses the default port' do - expect(resolver).to have_attributes(port: 8126, hostname: 'custom-hostname', adapter: :net_http) + it 'prioritizes the uds configuration and ignores the default port' do + expect(resolver).to have_attributes(adapter: :unix) end - it 'logs a warning including the hostname and default port' do + it 'logs a warning including the uds path' do expect(logger).to receive(:warn) - .with(/ - Configuration\ mismatch:\ values\ differ\ between\ configuration.* - Using\ "hostname:\ 'custom-hostname',\ port:\ '8126'".* - /x) + .with(/Configuration mismatch.*Using "unix:\/some\/path"/) resolver end @@ -204,51 +201,45 @@ context 'when there is a port specified' do let(:with_agent_port) { 1234 } - it 'prioritizes the http configuration and uses the specified port' do - expect(resolver).to have_attributes(port: 1234, hostname: 'custom-hostname', adapter: :net_http) + it 'prioritizes the uds path' do + expect(resolver).to have_attributes(adapter: :unix) end - it 'logs a warning including the hostname and port' do + it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(/ - Configuration\ mismatch:\ values\ differ\ between\ configuration.* - Using\ "hostname:\ 'custom-hostname',\ port:\ '1234'".* - /x) + .with(/Configuration mismatch.*Using "unix:\/some\/path"/) resolver end end end - context 'when there is a port specified along with uds configuration' do + context 'when there is a port specified along with a uds configuration' do let(:with_agent_port) { 5678 } - it 'prioritizes the http configuration' do - expect(resolver).to have_attributes(port: 5678, adapter: :net_http) + it 'prioritizes the uds configuration' do + expect(resolver).to have_attributes(port: 5678, adapter: :unix) end - it 'logs a warning including the uds path' do + it 'logs a warning including the mismatching port' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*configuration for unix domain socket \("unix:.*/some/path"\)}) + .with(/Configuration mismatch:.*port: '5678'.*/) resolver end - it 'does not include a uds_path in the configuration' do - expect(resolver).to have_attributes(uds_path: nil) + it 'includes the uds path in the configuration' do + expect(resolver).to have_attributes(uds_path: '/some/path') end context 'when there is no hostname specified' do - it 'prioritizes the http configuration and uses the default hostname' do - expect(resolver).to have_attributes(port: 5678, hostname: '127.0.0.1', adapter: :net_http) + it 'prioritizes the uds configuration' do + expect(resolver).to have_attributes(port: 5678, hostname: nil, adapter: :unix) end - it 'logs a warning including the default hostname and port' do + it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(/ - Configuration\ mismatch:\ values\ differ\ between\ configuration.* - Using\ "hostname:\ '127.0.0.1',\ port:\ '5678'".* - /x) + .with(/Configuration mismatch.*Using "unix:\/some\/path"/) resolver end @@ -257,16 +248,13 @@ context 'when there is a hostname specified' do let(:with_agent_host) { 'custom-hostname' } - it 'prioritizes the http configuration and uses the specified hostname' do - expect(resolver).to have_attributes(port: 5678, hostname: 'custom-hostname', adapter: :net_http) + it 'prioritizes the uds configuration' do + expect(resolver).to have_attributes(adapter: :unix) end - it 'logs a warning including the hostname and port' do + it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(/ - Configuration\ mismatch:\ values\ differ\ between\ configuration.* - Using\ "hostname:\ 'custom-hostname',\ port:\ '5678'".* - /x) + .with(/Configuration mismatch.*Using "unix:\/some\/path"/) resolver end From a1460a92b8fc8c638c64e4aefe531865db6a8cc7 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 24 Oct 2024 15:46:16 -0400 Subject: [PATCH 3/7] fix test --- .../core/configuration/agent_settings_resolver_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb index 329df3a5ac8..5dacb2b9569 100644 --- a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb +++ b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb @@ -192,7 +192,7 @@ it 'logs a warning including the uds path' do expect(logger).to receive(:warn) - .with(/Configuration mismatch.*Using "unix:\/some\/path"/) + .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) resolver end @@ -207,7 +207,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(/Configuration mismatch.*Using "unix:\/some\/path"/) + .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) resolver end @@ -223,7 +223,7 @@ it 'logs a warning including the mismatching port' do expect(logger).to receive(:warn) - .with(/Configuration mismatch:.*port: '5678'.*/) + .with(/Configuration mismatch:.*port: '5678'.*/) resolver end @@ -239,7 +239,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(/Configuration mismatch.*Using "unix:\/some\/path"/) + .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) resolver end @@ -254,7 +254,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(/Configuration mismatch.*Using "unix:\/some\/path"/) + .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) resolver end From f1a79bc960bab19823c7841d26a1d8693c98770f Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 25 Oct 2024 11:14:21 -0400 Subject: [PATCH 4/7] add back mixed_http_and_uds --- lib/datadog/core/configuration/agent_settings_resolver.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/datadog/core/configuration/agent_settings_resolver.rb b/lib/datadog/core/configuration/agent_settings_resolver.rb index 480c286419e..96262fe5b21 100644 --- a/lib/datadog/core/configuration/agent_settings_resolver.rb +++ b/lib/datadog/core/configuration/agent_settings_resolver.rb @@ -234,6 +234,11 @@ def uds_fallback def should_use_uds? # When we have mixed settings for http/https and uds, we print a warning # and use the uds settings. + mixed_http_and_uds + can_use_uds? + end + + def mixed_http_and_uds unless defined?(@mixed_http_and_uds) @mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds? if @mixed_http_and_uds @@ -251,7 +256,6 @@ def should_use_uds? ) end end - can_use_uds? end def can_use_uds? From 0b83cab12435a106c9d0f22db03dd56a65ed95fb Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 25 Oct 2024 14:09:12 -0400 Subject: [PATCH 5/7] fix unix path regex --- .../core/configuration/agent_settings_resolver_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb index 5dacb2b9569..adc2b645569 100644 --- a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb +++ b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb @@ -192,7 +192,7 @@ it 'logs a warning including the uds path' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) resolver end @@ -207,7 +207,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) resolver end @@ -239,7 +239,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) resolver end @@ -254,7 +254,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[/|]some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) resolver end From dfc600f4a1a0e30d1edea1e8d0f6032315495258 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 28 Oct 2024 09:42:23 -0400 Subject: [PATCH 6/7] update method Co-authored-by: Sergey Fedorov --- .../configuration/agent_settings_resolver.rb | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/datadog/core/configuration/agent_settings_resolver.rb b/lib/datadog/core/configuration/agent_settings_resolver.rb index 96262fe5b21..fab5c874aa7 100644 --- a/lib/datadog/core/configuration/agent_settings_resolver.rb +++ b/lib/datadog/core/configuration/agent_settings_resolver.rb @@ -239,23 +239,25 @@ def should_use_uds? end def mixed_http_and_uds - unless defined?(@mixed_http_and_uds) - @mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds? - if @mixed_http_and_uds - warn_if_configuration_mismatch( - [ - DetectedConfiguration.new( - friendly_name: 'configuration for unix domain socket', - value: parsed_url.to_s, - ), - DetectedConfiguration.new( - friendly_name: 'configuration of hostname/port for http/https use', - value: "hostname: '#{hostname}', port: '#{port}'", - ), - ] - ) - end + return @mixed_http_and_uds if defined?(@mixed_http_and_uds) + + @mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds? + if @mixed_http_and_uds + warn_if_configuration_mismatch( + [ + DetectedConfiguration.new( + friendly_name: 'configuration for unix domain socket', + value: parsed_url.to_s, + ), + DetectedConfiguration.new( + friendly_name: 'configuration of hostname/port for http/https use', + value: "hostname: '#{hostname}', port: '#{port}'", + ), + ] + ) end + + @mixed_http_and_uds end def can_use_uds? From ebaa0bf646ea57c0b5838f7ef57eb8d856301f06 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 1 Nov 2024 09:33:54 -0400 Subject: [PATCH 7/7] lint --- lib/datadog/core/configuration/agent_settings_resolver.rb | 4 ++-- .../core/configuration/agent_settings_resolver_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/datadog/core/configuration/agent_settings_resolver.rb b/lib/datadog/core/configuration/agent_settings_resolver.rb index fab5c874aa7..7d7c048f553 100644 --- a/lib/datadog/core/configuration/agent_settings_resolver.rb +++ b/lib/datadog/core/configuration/agent_settings_resolver.rb @@ -240,7 +240,7 @@ def should_use_uds? def mixed_http_and_uds return @mixed_http_and_uds if defined?(@mixed_http_and_uds) - + @mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds? if @mixed_http_and_uds warn_if_configuration_mismatch( @@ -256,7 +256,7 @@ def mixed_http_and_uds ] ) end - + @mixed_http_and_uds end diff --git a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb index adc2b645569..ef2a6766ea1 100644 --- a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb +++ b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb @@ -192,7 +192,7 @@ it 'logs a warning including the uds path' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass resolver end @@ -207,7 +207,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass resolver end @@ -239,7 +239,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass resolver end @@ -254,7 +254,7 @@ it 'logs a warning including the uds configuration' do expect(logger).to receive(:warn) - .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) + .with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass resolver end