Skip to content

Commit

Permalink
Minor: Improve Datadog::Profiling::HttpTransport error logging
Browse files Browse the repository at this point in the history
**What does this PR do?**:

This PR tweaks the `HttpTransport` error logging to include the
configuration being used.

For instance, it will now look like:

```
ERROR (lib/datadog/profiling/http_transport.rb:58:in `export') Failed to
report profiling data ({:agent=>"unix:///fake-apm-socket.socket"}):
failed ddog_prof_Exporter_send: error trying to connect: No such file
or directory (os error 2): No such file or directory (os error 2)
```

...I did notice the libdatadog error message seems to be repeated and
isn't particularly great; that's a separate issue for a future fix
(on the libdatadog side).

**Motivation**:

While investigating #3037 I noticed this information was not being
logged when the profiler failed to report, and I thought it would be
come in handy when debugging future issues.

**Additional Notes**:

N/A

**How to test the change?**:

The change includes test coverage. You can also manually trigger an
issue by running dd-trace-rb with an invalid configuration, e.g.
the above error message was triggered by running with

```bash
$ DD_PROFILING_ENABLED=true DD_TRACE_AGENT_URL=unix:///fake-apm-socket.socket \
  bundle exec ddtracerb exec ruby -e "sleep 1"
```
  • Loading branch information
ivoanjo committed Aug 9, 2023
1 parent b1bec43 commit 1212ece
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 9 deletions.
11 changes: 9 additions & 2 deletions lib/datadog/profiling/http_transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,14 @@ def export(flush)
Datadog.logger.debug('Successfully reported profiling data')
true
else
Datadog.logger.error("Failed to report profiling data: server returned unexpected HTTP #{result} status code")
Datadog.logger.error(
"Failed to report profiling data (#{config_without_api_key}): " \
"server returned unexpected HTTP #{result} status code"
)
false
end
else
Datadog.logger.error("Failed to report profiling data: #{result}")
Datadog.logger.error("Failed to report profiling data (#{config_without_api_key}): #{result}")
false
end
end
Expand Down Expand Up @@ -128,6 +131,10 @@ def do_export(
internal_metadata_json,
)
end

def config_without_api_key
[@exporter_configuration[0..1]].to_h
end
end
end
end
2 changes: 2 additions & 0 deletions sig/datadog/profiling/http_transport.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ module Datadog
Array[[::String, ::String]] tags_as_array,
::String internal_metadata_json,
) -> [:ok | :error, ::Integer | ::String]

def config_without_api_key: () -> ::Hash[:agent | :agentless, ::String]
end
end
end
60 changes: 53 additions & 7 deletions spec/datadog/profiling/http_transport_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,64 @@
end

context 'when failed' do
before do
expect(described_class).to receive(:_native_do_export).and_return([:ok, 500])
allow(Datadog.logger).to receive(:error)
context 'with a http status code' do
before do
expect(described_class).to receive(:_native_do_export).and_return([:ok, 500])
allow(Datadog.logger).to receive(:error)
end

it 'logs an error message' do
expect(Datadog.logger).to receive(:error).with(
'Failed to report profiling data ({:agent=>"http://192.168.0.1:12345/"}): ' \
'server returned unexpected HTTP 500 status code'
)

export
end

it { is_expected.to be false }
end

it 'logs an error message' do
expect(Datadog.logger).to receive(:error)
context 'with a failure without an http status code' do
before do
expect(described_class).to receive(:_native_do_export).and_return([:error, 'Some error message'])
allow(Datadog.logger).to receive(:error)
end

export
it 'logs an error message' do
expect(Datadog.logger).to receive(:error)
.with('Failed to report profiling data ({:agent=>"http://192.168.0.1:12345/"}): Some error message')

export
end

it { is_expected.to be false }
end
end
end

describe '#config_without_api_key' do
subject(:config_without_api_key) { http_transport.send(:config_without_api_key) }

context 'when using agentless mode' do
let(:site) { 'test.datadoghq.com' }
let(:api_key) { SecureRandom.uuid }

around do |example|
ClimateControl.modify('DD_PROFILING_AGENTLESS' => 'true') do
example.run
end
end

it 'returns the mode and site, but not the api key' do
is_expected.to eq(agentless: 'test.datadoghq.com')
end
end

it { is_expected.to be false }
context 'when using agent mode' do
it 'returns the mode the agent url' do
is_expected.to eq(agent: 'http://192.168.0.1:12345/')
end
end
end

Expand Down

0 comments on commit 1212ece

Please sign in to comment.