chore(antithesis): dogstatsd generation with byte limit#1974
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Binary Size Analysis (Agent Data Plane)Baseline: 92cb9bd · Comparison: f116dac · 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 ( |
There was a problem hiding this comment.
Pull request overview
This PR updates the Antithesis DogStatsD harness to generate byte-limited, newline-packed payloads (instead of single lines) and refactors payload sampling to use rand’s slice/array selection APIs with an explicit RNG, adding property tests to lock in the new size/invariant behavior.
Changes:
- Introduce
PAYLOAD_BYTE_LIMIT,Payload,write_line, andwrite_payloadto generate\n-terminated lines packed into datagrams without exceeding a byte limit. - Update the load driver to send payload datagrams and report sent line counts / max packed counts across payloads.
- Add proptest-based invariants (and a regression seed file) for payload/line size and newline-count properties.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/antithesis/scenarios/general/src/bin/parallel_driver_send_dogstatsd.rs | Switch scenario to use driver::sample() after driver API refactor. |
| test/antithesis/scenarios/differential/src/bin/parallel_driver_send_dogstatsd_differential.rs | Update imports and batch sampling call site for the refactored driver API. |
| test/antithesis/harness/src/payload/dogstatsd/service_checks.rs | Replace random_choice usage with rand selection APIs and thread RNG through helpers. |
| test/antithesis/harness/src/payload/dogstatsd/metrics.rs | Same as above; also refactors metric extension selection and value-kind selection. |
| test/antithesis/harness/src/payload/dogstatsd/events.rs | Same as above for event option selection and field writing. |
| test/antithesis/harness/src/payload/dogstatsd/common.rs | Make vibe/choice/field helpers RNG-driven and switch selection to rand APIs. |
| test/antithesis/harness/src/payload/dogstatsd.rs | Add byte-limited line/payload writers and proptest invariants for size/newline properties. |
| test/antithesis/harness/src/driver.rs | Change driver from per-line sends to byte-limited payload datagrams, updating stats semantics. |
| test/antithesis/harness/proptest-regressions/payload/dogstatsd.txt | Check in proptest regression seed(s) for reproducibility. |
| test/antithesis/harness/Cargo.toml | Add proptest dev-dependency for the new property tests. |
| Cargo.lock | Record proptest being pulled in (via harness dev-deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4b011df to
67ce101
Compare
51945fe to
30e11dd
Compare
67ce101 to
7024db6
Compare
30e11dd to
7ac69cc
Compare
7024db6 to
9d89983
Compare
7ac69cc to
3fe9725
Compare
9d89983 to
d652f5f
Compare
5b84354 to
7d5c1d2
Compare
d652f5f to
f924078
Compare
7d5c1d2 to
0984845
Compare
3436479 to
f08152f
Compare
0984845 to
2452073
Compare
| }; | ||
|
|
||
| // Agent first, ADP second: `stats.sent` and `stats.max_packed` are indexed in this order. | ||
| let batch = Batch::sample(); | ||
| let batch = driver::sample(); | ||
| let stats = driver::run(batch, vec![agent_socket, adp_socket])?; | ||
| let received = stats.received; | ||
| let agent_sent = stats.sent[0]; |
| /// Strange-but-`ParseFloat`-valid metric values: signed zeros, infinities, NaN, | ||
| /// hex-float, underscore, bare-dot forms, a `:`-packed run, and cursed-long but | ||
| /// exact encodings. Anything that fails `ParseFloat` stays out so a whole feral | ||
| /// line reads clean. The unparseable curses live in name-like fields instead. | ||
| pub(crate) const ABERRANT_VALUE: &[&[u8]] = &[ |
| let mut rng = AntithesisRng; | ||
| for _ in 0..count { | ||
| let mut bytes = Vec::new(); | ||
| let line = match dogstatsd::send(&mut rng, &mut bytes, batch.vibe()) { | ||
| None => Line::Single { bytes }, | ||
| Some(count) => Line::Multi { bytes, count }, | ||
| }; | ||
| if tx.send(line).is_err() { | ||
| let payload = dogstatsd::write_payload(&mut rng, &mut bytes, batch, dogstatsd::PAYLOAD_BYTE_LIMIT); | ||
| if tx.send(Datagram { bytes, payload }).is_err() { | ||
| break; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24520735bc
ℹ️ 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".
|
|
||
| /// Write one `DogStatsD` message of a sampled type to `buf` at the given vibe. | ||
| /// The `dogstatsd_buffer_size` default, via Datadog Agent. | ||
| pub const PAYLOAD_BYTE_LIMIT: usize = 8_192; |
There was a problem hiding this comment.
Respect the sampled DogStatsD buffer size
This hard-codes the payload cap to the Agent default even though Antithesis writes a per-run dogstatsd_buffer_size that can be much smaller (sample_buffer_size returns 128..=65_536 in test/antithesis/harness/src/config.rs:243-248). When a differential run samples, for example, a 512-byte buffer, write_payload can still pack and send up to 8 KiB in one UDS datagram, so the targets are exercising truncation/drop behavior instead of the intended DogStatsD payload and stats.sent counts lines that were never readable by the SUT. The payload limit needs to come from the sampled config, or the sampler must not choose values below this cap.
Useful? React with 👍 / 👎.
| const COMPLIANT_RATE: &[&[u8]] = &[b"1", b"0.5", b"0.25", b"0.1", b"0.001"]; | ||
|
|
||
| const ABERRANT_RATE: &[&[u8]] = &[ | ||
| b"+0.5", b"1.", b".5", b"0x1p-1", b"1_000", b"2", b"inf", b"+inf", b"-inf", b"nan", |
There was a problem hiding this comment.
Keep sample-rate fuzz values accepted by ADP
When a feral metric extension chooses 0x1p-1 or one of the non-finite/out-of-range entries here, ADP rejects the whole metric in parse_dogstatsd_metric: sample-rate chunks are parsed with all_consuming(double) and then SampleRate::try_from, so parse/range failures bubble out instead of ignoring only the extension. In differential runs this can make generated metrics disappear before the context comparison; keep this pool to decimal 0..=1 values both targets accept, or make these forms scenario-specific.
Useful? React with 👍 / 👎.
| /// Strange-but-parseable unix timestamps: leading plus, leading zeros, and the | ||
| /// `i64::MAX` boundary. Base-10 `ParseInt`, always `> 0`, so safe even at the | ||
| /// strict metric `T` site. | ||
| pub(crate) const ABERRANT_TS: &[&[u8]] = &[b"+1700000000", b"0000001700000000", b"9223372036854775807"]; |
There was a problem hiding this comment.
Drop the leading-plus timestamp from shared fuzzing
When a feral metric/event/service-check timestamp chooses +1700000000, ADP rejects the frame because its shared unix_timestamp parser is nom's unsigned parse_u64 (lib/saluki-io/src/deser/codec/dogstatsd/helpers.rs:127-128), which does not accept a leading plus. This pool is now used by the metric T path as well as d: fields, so differential DogStatsD load can drop otherwise valid metric contexts on ADP; remove the plus form or update the parser before generating it.
Useful? React with 👍 / 👎.
| "café".as_bytes(), // non-conforming but valid UTF-8 | ||
| "Ωμέγα".as_bytes(), // Greek | ||
| "日本語".as_bytes(), // CJK | ||
| "🦆".as_bytes(), // emoji, non-ASCII multi-byte |
There was a problem hiding this comment.
Avoid Unicode metric-name segments in differential load
When these new non-ASCII segments are selected for a metric name, ADP drops the frame before it can produce a context: even with permissive decoding, permissive_metric_name only consumes printable ASCII bytes (lib/saluki-io/src/deser/codec/dogstatsd/metric.rs:197-205). In differential runs where the Agent accepts the same valid UTF-8 name, this creates artificial missing contexts rather than testing ADP/Agent equivalence; keep shared metric-name fuzzing to the ASCII subset ADP can parse or gate these entries out of differential payloads.
Useful? React with 👍 / 👎.
f08152f to
55effb5
Compare
2452073 to
f60e8c1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f60e8c1ebe
ℹ️ 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".
| #[must_use] | ||
| pub fn sample() -> Batch { | ||
| let mut rng = AntithesisRng; | ||
| match [Batch::Clean, Batch::Clean, Batch::Feral, Batch::Mixed].choose(&mut rng) { |
There was a problem hiding this comment.
Use structured choices for batch mode
In Antithesis runs this high-level Clean/Feral/Mixed branch is now selected through rand::choose instead of antithesis_sdk::random::random_choice. The SDK documents random_choice as the way to signal structured choices to the Antithesis platform, so hiding this branch behind raw RNG makes experiments less steerable toward the specific batch modes that the later assert_sometimes! anchors require; keep random_choice here or wrap it for runtime categorical choices.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
This commit updates the harness driver to build dogstatsd to a byte limit, in the manner of datadog/lading. I have been unable to investigate SMPTNG-7611 well owing to the error log emission by Datadog Agent. I've added property tests to assert the payload limit is obeyed, accepting that this means making the dogstatsd generator pure with regard to Rng and may not use antithesis SDK's random_* directly.
f60e8c1 to
f116dac
Compare
55effb5 to
5d2184d
Compare
| /// Sample a line composition: half clean, a quarter feral, a quarter mixed. | ||
| #[must_use] | ||
| pub fn sample() -> Batch { | ||
| let mut rng = AntithesisRng; |
| let mut rng = AntithesisRng; | ||
| rng.random_range(0..=10_000u64) |
|
|
||
| let producer = thread::spawn(move || { | ||
| let mut rng = UnwrapErr(AntithesisRng); | ||
| let mut rng = AntithesisRng; |
| /// The `dogstatsd_buffer_size` default, via Datadog Agent. | ||
| pub const PAYLOAD_BYTE_LIMIT: usize = 8_192; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f116dac109
ℹ️ 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".
| /// Strange-but-`ParseFloat`-valid metric values: signed zeros, infinities, NaN, | ||
| /// hex-float, underscore, bare-dot forms, a `:`-packed run, and cursed-long but | ||
| /// exact encodings. Anything that fails `ParseFloat` stays out so a whole feral | ||
| /// line reads clean. The unparseable curses live in name-like fields instead. |
There was a problem hiding this comment.
Keep feral metric values parseable by ADP
When this value pool selects 0x1p4 or 1_000 for the metric value field, ADP rejects the metric because FloatIter parses values with Rust str::parse::<f64>(), which does not accept Go-style hex floats or underscores, even though the Agent's strconv.ParseFloat does. In differential runs that means otherwise valid metric contexts disappear only on the ADP side; remove those two forms from the shared metric-value pool or gate them to an Agent-only/parser-compatibility scenario.
Useful? React with 👍 / 👎.

Summary
This commit updates the harness driver to build dogstatsd to a byte
limit, in the manner of datadog/lading. I have been unable to
investigate SMPTNG-7611 well owing to the error log emission by Datadog
Agent. I've added property tests to assert the payload limit is obeyed,
accepting that this means making the dogstatsd generator pure with
regard to Rng and may not use antithesis SDK's random_* directly.
I have tweaked also how 'feral' load is produced, making payloads more
compact but also avoiding producing feral load that Datadog Agent will
not outright reject.
Change Type
How did you test this PR?
References
Discovered the following debugging this work:
DataDog/datadog-agent#53170
SMPTNG-761