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

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

Merged
merged 6 commits into from
Jan 3, 2025
Merged
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
1 change: 0 additions & 1 deletion Steepfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
50 changes: 39 additions & 11 deletions lib/datadog/core/configuration/agent_settings_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 0 additions & 28 deletions lib/datadog/core/crashtracking/agent_base_url.rb

This file was deleted.

4 changes: 1 addition & 3 deletions lib/datadog/core/crashtracking/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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
Expand Down
27 changes: 1 addition & 26 deletions lib/datadog/profiling/http_transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
53 changes: 39 additions & 14 deletions sig/datadog/core/configuration/agent_settings_resolver.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions sig/datadog/core/configuration/ext.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
10 changes: 0 additions & 10 deletions sig/datadog/core/crashtracking/agent_base_url.rbs

This file was deleted.

4 changes: 0 additions & 4 deletions sig/datadog/profiling/http_transport.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -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?]
Expand Down
57 changes: 57 additions & 0 deletions spec/datadog/core/configuration/agent_settings_resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading
Loading