Conversation
Binary Size Analysis (Agent Data Plane)Baseline: 216e499 · Comparison: 5b376c9 · 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 ( |
98c08db to
33667e2
Compare
33667e2 to
61d5ec0
Compare
a0a6ecf to
d6f56de
Compare
ed6878e to
3bd7f18
Compare
Move the three functions that turn the Agent's config stream into config settings from the binary into agent-data-plane-config-system. Their bodies and tests are unchanged; the binary calls them from there. Add a test-only loader that turns one recorded corpus case into the exact stream of ConfigEvent messages the Agent sent: the first snapshot rebuilt from its three layers, then each update in sequence order. Numbers that do not fit a double exactly are errors. The vendored proto has no unset_source field, so the loader drops it, as prost does on the real wire. Tests round-trip every corpus case and check the stream against rules that do not repeat the loader's own arithmetic.
The config system folds each Agent update, then deserializes, translates and validates the result in one go. Pull that into one step that reports each stage on its own, and call it from both the first snapshot and the update loop. Production still boots only when every stage passes and still keeps the last good config when an update is rejected. Move the ConfigEvent to ConfigUpdate match from the binary into the library next to the other stream conversions. Add a test-only driver that replays each recorded corpus case through these same steps. It commits an update when translation passes, even if validation fails, because the recorded cases carry no API key. It keeps the result of every stage so later tests can compare them with the Agent.
Tests need to read, for a dotted Agent setting key, the typed value ADP deserialized, and to know what kind of value it is. The witness trait walks every key, but it clones each value and loses the key. Generate a second table next to it: every supported key, its serde aliases, and a borrowed accessor that returns one variant per leaf type. A leaf type the generator does not know fails the build.
Add a test-only set of rules that decide whether a typed value ADP deserialized matches what an Agent getter returned for the same key. There is one rule for each kind of leaf and the getter it stands in for, never one per key. A pair with no rule is counted as not compared instead of guessed. Values are compared exactly. The only allowances are ones Go code cannot see: map key order, a nil list or map against an empty one, and an unset optional value against the Go zero value. Write the rules down in docs/comparison.md next to the other record docs.
For every recorded case, fold the Agent's stream as the Agent applied it and read each supported key from ADP's typed config. Compare that value with each getter result the Agent recorded, after the first snapshot and after the last update. A value ADP refuses to deserialize fails the whole config, and the error does not name the key. So each key is also deserialized on its own, and a test checks that doing this agrees with the whole config. Keys ADP does not model are counted, not failed.
Replaying the recorded Agent cases now ends in a check. Every result that is not a plain match is written to known-results.txt: each key ADP reads differently or refuses, each key whose translation fails, each update ADP would reject, the keys ADP does not model, and the supported keys no case covers. The test fails when the computed results and the file differ in either direction, so a new difference and a fixed one both show up. Setting ADP_CORPUS_REPLAY_BLESS=1 rewrites the file, keeping any notes added to its lines. Give TranslateError a key() method so the replay can name the key of each translation error without parsing text.
A case with no line in the known results might have matched on every key, or had nothing compared. Add one line per case with its counts of matches, differences, rejections and keys not compared, and name each case whose Agent startup failed. Leaf lines now say which kind of value the key holds and what the stream carried, so lines can be grouped by type. Count labels use stable codes instead of message text, and case names are escaped like other fields.
ADP works out the topology stop timeout itself: the configured data_plane.stop_timeout, or else the aggregator and forwarder stop timeouts added together. The Agent computes the same value once at load and streams it, so the two can disagree once an input changes. Move that computation from the binary into the config library so tests can call it, and add a derived tier to the corpus replay that compares it with what the Agent's getter returned. The known results now show two cases where ADP picks a longer timeout than the Agent, and list the derived values the replay does not yet cover, each with its reason.
Before the Agent's config stream arrives, and in standalone mode, ADP reads datadog.yaml and DD_* variables itself. Split that reader so a pure function takes the file text and an explicit environment; the startup path still reads the file and the process environment and calls it. Add a bootstrap tier to the corpus replay: feed each recorded case's YAML and environment to that function and compare each supported key with what the Agent's getter returned after startup. The known results now list the inputs that make ADP abort its boot, the YAML and environment values ADP reads differently, and the values the Agent writes at load that ADP's own reader cannot see.
The known results file listed every result that differs from the Agent's getters, but not why. Declare divergence types in the file, each with the layer a fix would land in (ADP's deserializer, translator, bootstrap reader, defaults or derived values; or the Agent's load or wire, which ADP cannot fix), and annotate every divergence line with its type. The check now fails when a divergence line has no type, names an undeclared one, or a declared type is unused. Regenerating the file keeps an annotation only on a line that is unchanged, so a changed verdict or value must be triaged again.
The replay compared byte-size settings only as strings, so it could not see that ADP parses them differently. The Agent's GetSizeInBytes reads KB, MB and GB as powers of 1024, and ADP's translator reads them as powers of 1000: a 10MB log size is 10485760 bytes in the Agent and 10000000 in ADP, and the defaults differ the same way. Record GetSizeInBytes in two new cases and compare the byte counts ADP translates for log_file_max_size and dogstatsd_log_file_max_size in the derived tier. The divergence is recorded under a new type, not fixed. Also reject integers the corpus loader cannot hold exactly as f64 at the i64 and u64 limits, where the check used to saturate, and document which recorded getters have no leaf rule and how to add a kind or getter.
A schema update now regenerates the Agent config corpus and runs the replay tests: review the corpus diff by key and behavior, check coverage and the Agent's load-time writes, and triage each change in the known results file. The config-system review checklist now runs the replay tests for deserializer, environment reader and translation changes.
The replay now asserts what ADP does with the keys its typed model leaves out: for the unsupported, excluded and unknown case groups, the compatibility classifier reads each streamed value as the overlay declares it, and replaying the case fails no stage apart from the blank api_key validation. The derived tier now names every value the translator computes from settings. The ones no recorded Agent getter returns are listed with the reason, such as the trace sample rate default the Agent applies after its getter. Also corrects the recorder README, which said replay did not exist, and the comparison contract's account of blank api_key failures.
Rewords module and item docs for a reader new to the replay tests and corrects three comments the code contradicted: the getters a leaf kind stands for, how the loader handles unset_source, and the rows unmodeled keys produce.
GitHub collapses it in diffs, as it does the other generated configuration tables.
Replace the generated replay baseline and blessing command with exact per-check expectations. Separate known bugs from deliberate behavior, report all failures, and keep assertions stable across generated batches. Check intermediate deserialization, translation, and validation results. Exercise valid and rejected recorded updates through the running system. Catch production panics without stopping unrelated cases.
3bd7f18 to
fc288a5
Compare
Attach Saluki issue numbers to known replay differences and include the links in failure diagnostics. Keep exact per-check expectations. Document the consumer and provenance comparisons still needing work. Require a tracking issue for every known-bug expectation.
There was a problem hiding this comment.
#2762) ## Summary The Datadog streams `null` for an empty or cleared list or map. We need to handle this as an empty collection. Fixes #2750. ## Test plan - [x] New reader unit tests in `list_de.rs` and `cast_de.rs`: `null` and empty inputs read as empty, null `replace_tags` elements read as empty maps (sequence and JSON-string forms) - [x] New `null_collection_tests` in `datadog-agent-config`: every collection leaf in the generated model accepts `null`; `null` yields empty while an absent `histogram_aggregates` keeps its default - [x] New `system.rs` tests: a startup snapshot with null collections translates, and a `null` update clears a previously non-empty `additional_endpoints`/`histogram_aggregates` (both fail without the fix) - [x] New translator test: `replace_tags` rules missing `name` or `pattern` (including `[null]`) are rejected with the trace-agent's messages - [ ] Follow-up once #2722 lands: remove the replay expectations this makes match 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com>
#2762) ## Summary The Datadog streams `null` for an empty or cleared list or map. We need to handle this as an empty collection. Fixes #2750. ## Test plan - [x] New reader unit tests in `list_de.rs` and `cast_de.rs`: `null` and empty inputs read as empty, null `replace_tags` elements read as empty maps (sequence and JSON-string forms) - [x] New `null_collection_tests` in `datadog-agent-config`: every collection leaf in the generated model accepts `null`; `null` yields empty while an absent `histogram_aggregates` keeps its default - [x] New `system.rs` tests: a startup snapshot with null collections translates, and a `null` update clears a previously non-empty `additional_endpoints`/`histogram_aggregates` (both fail without the fix) - [x] New translator test: `replace_tags` rules missing `name` or `pattern` (including `[null]`) are rejected with the trace-agent's messages - [ ] Follow-up once #2722 lands: remove the replay expectations this makes match 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com> 6e3f308
Human Summary
Adds tests from a corpus of actual Agent output (see #2717 to learn how). Seems pretty valuable. It has found the below issues that either need to be fixed or investigated. The design allows for bugs to exist. So these issues listed here are locked in by the tests that found them, and when they are fixed we will flip the test assertion to a green assertion.
AI Summary
#2717 records what the real Datadog Agent streams and what its configuration getters return. This PR uses those recordings to test what ADP actually reads. The distinction matters: a setting declared as a map can arrive as a JSON string, and the Agent's getter can accept a value that Rust rejects. Hand-written fixtures in the expected schema shape missed exactly that failure in #2700.
The goal is to make those compatibility checks part of everyday configuration development, without running the Go Agent in CI. The checked-in corpus of recordings supplies the Agent evidence; ordinary Rust unit tests exercise ADP's production code. Existing differences are covered explicitly, so we can add regression protection now and fix bugs in separate PRs.
What the replay checks
The tests live in
lib/agent-data-plane-config-system/src/corpus_replay/. They look at the same recordings from several angles:DatadogConfigurationfor comparison with its corresponding Agent getter, after the initial snapshot and after the final update. Settings are also deserialized in isolation so one malformed value does not hide failures in unrelated fields."10MB"but disagree on the byte count used at runtime.Comparison rules are defined by Rust value kind and Agent getter, not by setting name, so a type-level fix is checked across the recorded settings of that kind.
Most recordings have no API key. Value comparisons therefore remain independent of whether the whole configuration is runnable; otherwise missing-key validation would obscure nearly all the interesting inputs. Validation still has its own assertions, and the connected test exercises the real accept-or-reject behavior.
The production changes make these paths reusable rather than copying them into a test implementation: stream conversion moves out of the binary, startup and updates share a stage evaluator, and bootstrap loading accepts explicit inputs. The shutdown-timeout calculation is shared too. No runtime behavior change is intended.
Known differences are assertions, not exemptions
For value comparisons, matching the recorded Agent getter is the default expectation.
corpus_replay/expected.rsholds hand-edited Rust entries for individual checks that currently differ. Each names the exact ADP value or error and explains whether it is a known bug or intentional behavior. Known-bug expectations also carry a Saluki tracking issue through their shared cause; missing issue references fail the replay test, and failure diagnostics link the issue. The follow-ups are tracked under #2169. An intentional rejection is a passing assertion of that specific rejection, not permission to return any error.For example, ADP currently interprets
"10MB"as 10,000,000 bytes, while the Agent'sGetSizeInBytesreturns 10,485,760. The test pins today's ADP result as a known bug. Fixing it makes that check fail with “This check now matches the Agent” and points to the expectation to remove. A different wrong byte count also fails. This PR records the difference; it does not fix it.Expectations cannot exempt an entire case or automatically cover new checks. Duplicate or unused entries fail, and check identities survive generated batch renames. There is no generated ADP-results baseline or blessing command.
Failures are collected in stable order and identify the case, input location, setting, checkpoint or update, expected and actual ADP results, recorded Agent result, and expectation location. A caught production panic always fails the affected case; other cases still run.
Working with these tests
The replay runs with the normal Rust unit-test suite, without Go or Docker. For a focused run:
An ADP fix changes production code and the relevant expectation together—not the recorded Agent corpus. Schema updates are different: after support decisions settle, regenerate the recordings, review the upstream behavior changes, and run replay. This PR adds those steps to the configuration maintenance guidance.
This is not complete equivalence testing. Some recorded getters, provenance-dependent section reads, and other computed values still lack comparisons;
lib/datadog-agent/config-recorder/docs/comparison.mdand the replay code document those gaps rather than count them as matches. Live transport, authentication, and timing are outside this PR. Structured consumer decoding is tracked in #2758, effective percentile parsing in #2759, and provenance/section comparisons in #2760. Existing resolved-default findings remain in #1802 and #2484.Change Type
How did you test this PR?
cargo nextest run --lib --bins -p agent-data-plane-config-system -p datadog-agent-config-corpus -p datadog-agent-config -p agent-data-plane-config -p agent-data-plane(655 passed).make fmt,make check-docs,make check-clippy,make check-fmt,make check-unused-deps,make check-licenses,make check-deny,cargo check --workspace --tests.corpus_replaytests passed;make fmt,make check-docs, andmake check-clippypassed. Temporary mutations verified that improved and changed byte-size results print Match core agent byte-size parsing for Datadog configuration #2751, and that a known-bug expectation without an issue fails. All mutations were restored.References