Skip to content

fix(cloudwatchlogs): send the events waiting in a queue when it stops - #2403

Open
AXEG0 wants to merge 1 commit into
aws:mainfrom
AXEG0:fix/cloudwatchlogs-queue-drain-on-stop
Open

AXEG0 wants to merge 1 commit into
aws:mainfrom
AXEG0:fix/cloudwatchlogs-queue-drain-on-stop

Conversation

@AXEG0

@AXEG0 AXEG0 commented Nov 11, 2026

Copy link
Copy Markdown

Description of the issue

Part of #2398. On its own this does not fix the case that issue reports; see the last paragraph.

When a log destination stops, queue.Stop closes stopCh, and merge returned as soon as it saw it. If events were still buffered in eventsCh, or in the non-blocking channel used for EMF, select picked at random between them and stopCh, so merge usually returned after forwarding a few of them. The rest were never sent, and their done callbacks never ran. For a tailed file, that means its offset did not advance, so the lines were read again after a restart, and lost if the instance was gone.

Events wait there whenever the queue goroutine is busy in a PutLogEvents call, which it makes itself unless concurrency is set above 1. A destination stops when the agent shuts down in logs-only mode (CloudWatchLogs.Close), and when its last source stops (NotifySourceStopped).

In #2398 the config has a metrics section, so the agent runs the collector, and nothing calls CloudWatchLogs.Close at exit. The tailer also keeps the newest line in its multi-line buffer and drops it on stop. Fixing those changes the shutdown order, so I'd like to agree on the approach in the issue first. This PR fixes a part that stands on its own and that such a fix needs: a shutdown that stops each destination after its sources publish their last lines would otherwise lose those lines here.

Description of changes

On stopCh, merge now forwards the events still buffered in eventsCh, then those in the non-blocking channel, before it returns. The queue goroutine batches and sends them as before, and then makes its final send.

  • The drain never waits for an event to be added. cwDest adds events and stops the queue under its lock, so nothing is added after Stop, and an empty channel stays empty.
  • A destination moves from the blocking to the non-blocking channel only once, so draining the blocking one first keeps the order.
  • Pusher.Stop stops the sender right after the queue, so each of these batches gets one attempt, as the final send already does. The drain adds to a stop at most what the channels hold: 100 events, or up to 20,000 EMF events.

#2023 and #1975 also change queue.go, but not merge. With the stop-aware AddEvent in #2023, an event added after the drain is dropped as it is today, and the drain still never blocks.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

TestStopQueueWouldSendEventsWaitingInChannel, beside TestStopQueueWouldDoFinalSend, holds the queue goroutine in its first PutLogEvents call, adds 50 events, stops the queue and then releases the call. It checks that all 51 events are sent, in order, and that each one's done callback ran. It runs once with AddEvent and once with AddEventNonBlocking, and has no sleeps.

go test -count=20 -run TestStopQueueWouldSendEventsWaitingInChannel ./plugins/outputs/cloudwatchlogs/internal/pusher/
  • Before (main at 7dbea90): 40 of 40 subtest runs fail, with 2 to 7 of the 51 events sent.
  • After: with -count=200, 400 of 400 subtest runs pass.

The queue reproduction in #2398 (TestReproStopDropsEventsWaitingInChannel) now sends all 51 events in 20 of 20 runs. Before, it dropped 46.

CGO_ENABLED=0 go test -count=1 ./plugins/outputs/cloudwatchlogs/... ./logs/... ./plugins/inputs/logfile/...

All pass, and the pusher package takes as long as on main (128 s against 123 s). go vet, gofmt, goimports -local, impi and addlicense are clean on the changed files. golangci-lint v2.12.2, the Makefile's version, reports 0 issues on ./plugins/outputs/cloudwatchlogs/.... Go 1.26.11, linux/arm64.

CGO_ENABLED=1 go test -race -parallel 4 (as make test-data-race runs it) passes on ./plugins/outputs/cloudwatchlogs/internal/pusher/ and ./plugins/outputs/cloudwatchlogs/ with no race reported, and so do the new test and the two existing stop tests with -race -count=50.

Not run: the whole make test, and the integration tests.

A possible release note line: * [Logs] Fix log events waiting to be sent being dropped when a log destination stops

Requirements

Before commiting your code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

Integration Tests

To run integration tests against this PR, add the ready for testing label.

🤖 Generated with Claude Code

When a log destination stopped, the queue's merge goroutine returned as
soon as it saw stopCh. select picks at random among ready cases, so the
events still buffered in the queue's channels were mostly left behind:
they were never sent, and their done callbacks never ran.

merge now forwards the events still buffered in both channels before it
returns, so that the queue sends them before it exits.

Part of aws#2398

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant