diff --git a/Steepfile b/Steepfile index 24d49808692..d2cce9c3ef6 100644 --- a/Steepfile +++ b/Steepfile @@ -80,7 +80,6 @@ target :datadog do ignore 'lib/datadog/core/buffer/thread_safe.rb' ignore 'lib/datadog/core/chunker.rb' ignore 'lib/datadog/core/configuration.rb' - ignore 'lib/datadog/core/configuration/agent_settings_resolver.rb' ignore 'lib/datadog/core/configuration/base.rb' ignore 'lib/datadog/core/configuration/components.rb' ignore 'lib/datadog/core/configuration/dependency_resolver.rb' diff --git a/lib/datadog/core/configuration/agent_settings_resolver.rb b/lib/datadog/core/configuration/agent_settings_resolver.rb index 7d7c048f553..5ced057274a 100644 --- a/lib/datadog/core/configuration/agent_settings_resolver.rb +++ b/lib/datadog/core/configuration/agent_settings_resolver.rb @@ -19,21 +19,49 @@ module Configuration # Whenever there is a conflict (different configurations are provided in different orders), it MUST warn the users # about it and pick a value based on the following priority: code > environment variable > defaults. class AgentSettingsResolver - AgentSettings = Struct.new( - :adapter, - :ssl, - :hostname, - :port, - :uds_path, - :timeout_seconds, - keyword_init: true - ) do - def initialize(*) - super + # Immutable container for the resulting settings + class AgentSettings + attr_reader :adapter, :ssl, :hostname, :port, :uds_path, :timeout_seconds + + def initialize(adapter: nil, ssl: nil, hostname: nil, port: nil, uds_path: nil, timeout_seconds: nil) + @adapter = adapter + @ssl = ssl + @hostname = hostname + @port = port + @uds_path = uds_path + @timeout_seconds = timeout_seconds freeze end + + def url + case adapter + when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER + hostname = self.hostname + hostname = "[#{hostname}]" if hostname =~ IPV6_REGEXP + "#{ssl ? 'https' : 'http'}://#{hostname}:#{port}/" + when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER + "unix://#{uds_path}" + else + raise ArgumentError, "Unexpected adapter: #{adapter}" + end + end + + def ==(other) + self.class == other.class && + adapter == other.adapter && + ssl == other.ssl && + hostname == other.hostname && + port == other.port && + uds_path == other.uds_path && + timeout_seconds == other.timeout_seconds + end end + # IPv6 regular expression from + # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses + # Does not match IPv4 addresses. + IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/.freeze # rubocop:disable Layout/LineLength + def self.call(settings, logger: Datadog.logger) new(settings, logger: logger).send(:call) end diff --git a/lib/datadog/core/crashtracking/agent_base_url.rb b/lib/datadog/core/crashtracking/agent_base_url.rb deleted file mode 100644 index 74b59a1cda5..00000000000 --- a/lib/datadog/core/crashtracking/agent_base_url.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -require_relative '../configuration/ext' - -module Datadog - module Core - module Crashtracking - # This module provides a method to resolve the base URL of the agent - module AgentBaseUrl - # IPv6 regular expression from - # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses - # Does not match IPv4 addresses. - IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/.freeze # rubocop:disable Layout/LineLength - - def self.resolve(agent_settings) - case agent_settings.adapter - when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER - hostname = agent_settings.hostname - hostname = "[#{hostname}]" if hostname =~ IPV6_REGEXP - "#{agent_settings.ssl ? 'https' : 'http'}://#{hostname}:#{agent_settings.port}/" - when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER - "unix://#{agent_settings.uds_path}" - end - end - end - end - end -end diff --git a/lib/datadog/core/crashtracking/component.rb b/lib/datadog/core/crashtracking/component.rb index ee04e1c5cc8..460a7974bc5 100644 --- a/lib/datadog/core/crashtracking/component.rb +++ b/lib/datadog/core/crashtracking/component.rb @@ -3,7 +3,6 @@ require 'libdatadog' require_relative 'tag_builder' -require_relative 'agent_base_url' require_relative '../utils/only_once' require_relative '../utils/at_fork_monkey_patch' @@ -31,8 +30,7 @@ class Component def self.build(settings, agent_settings, logger:) tags = TagBuilder.call(settings) - agent_base_url = AgentBaseUrl.resolve(agent_settings) - logger.warn('Missing agent base URL; cannot enable crash tracking') unless agent_base_url + agent_base_url = agent_settings.url ld_library_path = ::Libdatadog.ld_library_path logger.warn('Missing ld_library_path; cannot enable crash tracking') unless ld_library_path diff --git a/lib/datadog/profiling/http_transport.rb b/lib/datadog/profiling/http_transport.rb index 2c89c6548b7..9de76899494 100644 --- a/lib/datadog/profiling/http_transport.rb +++ b/lib/datadog/profiling/http_transport.rb @@ -13,13 +13,11 @@ class HttpTransport def initialize(agent_settings:, site:, api_key:, upload_timeout_seconds:) @upload_timeout_milliseconds = (upload_timeout_seconds * 1_000).to_i - validate_agent_settings(agent_settings) - @exporter_configuration = if agentless?(site, api_key) [:agentless, site, api_key].freeze else - [:agent, base_url_from(agent_settings)].freeze + [:agent, agent_settings.url].freeze end status, result = validate_exporter(exporter_configuration) @@ -75,29 +73,6 @@ def export(flush) private - def base_url_from(agent_settings) - case agent_settings.adapter - when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER - "#{agent_settings.ssl ? "https" : "http"}://#{agent_settings.hostname}:#{agent_settings.port}/" - when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER - "unix://#{agent_settings.uds_path}" - else - raise ArgumentError, "Unexpected adapter: #{agent_settings.adapter}" - end - end - - def validate_agent_settings(agent_settings) - supported_adapters = [ - Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER, - Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER - ] - unless supported_adapters.include?(agent_settings.adapter) - raise ArgumentError, - "Unsupported transport configuration for profiling: Adapter #{agent_settings.adapter} " \ - " is not supported" - end - end - def agentless?(site, api_key) site && api_key && Core::Environment::VariableHelpers.env_to_bool(Profiling::Ext::ENV_AGENTLESS, false) end diff --git a/sig/datadog/core/configuration/agent_settings_resolver.rbs b/sig/datadog/core/configuration/agent_settings_resolver.rbs index 040a9aa960a..e3a2472abf4 100644 --- a/sig/datadog/core/configuration/agent_settings_resolver.rbs +++ b/sig/datadog/core/configuration/agent_settings_resolver.rbs @@ -2,9 +2,8 @@ module Datadog module Core module Configuration class AgentSettingsResolver - class AgentSettings < ::Struct[untyped] + class AgentSettings def initialize: (adapter: untyped, ssl: untyped, hostname: untyped, port: untyped, uds_path: untyped, timeout_seconds: untyped) -> void - def merge: (**::Hash[untyped, untyped] member_values) -> AgentSettingsResolver attr_reader adapter: untyped attr_reader ssl: untyped @@ -12,8 +11,26 @@ module Datadog attr_reader port: untyped attr_reader uds_path: untyped attr_reader timeout_seconds: untyped + + def url: () -> ::String end + @settings: untyped + @logger: untyped + @configured_hostname: untyped + @configured_port: untyped + @configured_ssl: untyped + @configured_timeout_seconds: untyped + @configured_uds_path: untyped + @uds_fallback: untyped + @mixed_http_and_uds: untyped + @parsed_url: untyped + + # IPv6 regular expression from + # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses + # Does not match IPv4 addresses. + IPV6_REGEXP: ::Regexp + def self.call: (untyped settings, ?logger: untyped) -> untyped private @@ -38,33 +55,31 @@ module Datadog def configured_uds_path: () -> untyped - def try_parsing_as_integer: (value: untyped, friendly_name: untyped) -> untyped + def parsed_url_ssl?: () -> (nil | untyped) - def try_parsing_as_boolean: (value: untyped, friendly_name: untyped) -> untyped + def try_parsing_as_integer: (value: untyped, friendly_name: untyped) -> untyped - def ssl?: () -> bool + def ssl?: () -> (false | untyped) def hostname: () -> untyped def port: () -> untyped - def uds_path: () -> untyped - def timeout_seconds: () -> untyped - def uds_fallback: () -> untyped + def parsed_url_uds_path: () -> (nil | untyped) - def should_use_uds_fallback?: () -> untyped + def uds_path: () -> (nil | untyped) - def should_use_uds?: () -> bool + def uds_fallback: () -> untyped - def can_use_uds?: () -> bool + def should_use_uds?: () -> untyped - def parsed_url: () -> untyped + def mixed_http_and_uds: () -> untyped - def parsed_url_ssl?: () -> untyped + def can_use_uds?: () -> untyped - def parsed_url_uds_path: () -> untyped + def parsed_url: () -> untyped def pick_from: (*untyped configurations_in_priority_order) -> untyped @@ -72,7 +87,17 @@ module Datadog def log_warning: (untyped message) -> (untyped | nil) + def http_scheme?: (untyped uri) -> untyped + + def parsed_http_url: () -> (untyped | nil) + + def unix_scheme?: (untyped uri) -> untyped + class DetectedConfiguration + @friendly_name: untyped + + @value: untyped + attr_reader friendly_name: untyped attr_reader value: untyped diff --git a/sig/datadog/core/configuration/ext.rbs b/sig/datadog/core/configuration/ext.rbs index 71b7acc0951..77f9eae8265 100644 --- a/sig/datadog/core/configuration/ext.rbs +++ b/sig/datadog/core/configuration/ext.rbs @@ -15,6 +15,7 @@ module Datadog end module Agent + ENV_DEFAULT_HOST: 'DD_AGENT_HOST' ENV_DEFAULT_PORT: 'DD_TRACE_AGENT_PORT' ENV_DEFAULT_URL: 'DD_TRACE_AGENT_URL' ENV_DEFAULT_TIMEOUT_SECONDS: 'DD_TRACE_AGENT_TIMEOUT_SECONDS' diff --git a/sig/datadog/core/crashtracking/agent_base_url.rbs b/sig/datadog/core/crashtracking/agent_base_url.rbs deleted file mode 100644 index 3a2c77f8e85..00000000000 --- a/sig/datadog/core/crashtracking/agent_base_url.rbs +++ /dev/null @@ -1,10 +0,0 @@ -module Datadog - module Core - module Crashtracking - module AgentBaseUrl - IPV6_REGEXP: Regexp - def self.resolve: (Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings) -> ::String? - end - end - end -end diff --git a/sig/datadog/profiling/http_transport.rbs b/sig/datadog/profiling/http_transport.rbs index 8c58ea8180e..af663a225b7 100644 --- a/sig/datadog/profiling/http_transport.rbs +++ b/sig/datadog/profiling/http_transport.rbs @@ -19,10 +19,6 @@ module Datadog private - def base_url_from: (Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings) -> ::String - - def validate_agent_settings: (Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings) -> void - def agentless?: (::String? site, ::String? api_key) -> bool def validate_exporter: (exporter_configuration_array exporter_configuration) -> [:ok | :error, ::String?] diff --git a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb index ef2a6766ea1..c057e520739 100644 --- a/spec/datadog/core/configuration/agent_settings_resolver_spec.rb +++ b/spec/datadog/core/configuration/agent_settings_resolver_spec.rb @@ -803,4 +803,61 @@ end end end + + describe 'url' do + context 'when using HTTP adapter' do + before do + datadog_settings.agent.host = 'example.com' + datadog_settings.agent.port = 8080 + end + + context 'when SSL is enabled' do + before { datadog_settings.agent.use_ssl = true } + + it 'returns the correct base URL' do + expect(resolver.url).to eq('https://example.com:8080/') + end + end + + context 'when SSL is disabled' do + before { datadog_settings.agent.use_ssl = false } + + it 'returns the correct base URL' do + expect(resolver.url).to eq('http://example.com:8080/') + end + end + + context 'when hostname is an IPv4 address' do + before { datadog_settings.agent.host = '1.2.3.4' } + + it 'returns the correct base URL' do + expect(resolver.url).to eq('http://1.2.3.4:8080/') + end + end + + context 'when hostname is an IPv6 address' do + before { datadog_settings.agent.host = '1234:1234::1' } + + it 'returns the correct base URL' do + expect(resolver.url).to eq('http://[1234:1234::1]:8080/') + end + end + end + + context 'when using UnixSocket adapter' do + before { datadog_settings.agent.uds_path = '/var/run/datadog.sock' } + + it 'returns the correct base URL' do + expect(resolver.url).to eq('unix:///var/run/datadog.sock') + end + end + + context 'when using an unknown adapter' do + it 'raises an exception' do + agent_settings = Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new(adapter: :unknown) + + expect { agent_settings.url }.to raise_error(ArgumentError, /Unexpected adapter/) + end + end + end end diff --git a/spec/datadog/core/crashtracking/agent_base_url_spec.rb b/spec/datadog/core/crashtracking/agent_base_url_spec.rb deleted file mode 100644 index 407b74daf26..00000000000 --- a/spec/datadog/core/crashtracking/agent_base_url_spec.rb +++ /dev/null @@ -1,98 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require 'datadog/core/crashtracking/agent_base_url' - -RSpec.describe Datadog::Core::Crashtracking::AgentBaseUrl do - describe '.resolve' do - context 'when using HTTP adapter' do - context 'when SSL is enabled' do - let(:agent_settings) do - instance_double( - Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, - adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, - ssl: true, - hostname: 'example.com', - port: 8080 - ) - end - - it 'returns the correct base URL' do - expect(described_class.resolve(agent_settings)).to eq('https://example.com:8080/') - end - end - - context 'when SSL is disabled' do - let(:agent_settings) do - instance_double( - Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, - adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, - ssl: false, - hostname: 'example.com', - port: 8080 - ) - end - - it 'returns the correct base URL' do - expect(described_class.resolve(agent_settings)).to eq('http://example.com:8080/') - end - end - - context 'when hostname is an IPv4 address' do - let(:agent_settings) do - instance_double( - Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, - adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, - ssl: false, - hostname: '1.2.3.4', - port: 8080 - ) - end - - it 'returns the correct base URL' do - expect(described_class.resolve(agent_settings)).to eq('http://1.2.3.4:8080/') - end - end - - context 'when hostname is an IPv6 address' do - let(:agent_settings) do - instance_double( - Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, - adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, - ssl: false, - hostname: '1234:1234::1', - port: 8080 - ) - end - - it 'returns the correct base URL' do - expect(described_class.resolve(agent_settings)).to eq('http://[1234:1234::1]:8080/') - end - end - end - - context 'when using UnixSocket adapter' do - let(:agent_settings) do - instance_double( - Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, - adapter: Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER, - uds_path: '/var/run/datadog.sock' - ) - end - - it 'returns the correct base URL' do - expect(described_class.resolve(agent_settings)).to eq('unix:///var/run/datadog.sock') - end - end - - context 'when using unknownm adapter' do - let(:agent_settings) do - instance_double(Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, adapter: 'unknown') - end - - it 'returns nil' do - expect(described_class.resolve(agent_settings)).to be_nil - end - end - end -end diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index 034bd5f441e..fcbef942d43 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -9,7 +9,9 @@ describe '.build' do let(:settings) { Datadog::Core::Configuration::Settings.new } - let(:agent_settings) { double('agent_settings') } + let(:agent_settings) do + instance_double(Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings) + end let(:tags) { { 'tag1' => 'value1' } } let(:agent_base_url) { 'agent_base_url' } let(:ld_library_path) { 'ld_library_path' } @@ -19,8 +21,7 @@ it 'creates a new instance of Component and starts it' do expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) .and_return(tags) - expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) - .and_return(agent_base_url) + expect(agent_settings).to receive(:url).and_return(agent_base_url) expect(::Libdatadog).to receive(:ld_library_path) .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) @@ -42,32 +43,13 @@ end end - context 'when missing `agent_base_url`' do - let(:agent_base_url) { nil } - - it 'returns nil' do - expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) - .and_return(tags) - expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) - .and_return(agent_base_url) - expect(::Libdatadog).to receive(:ld_library_path) - .and_return(ld_library_path) - expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) - .and_return(path_to_crashtracking_receiver_binary) - expect(logger).to receive(:warn).with(/cannot enable crash tracking/) - - expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil - end - end - context 'when missing `ld_library_path`' do let(:ld_library_path) { nil } it 'returns nil' do expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) .and_return(tags) - expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) - .and_return(agent_base_url) + expect(agent_settings).to receive(:url).and_return(agent_base_url) expect(::Libdatadog).to receive(:ld_library_path) .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) @@ -84,8 +66,7 @@ it 'returns nil' do expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) .and_return(tags) - expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) - .and_return(agent_base_url) + expect(agent_settings).to receive(:url).and_return(agent_base_url) expect(::Libdatadog).to receive(:ld_library_path) .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) @@ -102,8 +83,7 @@ it 'returns an instance of Component that failed to start' do expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) .and_return(tags) - expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) - .and_return(agent_base_url) + expect(agent_settings).to receive(:url).and_return(agent_base_url) expect(::Libdatadog).to receive(:ld_library_path) .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) diff --git a/spec/datadog/profiling/http_transport_spec.rb b/spec/datadog/profiling/http_transport_spec.rb index 44423984fab..0cd98ba09e3 100644 --- a/spec/datadog/profiling/http_transport_spec.rb +++ b/spec/datadog/profiling/http_transport_spec.rb @@ -127,6 +127,19 @@ http_transport end end + + context "when hostname is an ipv6 address" do + let(:hostname) { "1234:1234::1" } + + it "provides the correct ipv6 address-safe url to the exporter" do + expect(described_class) + .to receive(:_native_validate_exporter) + .with([:agent, "http://[1234:1234::1]:12345/"]) + .and_return([:ok, nil]) + + http_transport + end + end end context "when additionally site and api_key are provided" do