-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[exporter/datadog] make error retryable if logs sender received nil response #28672
[exporter/datadog] make error retryable if logs sender received nil response #28672
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a high level I agree with this change, though AFAICT this can be a problem in 2 corner cases:
- May lead to duplicated logs being sent if the initial request went through but response is lost
- May increase the wait time if there's indeed a permanent error when exporter received nil response
WDYT? @dineshg13 @mx-psi
Just a quick note from my side. As we briefly discussed the migration to the logs agent should fix the issue as well - So I have taken a look on how it works in the logs agent. According to this source code the logs agent will retry a request on the error despite the fact empty/non-empty response. The only difference is that the logs agent source code also checks that the error is not a cancellation error triggered by the client (we can add this check as well). It looks like the described 2 cases can happen with the logs agent as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking, LGTM then
…esponse (open-telemetry#28672) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> The Datadog exporter threats network/connectivity errors (HTTP client doesn't receive a response) as permanent errors, which can lead to log records loss. This change makes these errors retryable. **Link to tracking Issue:** open-telemetry#24550 **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.>
…esponse (open-telemetry#28672) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> The Datadog exporter threats network/connectivity errors (HTTP client doesn't receive a response) as permanent errors, which can lead to log records loss. This change makes these errors retryable. **Link to tracking Issue:** open-telemetry#24550 **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.>
Description:
The Datadog exporter threats network/connectivity errors (HTTP client doesn't receive a response) as permanent errors, which can lead to log records loss. This change makes these errors retryable.
Link to tracking Issue: #24550
Testing:
Documentation: