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

[NO-TICKET] Fix error during telemetry debug logging attempt #3617

Merged
merged 1 commit into from
May 1, 2024

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented May 1, 2024

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.

**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.
@ivoanjo ivoanjo requested a review from a team as a code owner May 1, 2024 14:17
@github-actions github-actions bot added the core Involves Datadog core libraries label May 1, 2024
@ivoanjo ivoanjo added this to the 2.0.0 milestone May 1, 2024
@ivoanjo ivoanjo requested a review from a team May 1, 2024 14:28
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.13%. Comparing base (2be8d89) to head (a73b051).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3617   +/-   ##
=======================================
  Coverage   98.12%   98.13%           
=======================================
  Files        1223     1223           
  Lines       72136    72146   +10     
  Branches     3425     3425           
=======================================
+ Hits        70787    70797   +10     
  Misses       1349     1349           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marcotc marcotc merged commit e581d87 into master May 1, 2024
166 checks passed
@marcotc marcotc deleted the ivoanjo/fix-telemetry-debug-logging-error branch May 1, 2024 16:56
@TonyCTHsu TonyCTHsu mentioned this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants