Merge pull request #4252 from DataDog/ivoanjo/prof-11078-profiler-ipv…

…6-address

[PROF-11078] Fix profiling not working when agent url is an ipv6 address

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'

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

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

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

sig/datadog/core/configuration/agent_settings_resolver.rbs

@@ -2,18 +2,35 @@ 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
           attr_reader hostname: untyped
           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,41 +55,49 @@ 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
 
         def warn_if_configuration_mismatch: (untyped detected_configurations_in_priority_order) -> (nil | untyped)
 
         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

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'

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?]

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