fix: drain statsd on monitor shutdown signal#88
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70224bc113
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log.Print("Received shutdown signal, draining statsd...") | ||
| DrainStatsd(config, 5*time.Second) |
There was a problem hiding this comment.
Stop metric producers before draining queue
MonitorLoop starts a drain after receiving SIGINT/SIGTERM, but it never stops the dest.Monitor() goroutines that continue calling dest.Check() and enqueueing statsd payloads. Because DrainStatsd exits as soon as the queue is momentarily empty, shutdown can return while producers are still active, so metrics generated during shutdown are still dropped despite the new drain path.
Useful? React with 👍 / 👎.
| case s := <-queue: | ||
| emitStatsd(config, s) |
There was a problem hiding this comment.
Bound each drain send by the shutdown timeout
DrainStatsd is documented to flush for up to timeout, but inside the loop it performs a blocking send via emitStatsd; that path uses net.Dial without any deadline. If the statsd endpoint is slow or unreachable, a single drain iteration can block well past 5 seconds, preventing timely process termination after the shutdown signal.
Useful? React with 👍 / 👎.
Part of dolph#17. Monitor waits for SIGINT/SIGTERM, then flushes queued statsd metrics for up to five seconds before exiting instead of dropping them.
70224bc to
0365d01
Compare
Summary
Part of #17.
monitornow handles SIGINT/SIGTERM, logs shutdown, and drains the statsd queue for up to 5s viaDrainStatsdinstead of exiting immediately with buffered metrics dropped.Test plan
go build ./...go test -short ./... -run Statsd