From a73b051a9ab2944f5550a6d390a096c55abfd373 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 1 May 2024 15:08:00 +0100 Subject: [PATCH] [NO-TICKET] Fix error during telemetry debug logging attempt **What does this PR do?** This PR fixes the following debug error log that would happen when enabling debug logging + not having an agent running locally. ``` $ DD_TRACE_DEBUG=true bundle exec ruby -e "require 'datadog'; Datadog.configure {}; sleep 1" ... DEBUG -- datadog: [datadog] (lib/datadog/core/telemetry/http/adapters/net.rb:47:in `rescue in post') Unable to send telemetry event to agent: Failed to open TCP connection to 127.0.0.1:8126 (Connection refused - connect(2) for "127.0.0.1" port 8126) DEBUG -- datadog: [datadog] (lib/datadog/core/telemetry/emitter.rb:32:in `rescue in request') Unable to send telemetry request for event `app-started`: undefined method `code' for Datadog::Core::Telemetry::Http::InternalErrorResponse ok?: unsupported?:, not_found?:, client_error?:, server_error?:, internal_error?:true, payload:, error_type:Errno::ECONNREFUSED error:Failed to open TCP connection to 127.0.0.1:8126 (Connection refused - connect(2) for "127.0.0.1" port 8126):Datadog::Core::Telemetry::Http::InternalErrorResponse Datadog.logger.debug { "Telemetry sent for event `#{event.type}` (status code: #{res.code})" } ^^^^^ ``` Specifically, the class used to communicate errors (`Datadog::Core::Telemetry::Http::Response::InternalErrorResponse`) did not define a `#code` accessor, thus making the log message that tries to debug the response code fail. **Motivation:** Fix this error. **Additional Notes:** The error itself was annoying, but harmless, as we did catch the exception anyway. I think the current approach of having a `Response` module that then gets inserted into a few classes is a bit error prone. And this is one where we definitely would've been saved by having type signatures for this part of the code. **How to test the change?** I've added unit coverage + you can use the example above to confirm it no longer happens manually. --- lib/datadog/core/telemetry/emitter.rb | 2 +- lib/datadog/core/telemetry/http/response.rb | 4 ++++ spec/datadog/core/telemetry/emitter_spec.rb | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/datadog/core/telemetry/emitter.rb b/lib/datadog/core/telemetry/emitter.rb index 1bf1dca8333..c4a18c264ce 100644 --- a/lib/datadog/core/telemetry/emitter.rb +++ b/lib/datadog/core/telemetry/emitter.rb @@ -26,7 +26,7 @@ def request(event) seq_id = self.class.sequence.next payload = Request.build_payload(event, seq_id) res = @http_transport.request(request_type: event.type, payload: payload.to_json) - Datadog.logger.debug { "Telemetry sent for event `#{event.type}` (status code: #{res.code})" } + Datadog.logger.debug { "Telemetry sent for event `#{event.type}` (code: #{res.code.inspect})" } res rescue => e Datadog.logger.debug("Unable to send telemetry request for event `#{event.type rescue 'unknown'}`: #{e}") diff --git a/lib/datadog/core/telemetry/http/response.rb b/lib/datadog/core/telemetry/http/response.rb index 81a1025647b..696ecd1432d 100644 --- a/lib/datadog/core/telemetry/http/response.rb +++ b/lib/datadog/core/telemetry/http/response.rb @@ -34,6 +34,10 @@ def internal_error? nil end + def code + nil + end + def inspect "#{self.class} ok?:#{ok?} unsupported?:#{unsupported?}, " \ "not_found?:#{not_found?}, client_error?:#{client_error?}, " \ diff --git a/spec/datadog/core/telemetry/emitter_spec.rb b/spec/datadog/core/telemetry/emitter_spec.rb index 3aedf0cb1e3..e7373a5874f 100644 --- a/spec/datadog/core/telemetry/emitter_spec.rb +++ b/spec/datadog/core/telemetry/emitter_spec.rb @@ -74,6 +74,21 @@ expect(emitter.class.sequence.instance_variable_get(:@current)).to be(original_seq_id + 1) end end + + context 'when call is not successful and debug logging is enabled' do + let(:response) do + Datadog::Core::Telemetry::Http::InternalErrorResponse.new(StandardError.new('Failed call')) + end + + it 'logs the request correctly' do + log_message = nil + expect(Datadog.logger).to receive(:debug) { |&message| log_message = message.call } + + request + + expect(log_message).to match('Telemetry sent for event') + end + end end end