Skip to content

Commit

Permalink
[PROF-11078] Fix profiling exception when agent url is an ipv6 address
Browse files Browse the repository at this point in the history
**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 DataDog/libdatadog#283 )
  • Loading branch information
ivoanjo committed Jan 2, 2025
1 parent 65c92d2 commit e4f5e19
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 30 deletions.
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
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
13 changes: 13 additions & 0 deletions spec/datadog/profiling/http_transport_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e4f5e19

Please sign in to comment.