From e4f5e19d9007907c3c17e6f2c5bb871a950e6288 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Thu, 2 Jan 2025 17:32:49 +0000 Subject: [PATCH] [PROF-11078] Fix profiling exception when agent url is an ipv6 address **What does this PR do?** This PR builds atop #4237 and fixes a similar-ish issue in the profiler caused by the same mishandling of ipv6 addresses. In particular, when provided with an ipv6 address in the agent url, the profiler would fail with an exception: ``` $ env DD_AGENT_HOST=2001:db8:1::2 DD_PROFILING_ENABLED=true \ bundle exec ddprofrb exec ruby -e "sleep 2" dd-trace-rb/lib/datadog/profiling/http_transport.rb:27:in `initialize': Failed to initialize transport: invalid authority (ArgumentError) ``` **Motivation:** Luckily we didn't have any customers using this, as it fails immediately and loudly, but it's still a bug on a configuration that should be supported. **Additional Notes:** Since we had similar buggy logic copy-pasted in crashtracking and profiling (crashtracking had been fixed in #4237) I chose to extract out the relevant logic into the `AgentSettings` class, so that both can reuse it. **How to test the change?** I've added unit test coverage for this issue to profiling, and the snippet above can be used to end-to-end test it's working fine. Here's how it looks on my machine now: ``` E, [2025-01-02T17:32:32.398756 #359317] ERROR -- datadog: [datadog] (dd-trace-rb/lib/datadog/profiling/http_transport.rb:68:in `export') Failed to report profiling data (agent: http://[2001:db8:1::2]:8126/): failed ddog_prof_Exporter_send: error trying to connect: tcp connect error: Network is unreachable (os error 101): tcp connect error: Network is unreachable (os error 101): Network is unreachable (os error 101) ``` E.g. we correctly try to connect to the dummy address, and fail :) (Note: The error message is a bit ugly AND repeats itself a bit. That's being tracked separately in https://github.com/DataDog/libdatadog/issues/283 ) --- lib/datadog/profiling/http_transport.rb | 27 +------------------ sig/datadog/profiling/http_transport.rbs | 4 --- spec/datadog/profiling/http_transport_spec.rb | 13 +++++++++ 3 files changed, 14 insertions(+), 30 deletions(-) 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/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/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