feat(dogstatsd): add Windows named pipe intake#1956
feat(dogstatsd): add Windows named pipe intake#1956gh-worker-dd-mergequeue-cf854d[bot] merged 27 commits into
Conversation
Binary Size Analysis (Agent Data Plane)Baseline: 94ecf81 · Comparison: 3d25c15 · 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 ( |
This comment has been minimized.
This comment has been minimized.
…atsd-named-pipe # Conflicts: # lib/saluki-io/src/net/addr.rs
Replace the DogStatsD-specific Panoramic action with a reusable container_exec action. The named-pipe integration case now mounts a small PowerShell script and executes it inside the Windows target container, keeping DogStatsD-specific behavior in the test case instead of the harness API. This still verifies the end-to-end Windows named-pipe path: ADP starts the listener, PowerShell connects to the named pipe, writes a raw newline-delimited DogStatsD payload, and the test asserts ADP decodes and logs the invalid frame.
Rename the integration-test helper from container_exec to target_exec and make it run in either target environment. Container-backed runtimes still use Docker exec, while host-process runtimes execute the command directly with tokio::process::Command. The Windows named-pipe integration case keeps using a mounted PowerShell script, but no longer requires a DogStatsD-specific or container-only Panoramic action.
| uds: bool, | ||
| // The core Agent exposes `named_pipe` as a separate `dogstatsd_eol_required` target. | ||
| // Named pipes use raw newline framing like UDP datagrams, but they have their own switch | ||
| // so Windows clients can require newline-terminated pipe frames without affecting UDS. |
target_exec is a generic command action, not a readiness poller. Run the command once with timeout/cancellation semantics so script errors fail immediately with their stdout/stderr instead of being retried until the action timeout expires. Keep host-process support by executing commands with tokio::process::Command and kill_on_drop enabled, and keep container support by wrapping a single Docker exec call in the same timeout/cancellation behavior.
Remove the explanatory comment on the named_pipe eol_required field per PR review. The surrounding code and tests already document the behavior sufficiently.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 496d200708
ℹ️ 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".
webern
left a comment
There was a problem hiding this comment.
Config changes look good. You did the right thing with the overlay and the generated diff flows from there and looks like what we expect.
|
I'll tell Claude he did a great job |
Named-pipe DogStatsD traffic uses raw newline framing to match the core Agent. Unlike UDP datagrams, named pipes are connection-oriented, so a partial frame can fill the fixed I/O buffer before a newline arrives. Detect that full-buffer/no-newline state for named-pipe listeners, count it as a framing error, clear the buffer, and continue reading. This matches the core Agent behavior of dropping oversized named-pipe messages instead of decoding truncated data or disconnecting the client.
Avoid mounting the PowerShell helper into the Windows container. Docker rejected the POSIX-style container destination path in CI, so keep the named-pipe client script inline in the target_exec command. Also relax the target_exec nonzero-exit unit test so it asserts preserved stderr rather than Unix-specific exit status formatting. Windows reports process status differently, but the behavior under test is immediate failure with diagnostic output.
…atsd-named-pipe # Conflicts: # docs/agent-data-plane/configuration/dogstatsd.md # lib/datadog-agent/config-testing/src/config_registry/dogstatsd.rs # lib/saluki-components/src/sources/dogstatsd/mod.rs
Use memchr::memchr in the named-pipe oversized-frame guard instead of slice::contains. This matches the surrounding framing code's optimized delimiter search pattern.
Record the Cargo.lock update from adding memchr as a direct saluki-components dependency. The named-pipe oversized-frame guard uses memchr for newline detection.
Extend the Windows named-pipe integration case with two behaviors covered by the core Agent tests: multiple newline-delimited messages on one connection, and an oversized unterminated write followed by a valid message on a new connection. The test lowers dogstatsd_buffer_size to make the oversized path deterministic, then asserts ADP still decodes messages sent after the oversized frame is dropped.
Do not lower dogstatsd_buffer_size for the named-pipe integration test. The core Agent also writes DogStatsD telemetry to the configured named pipe, and a tiny pipe/read buffer fragments those background metrics into noisy parse failures. Keep the oversized-frame coverage deterministic by writing a payload larger than the default DogStatsD buffer instead, then assert a subsequent valid frame is decoded on a new connection.
…atsd-named-pipe # Conflicts: # lib/datadog-agent/config/src/classifier/classifier_data.rs
Extend the Windows named-pipe integration test with paired payloads that differ only by newline termination. With DD_DOGSTATSD_EOL_REQUIRED=named_pipe, the newline-terminated invalid payload should reach DogStatsD parsing and appear in logs, while the short unterminated payload should be rejected by the framer and stay out of parse logs.
## Summary - Add a reusable Windows named-pipe listen address/transport in `saluki-io`. - Wire DogStatsD config to enable `dogstatsd_pipe_name` and `dogstatsd_windows_pipe_security_descriptor`. - Use core-Agent-compatible raw newline framing for Windows named-pipe DogStatsD payloads. - Drop oversized named-pipe frames that fill the I/O buffer before a newline arrives, matching the core Agent's oversized-message behavior. - Match core Agent pipe creation behavior by applying `dogstatsd_buffer_size` as the Windows pipe input buffer and using an output buffer size of 0. - Accept either bare pipe names or full `\\.\pipe\...` values when constructing named-pipe listen addresses. - Use `npipe://` for URL-style display/parsing of Windows named-pipe addresses, matching existing Agent conventions for named-pipe socket paths. - Update DogStatsD config inventory/docs from planned unsupported to supported and cover named-pipe newline/stream behavior. - Add a Windows integration case that executes a PowerShell named-pipe client via `target_exec`, writes to the named pipe, and asserts ADP decodes the frame. ## Test plan - `cargo test -p panoramic` - `cargo run --release --bin panoramic -- list -d test/integration/cases --runtime windows --json | rg 'dogstatsd-named-pipe-windows'` - `cargo check --workspace` - `cargo check --workspace --tests` - `cargo test -p saluki-io` - `cargo test -p saluki-components dogstatsd` - `cargo xwin check -p saluki-io --target x86_64-pc-windows-msvc` - `cargo xwin check -p saluki-components --target x86_64-pc-windows-msvc` - `make fmt` CI investigation note: earlier Windows integration runs proved the action connected/wrote to the pipe, then exposed two bugs: ADP initially treated named-pipe bytes as length-delimited stream frames, and a mounted helper script used a container path Docker rejected on Windows. The test now runs an inline PowerShell client through `target_exec`, and ADP uses raw newline framing for named pipes. Co-authored-by: travis.thieman <travis.thieman@datadoghq.com> 54d3177
Summary
saluki-io.dogstatsd_pipe_nameanddogstatsd_windows_pipe_security_descriptor.dogstatsd_buffer_sizeas the Windows pipe input buffer and using an output buffer size of 0.\\.\pipe\...values when constructing named-pipe listen addresses.npipe://for URL-style display/parsing of Windows named-pipe addresses, matching existing Agent conventions for named-pipe socket paths.target_exec, writes to the named pipe, and asserts ADP decodes the frame.Test plan
cargo test -p panoramiccargo run --release --bin panoramic -- list -d test/integration/cases --runtime windows --json | rg 'dogstatsd-named-pipe-windows'cargo check --workspacecargo check --workspace --testscargo test -p saluki-iocargo test -p saluki-components dogstatsdcargo xwin check -p saluki-io --target x86_64-pc-windows-msvccargo xwin check -p saluki-components --target x86_64-pc-windows-msvcmake fmtCI investigation note: earlier Windows integration runs proved the action connected/wrote to the pipe, then exposed two bugs: ADP initially treated named-pipe bytes as length-delimited stream frames, and a mounted helper script used a container path Docker rejected on Windows. The test now runs an inline PowerShell client through
target_exec, and ADP uses raw newline framing for named pipes.