feat(dogstatsd): add dogstatsd top - #2185
Conversation
Coordinate retained-context snapshots through the aggregate owner task. Clone only shared context handles and small metric metadata so serialization can happen without holding up ingestion.
Represent absent snapshot units as None so downstream serializers can distinguish missing metadata without inspecting an empty string.
Account for the peak owner-side snapshot allocation alongside retained aggregation state. Exercise snapshot delivery, canceled request handling, and clean shutdown through the production aggregate run loop.
Read plain or zstd Agent NDJSON by magic bytes and render retained-context summaries with Agent-compatible grouping, ordering, cardinality, and limit behavior.
Buffer decompressed zstd output before serde decoding while retaining buffered compressed input, preserving bounded-memory streaming and avoiding small-read throughput penalties. Document the temporary module-level dead-code allowance until subsequent DogStatsD CLI and API tasks wire it.
Branch on zstd magic while readers are concrete and monomorphize serde decoding for plain and compressed streams. This removes heap allocation and per-byte vtable dispatch when processing huge context artifacts.
Stream Agent-compatible records through zstd into a mode-0600 temporary file, finalize and sync it, then atomically replace the fixed run-path artifact without exposing partial output.
Normalize newly created Unix staging files to exact owner-only permissions before writing, generate UUID v4 names from fallible entropy, and report explicit cleanup failures alongside the primary publication error while retaining best-effort fallback cleanup.
Protect dump creation with the Agent bearer token, coordinate bounded owner snapshots, serialize fixed-path requests, and return only the completed artifact path.
Add a test-only responder that accepts a snapshot request and drops its response sender. Verify the resulting stopped-owner error maps to the same safe 503 response as other unavailable snapshots.
Reuse the first owner snapshot as the combined buffer to bound peak coordination memory. Make cancellation fixtures deterministic and ensure blocking test publishers are always released during unwinding.
Re-export the accepted snapshot response wrapper with the test utilities so downstream fixtures can name the type returned by AggregateContextSnapshotResponder::receive.
Allow the HTTP client to consume the Agent IPC TLS configuration, pin the server certificate, and attach the Agent bearer token to DogStatsD context dump requests.
Clear preconfigured ALPN values from complete client TLS configurations before constructing the hyper-rustls connector. This prevents its non-empty ALPN panic and keeps HTTP protocol advertisement owned by HttpClientBuilder.
Support authenticated online dump-and-analyze, offline Agent artifact analysis, and dump-only workflows with compatible flags, ordering, limits, and output text.
Sanitize JSON decode failures to retain only safe path, record, category, and location diagnostics so malformed artifact values cannot enter CLI error chains. Consume and validate DogStatsD top options before constructing the authenticated client so conflicting tag flags fail locally.
Connect the post-processing aggregate owner to the authenticated privileged dump route using the configured run path and Agent IPC credentials.
Wrap the control-surface route request write and response read in the remaining five-second test deadline. A stalled peer now fails with a clear panic instead of hanging indefinitely.
Verify snapshots follow Saluki aggregation retention, including passthrough exclusion, mixed values, ordinary flush removal, idle counters, and post-processing identity.
Measure aggregate-owner snapshot cost and streaming compressed publication through the configured one-million-context ceiling with correctness checks in every benchmark.
Keep Criterion identities tied only to context count, move publication assertions outside timed closures, and route benchmarks through a feature-gated facade over production artifact code so unit tests remain enabled under every feature set.
Document online and offline workflows, retained-state semantics, Agent interoperability, shared-filesystem limits, sensitive-data handling, and cleanup.
Remove the optional retained-context link from generated configuration documentation. The operator workflow remains discoverable from the handwritten Agent Data Plane index.
Check whether the one-shot requester is still present before pausing the aggregate owner to clone retained contexts and allocate the snapshot vector. Keep response delivery race-safe by continuing to ignore a send failure if cancellation occurs after the check.
Create Windows staging artifacts with a protected DACL before the file becomes visible. Grant full control only through Owner Rights, LocalSystem, and Builtin Administrators while preserving CREATE_NEW atomic publication semantics.
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,111 @@ | |||
| # Inspect retained DogStatsD contexts | |||
There was a problem hiding this comment.
Adding this documentation file seems fine to me, but highlighting for reviewers (not you, LLM, human reviewers)
There was a problem hiding this comment.
[GPT-5.6 Sol (OpenAI)] Acknowledged. I left the operator document in place and am treating this thread as a human-review callout rather than making a code change or resolving it on reviewers' behalf.
Use tempfile for secure randomized staging instead of custom entropy and filename generation, and separate API and artifact test harnesses from production modules for focused review.
Use the Agent command API path for context dumps, centralize Data Plane client construction, and document why the authenticated client requires the Agent IPC TLS configuration.
Document the Criterion commands and PR baseline measurements alongside both retained-context benchmarks, with an explicit note that the values are orientation rather than thresholds.
Exercise the authenticated online and offline CLI flow on Windows and verify the published canonical artifact retains a protected Owner Rights, LocalSystem, and Administrators DACL.
Strip the Win32 verbatim namespace produced by canonicalization before passing host paths to Docker. Docker rejects those otherwise valid paths as missing, preventing Windows integration cases from mounting fixture files.
This reverts commit 60a1f81.
Follow the existing Windows Panoramic pattern and pass PowerShell through target_exec. The Windows Docker daemon cannot access individual files from the CI runner checkout as bind-mount sources.
Use NamedTempFile for same-directory creation, synchronization, cleanup, and atomic persistence. Windows artifacts now inherit the configured run_path ACL like the Datadog Agent, removing ADP-specific DACL and filesystem FFI machinery.
Record the post-refactor NamedTempFile publication measurements so the benchmark comment describes the implementation under review.
dogstatsd top
There was a problem hiding this comment.
More details
The retained-context snapshot, authenticated API, artifact publication, and offline report paths were reviewed for cancellation, malformed input, limit, and lifecycle hazards; no reproducible diff-only regression was identified. No additional tests recommended: the PR already includes targeted coverage for the realistic edge cases found during review.
🤖 Datadog Autotest · Commit 6454800 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Import the generic error macro directly in DogStatsD CLI modules and use rest-pattern destructuring for report records. These are mechanical review fixes with no behavior change.
Expose only the correctly spelled --num-tags option for DogStatsD top and remove the compatibility-only conflict validation and tests.
The one-time performance measurements established acceptable snapshot and publication costs, but the non-gating Criterion targets required public benchmark-only facades and fixtures. Remove that scaffolding while retaining the recorded PR evidence.
Delete a trivial handler-registration test and a timeout test that exercised only test-local networking helpers rather than production behavior.
Had the clanker try to self-remediate based on this discussion around some useless tests #2185 (comment) ## Summary - Require unit tests to exercise a production behavior or branch. - Prohibit tests whose sole subject is a test-only helper. - Add a counterfactual self-review check based on mutating or removing the claimed production behavior. ## Motivation A review of test-only networking helpers in #2185 showed that realistic setup and strong-looking assertions can still provide no production coverage. These rules make production reachability and mutation sensitivity explicit review requirements. ## Test plan - `make check-docs` - Repository pre-commit checks
Had the clanker try to self-remediate based on this discussion around some useless tests #2185 (comment) ## Summary - Require unit tests to exercise a production behavior or branch. - Prohibit tests whose sole subject is a test-only helper. - Add a counterfactual self-review check based on mutating or removing the claimed production behavior. ## Motivation A review of test-only networking helpers in #2185 showed that realistic setup and strong-looking assertions can still provide no production coverage. These rules make production reachability and mutation sensitivity explicit review requirements. ## Test plan - `make check-docs` - Repository pre-commit checks 151ad17
Remove the context-dump route bearer token, token-file reads, Authorization header, and auth-specific tests. Keep IPC certificate pinning and client identity so a separate privileged-API mTLS change can enforce the shared access-control boundary.
Route DogStatsD top and dump-contexts through the same DataPlaneAPIClient path as existing privileged endpoints. Remove all mTLS preparation, certificate pinning support, TLS tests, and dependencies so the separate privileged-API mTLS PR owns that change surface.
Summary
This PR adds the retained-context diagnostic exposed by the Datadog Agent as
dogstatsd topto theagent-data-planebinary.It adds:
agent-data-plane dogstatsd topfor an online privileged-API snapshot followed by local analysis.agent-data-plane dogstatsd dump-contextsfor dump-only workflows.agent-data-plane dogstatsd top --path <artifact>for offline and off-host analysis.The diagnostic reports contexts currently retained by DogStatsD aggregation. It does not report packet rate, sample frequency, or contexts observed during a collection window.
Tracking
dogstatsd topcommand to show top DogStatsD contextsReviewer TL;DR
The intended review is primarily about the approach and contracts rather than every test helper or injected failure seam.
The central design is:
dsd_aggremains the sole owner ofAggregationState.select!loop.The most useful review order is:
docs/agent-data-plane/dogstatsd-top.mdbin/agent-data-plane/src/cli/dogstatsd/top.rsbin/agent-data-plane/src/dogstatsd_contexts/report.rslib/saluki-components/src/transforms/aggregate/mod.rsbin/agent-data-plane/src/dogstatsd_contexts/api.rsbin/agent-data-plane/src/dogstatsd_contexts/artifact.rsbin/agent-data-plane/src/cli/run.rsandinternal/control_surfaces.rsdogstatsd-topanddogstatsd-top-windowsSuggested review questions:
AggregationStatethe right authoritative retained-state boundary?Motivation
High-cardinality retained contexts can account for significant Agent memory. Traffic-window statistics answer "what arrived recently?" but not "what is still retained for aggregation?"
Saluki already had
dogstatsd stats, including cardinality analysis, but it is not a valid source for this command:dsd_statsbranches directly from rawdsd_in.metrics.This implementation therefore snapshots the context map owned by the actual post-mapper, post-prefix-filter, post-tag-filterlist
dsd_aggtransform.User interface
Command tree
Online dump and analysis
The command:
Wrote <path>;Printing/flushing the path before parsing is intentional: if later local analysis fails, the operator still knows where the completed diagnostic artifact is.
Dump only
This is useful when an operator wants to copy the artifact away from a pressured host or analyze it later.
Offline analysis
Offline mode does not construct an API client or contact the running ADP process.
Input validation
Compared with the Agent command, the ADP CLI intentionally rejects rather than copying unsafe edge behavior:
--mum-tagstypo is intentionally unsupported; use--num-tags.Zero remains valid and follows the Agent's existing "do not summarize only one remainder" rule.
Privileged API contract
Request
The route is registered only on ADP's TLS-enabled privileged API and only when the DogStatsD pipeline is enabled.
The CLI uses the existing
DataPlaneAPIClient::from_configpath shared by other ADP privileged endpoints. This PR adds no endpoint-specific authentication, certificate handling, TLS configuration, or HTTP-client infrastructure; the route relies entirely on the existing privileged API access-control boundary.Successful response
The response contains only the path. Context records are never returned or buffered in HTTP.
This preserves the Agent's shared-filesystem operational model. Remote callers that cannot see ADP's filesystem must securely copy the completed artifact and use offline mode; this PR does not add a download endpoint.
Error responses
503504500Public bodies are fixed and redacted. Metric names, hosts, tags, credentials, raw serde errors, and filesystem internals are not reflected to API callers.
Snapshot and ownership model
AggregationStateremains exclusively owned by the aggregate task. The new public boundary is a cloneableAggregateContextSnapshotHandlecontaining the request sender; the owner-side receiver is taken exactly once when the transform is built.A request contains a one-shot response channel. The aggregate loop:
AggregateContextSnapshotEntryper retained context;Each entry contains:
Arc-backedContextclone, preserving name, host, client tags, and origin tags without deep-copying their contents;MetaStringunit.Serialization does not occur on the owner task.
Consistency
The current DogStatsD topology supplies one aggregate owner, so the snapshot is internally point-in-time for that owner.
The API coordinator accepts multiple handles and requests them concurrently. If future topology changes add multiple aggregation owners, each owner snapshot remains internally consistent while the combined result is a documented rolling snapshot rather than a globally atomic barrier.
Memory and pause behavior
Snapshot memory is
O(contexts)and is included in firm aggregate memory accounting:At one million contexts, the structural entry vector is 40 MB in the current build. The API reuses the first owner vector when combining results, so the current one-owner case does not allocate a second million-entry buffer.
The public handle documents that ownership transfers to the caller, which must account for the vector while retaining it.
What constitutes a reported context
The source is the set currently retained in Saluki's
AggregationState:This intentionally follows Saluki's authoritative aggregation lifecycle. It does not force the Agent context resolver's exact internal TTL model onto a different architecture.
Artifact contract
Location
A successful dump replaces the prior canonical artifact. ADP does not automatically remove the latest artifact.
Record schema
The artifact is an unversioned stream of top-level JSON objects, one per context, compatible with the Agent's
ContextDebugRepr:{"Name":"requests.count","Host":"node-a","Type":"Counter","TaggerTags":["pod_name:web-a"],"MetricTags":["env:prod","instance:a"],"NoIndex":false,"Source":1}NameHost""TypeHistorateTaggerTagsMetricTagsNoIndexfalse; Saluki has no corresponding retained flagSource1, AgentMetricSourceDogstatsdThe writer borrows names and tags from the snapshot while serializing. It does not build a second raw JSON representation in memory.
Compression and reading
ADP writes zstd. The reader detects zstd by magic bytes rather than filename suffix and also accepts plain Agent NDJSON. Decoding is buffered and streaming.
A compressed artifact renamed without
.zstdremains readable by ADP. ADP-produced artifacts retain the.zstdsuffix required by the stock Agent reader.Publication guarantees
ADP improves the Agent's direct create/truncate behavior while delegating cross-platform staging and replacement to
tempfile:NamedTempFileexclusively.0600; on Windows, inherit the configuredrun_pathACL like the Agent.NamedTempFile::persist.tempfileremove an unpersisted staging file on drop after failure.The previous canonical artifact remains intact on every failure before persistence. Cleanup is best-effort through
tempfile; ADP no longer carries custom Windows file/security FFI, replacement code, or compound cleanup-error machinery.Report contract
For every decoded record, the report:
Nameonly;MetricTags;NoIndex, and source while rendering.Consequences:
Ordering is deterministic:
Both metric and tag limits preserve the Agent's
limit + 1rule: exactly one item that would otherwise be hidden is shown rather than summarized as one remainder. Hidden metric rows sum context counts; hidden tag text sums cardinalities.Comparison with Datadog Agent
dogstatsd top,dogstatsd dump-contextsagent-data-plane dogstatsdtop --path--mum-tags/-t--num-tags/-t; typo intentionally unsupported/agent/dogstatsd-contexts-dumpon Agent command APIdsd_aggAggregationState<run_path>/dogstatsd_contexts.json.zstdNamedTempFilestaging, sync, atomic persistence, drop cleanup0600run_pathfile securityrun_pathACL model.zstdsuffixlimit + 1The route, method, response shape, artifact path, schema, and report behavior match the Agent contract. ADP serves the route on its existing privileged API listener rather than the Agent command-port server.
Bidirectional interoperability evidence
This was tested with the installed Datadog Agent 7.80.3 and with the containerized Agent used by Panoramic.
Agent writes, ADP reads
An isolated real Agent received DogStatsD traffic and ran:
agent dogstatsd dump-contextsBoth commands then analyzed that exact Agent-produced artifact:
The reports matched exactly.
The actual Agent 7.80.3 compressed artifact and its exact decompressed NDJSON are checked in under
test/integration/cases/dogstatsd-top/, so this direction remains automated.ADP writes, Agent reads
An isolated real ADP received DogStatsD traffic and ran:
agent-data-plane dogstatsd dump-contextsBoth commands then analyzed the ADP-produced artifact:
The reports matched exactly.
The Linux Panoramic case repeats both directions with the real binaries. A separate Windows Panoramic case exercises the privileged ADP online/offline workflow while allowing the artifact to inherit the configured
run_pathACL like the Agent.Code map
lib/saluki-components/src/transforms/aggregate/mod.rslib/saluki-components/src/transforms/mod.rsbin/agent-data-plane/src/dogstatsd_contexts/api.rsbin/agent-data-plane/src/dogstatsd_contexts/artifact.rsNamedTempFilestaging, sync, and atomic persistencebin/agent-data-plane/src/dogstatsd_contexts/report.rsbin/agent-data-plane/src/cli/dogstatsd/top.rsbin/agent-data-plane/src/cli/utils.rsbin/agent-data-plane/src/cli/run.rsdsd_agghandle and constructs the handler from the configured run pathbin/agent-data-plane/src/internal/control_surfaces.rstest/integration/cases/dogstatsd-top/,test/integration/cases/dogstatsd-top-windows/docs/agent-data-plane/dogstatsd-top.mdVerification strategy
The test suite is intentionally layered so reviewers can evaluate contracts without tracing every failure-injection helper.
1. Aggregate ownership and lifecycle
Coverage includes:
2. Real processing-chain identity
An in-process topology sends metrics through the actual:
The snapshot must contain only the mapped, prefixed, filtered identity; raw names, removed tags, and blocked metrics must be absent.
3. API contract and concurrency
Coverage includes:
POSTroute andGETrejection;500responses.4. Artifact schema, corruption, and publication
Coverage includes:
tempfiledrop cleanup;0600;5. Privileged API client and CLI
Coverage includes:
6. Black-box Linux and Windows integration
The committed Linux Panoramic case:
agent-data-plane dogstatsd topcommand;application/jsonpath-only response;dump-contextsand verifies exact output and mode0600;Local result against a freshly rebuilt image:
The Windows Panoramic case uses the real Windows ADP binary and PowerShell to:
top,dump-contexts, and copied-artifact offlinetopthrough the existing privileged API client path;Windows publication intentionally applies no artifact-specific DACL; the completed artifact inherits the configured
run_pathACL, matching the Agent'sos.Createbehavior.Native Windows privileged-boundary/inherited-ACL/
NamedTempFile::persistresult:7. Repository/platform verification
cargo nextest run -p saluki-components transforms::aggregate::tests— 26 passed.cargo nextest run -p agent-data-plane— 208 passed.cargo nextest run -p saluki-io— 113 passed.cargo check --workspace && cargo check --workspace --tests— passed.make fmt— passed with no tracked changes.make check-all— passed, including the FIPS feature matrix.make test-all— passed:make build-windows-cross— passed forx86_64-pc-windows-msvc.cargo xwin check --target x86_64-pc-windows-msvc --package agent-data-plane --tests— passed.core.autocrlf=truecheckout preserves both NDJSON fixtures as LF and byte-matches their compressed artifacts.Performance evidence
One-time local Criterion sample-size-10 measurements taken while selecting the implementation (the benchmark-only scaffolding was removed after review):
The owner pause measurement covers only snapshot construction. Artifact serialization and publication occur after the owner resumes.
These measurements are evidence, not regression thresholds. The benchmark workload shares realistic tag sets; it does not model worst-case unique tag-value memory during offline report aggregation.
Failure-handling matrix
503504500, no CWD fallback500; unpersistedNamedTempFileis removed on drop and the prior canonical remains500;tempfileretains cleanup ownership and the prior canonical remainsNon-goals and explicit trade-offs
dogstatsd stats.O(contexts)and briefly pauses each owner; serialization does not.Residual validation and review notes
NamedTempFile::persistpublication (dogstatsd-top-windows: 7 assertions; full suite: 31 passed).dogstatsd-topLinux integration case passed locally (PASS dogstatsd-top, 7 assertions). The complete integration and correctness suites were not run locally.run_pathACL security is an installer/operator responsibility, matching the Agent's inherited-permission model.