Skip to content

fix(logs): store failed log events on intake errors#1131

Merged
ndakkoune merged 1 commit into
DataDog:masterfrom
patbos:fix/store-failed-log-events
May 22, 2026
Merged

fix(logs): store failed log events on intake errors#1131
ndakkoune merged 1 commit into
DataDog:masterfrom
patbos:fix/store-failed-log-events

Conversation

@patbos

@patbos patbos commented May 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the DD_STORE_FAILED_EVENTS feature for log events so failed log batches are actually stored in S3 and retried on subsequent invocations.

In DatadogHTTPClient.send(), the future returned by FuturesSession.post() was not awaited and the response status was never checked. Failures (network errors, HTTP 4xx/5xx) were either resolved later in _close() and silently swallowed, or never surfaced at all. As a result, forwarder._forward_logs() never saw an exception, failed_logs stayed empty, and nothing was written to the S3 retry prefix.

This PR:

  • Resolves the future inside send() so failures are attributed to the current batch.
  • Calls response.raise_for_status() so HTTP 4xx/5xx propagate.
  • Removes the unused _futures list and the swallow-all loop in _close().
  • Adds unit tests covering network failure, HTTP error response, success, and end-to-end storage of a failed batch via Forwarder._forward_logs.

Motivation

Fixes #1129. Users relying on DD_STORE_FAILED_EVENTS=true to recover from transient Datadog intake errors were silently losing logs.

Testing Guidelines

  • Unit tests: PYTHONPATH=. python3 aws/logs_monitoring/tests/test_datadog_http_client.py (4 tests pass).
  • Manual end-to-end on a deployed forwarder Lambda:
    1. Set DD_STORE_FAILED_EVENTS=true and point DD_URL at an unresolvable host.
    2. Invoke with a synthetic CloudWatch Logs payload — failed batches appear under s3://<bucket>/failed_events/<function-sha1>/logs/.
    3. Restore DD_URL, invoke with {"retry": true} — stored batches are resent and the S3 objects are deleted.

Additional Notes

Behavior changes worth noting for reviewers:

  • send() is now synchronous per batch. Previously batches were submitted concurrently via FuturesSession, but errors were swallowed in _close(). Concurrency can be reintroduced in a follow-up if needed; the immediate priority is correctness.
  • HTTP 5xx and network errors now flow through the DD_STORE_FAILED_EVENTS S3 retry path rather than the in-process retry loop in DatadogClient, which was effectively unreachable for HTTP errors before this fix.

Types of changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)
  • This PR passes the unit tests
  • This PR passes the installation tests (ask a Datadog member to run the tests)

@patbos
patbos requested a review from a team as a code owner May 22, 2026 05:26
@ndakkoune ndakkoune self-assigned this May 22, 2026
@ndakkoune
ndakkoune merged commit 439af17 into DataDog:master May 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DD_STORE_FAILED_EVENTS does not store events on HTTP 4xx/5xx responses from log intake

2 participants