Conversation
write(chunk) swallowed all exceptions (rescue Exception, no re-raise), so raw errors like Net::ReadTimeout/EOFError bypassed the plugin's own RetryableError-only retry path, and write() returning normally meant Fluentd core's buffer retry never saw a failure either. On a transient network blip the in-flight batch was silently dropped with no retry from either side. send_retries had the same issue on retry exhaustion. Fixes AGENT-16861.
ian28223
marked this pull request as ready for review
August 26, 2026 07:44
Collaborator
|
Incorporated in to #92, which adds the option to more aggressively retry a subset of error cases before finally raising the exception. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
write(chunk)currently wraps its entire body inrescue Exception => e ... endwith no re-raise. Raw exceptions likeNet::ReadTimeoutorEOFErrorbypass the plugin's ownsend_retries(which only rescuesRetryableError, raised for HTTP 5xx/429), and sincewrite()returns normally regardless, Fluentd core's own buffer retry (update_retry_state/try_flush) never fires either. Net effect: on a transient network error, the in-flight batch of log records is silently dropped, with no retry from either the plugin or Fluentd core.This PR re-raises after logging in
write(chunk), and also re-raises insend_retriesonce its own retry budget is exhausted (previously it silently returned in that case too), so Fluentd's core retry/backoff can engage as designed.Motivation
Reported by a customer (NRI) across two Zendesk tickets (
Net::ReadTimeoutandend of file reachedvariants of the same swallowed-exception path). Investigation traced the swallow-all rescue back to a 2020 fix that was addressing a different, narrower issue (persistent HTTP connection setup crashes) — the retry-suppression side effect looks unintentional and was never revisited. Filed as AGENT-16861.Additional Notes
test/plugin/test_out_datadog.rb).send_retriesalready retries internally before this change, re-raising means a chunk can now also be retried by Fluentd core after the plugin's own retries are exhausted — i.e. two layers of retry instead of one. This trades a bit of duplicate retry effort for no longer silently dropping data, which seems like the right call, but flagging it explicitly in case reviewers want to simplify to a single retry layer instead.(Supersedes #90, closed by an accidental branch rename.)