chore(agent-data-plane): share shutdown signal wait logic - #2323
Conversation
SIGTERM is the signal used by systemd, container runtimes, and Kubernetes to request shutdown, but ADP only listened for SIGINT, so it terminated immediately without draining the topology or honoring the shutdown timeout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dows On Windows, ADP is managed as a subprocess by dd-procmgr, which requests a graceful stop by sending CTRL_BREAK_EVENT (not CTRL_C_EVENT) via GenerateConsoleCtrlEvent. The previous fallback only listened for tokio::signal::ctrl_c(), so this request went unnoticed and ADP would sit until dd-procmgr's stop timeout expired and force-killed it via its Job Object, reproducing the same "no graceful shutdown" behavior as SIGTERM on Unix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ulary The new SIGTERM/CTRL_BREAK doc comment on wait_for_shutdown_signal uses these terms, which Vale's spelling check doesn't recognize, failing check-docs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signal registration via `tokio::signal::unix::signal`/`ctrl_c` happens synchronously when called, but the previous code only called it inside the `async fn` polled at the point we're ready to wait for shutdown -- so a signal delivered anywhere during startup (e.g. while waiting on the Datadog Agent's initial configuration) fell through to the OS default disposition and killed the process immediately, skipping cleanup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Binary Size Analysis (Agent Data Plane)Baseline: 6825bb3 · Comparison: d601d0f · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
…tdown-signal-early # Conflicts: # bin/agent-data-plane/src/cli/run.rs
…`run` and `dogstatsd replay` Per discussion on #2322, a signal received before the supervisor starts doesn't need graceful handling, so this drops the earlier eager pre-registration approach and instead extracts the existing lazy signal-wait logic into a shared helper reused by both the `run` and `dogstatsd replay` commands, removing the duplication. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
More details
The shared helper preserves the run command’s platform-specific signal branches and replay’s signal-to-cancellation transition. Its replay import and caller remain Linux-gated, so the extraction does not introduce a cross-platform compilation or shutdown-flow regression.
🤖 Datadog Autotest · Commit 52ee2de · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
…uki-app Moves the shared shutdown-signal-wait logic into `saluki_app::util` so it's reusable by any binary built on the saluki crates, not just agent-data-plane. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Human Summary Refactors the shutdown wait handler into a shared module that can be used where ever we need to wait to ensure we consistently rely on the same signals, cross-platform. ## AI Summary #2322 added SIGTERM/SIGINT handling for graceful shutdown, but the same signal-waiting logic was duplicated between the `run` command and `dogstatsd replay`'s cancellation setup. An earlier version of this PR tried to close a related gap -- a signal delivered before the supervisor starts falling through to the OS's default disposition -- by registering signal handlers eagerly at process start. Per [discussion on #2322](#2322 (comment)), that gap doesn't need graceful handling: an abrupt exit before the supervisor is running is acceptable. This PR instead does a pure refactor, extracting the existing lazy signal-wait logic into a shared `saluki_app::util::wait_for_shutdown_signal` helper used by both `run` and `dogstatsd replay`, removing the duplication and making it reusable by any binary built on the saluki crates, not just agent-data-plane. ## Test plan - [x] `cargo check --workspace`, `cargo clippy -p agent-data-plane -p saluki-app --no-deps` pass. - No new automated tests added; this is a non-behavioral refactor of existing signal-handling code. Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com> e760adf
Human Summary
Refactors the shutdown wait handler into a shared module that can be used where ever we need to wait to ensure we consistently rely on the same signals, cross-platform.
AI Summary
#2322 added SIGTERM/SIGINT handling for graceful shutdown, but the same signal-waiting logic was duplicated between the
runcommand anddogstatsd replay's cancellation setup. An earlier version of this PR tried to close a related gap -- a signal delivered before the supervisor starts falling through to the OS's default disposition -- by registering signal handlers eagerly at process start. Per discussion on #2322, that gap doesn't need graceful handling: an abrupt exit before the supervisor is running is acceptable. This PR instead does a pure refactor, extracting the existing lazy signal-wait logic into a sharedsaluki_app::util::wait_for_shutdown_signalhelper used by bothrunanddogstatsd replay, removing the duplication and making it reusable by any binary built on the saluki crates, not just agent-data-plane.Test plan
cargo check --workspace,cargo clippy -p agent-data-plane -p saluki-app --no-depspass.