Repository navigation
fix(config): accept streamed null as an empty configuration collection (backport #2762 to 1.7.x) - #2765
Conversation
#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> (cherry picked from commit 6e3f308)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-datadog-us1-prod[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
| None => return Ok(Vec::new()), | ||
| Some(JsonArrayOrString::Array(values)) => values, | ||
| Some(JsonArrayOrString::String(value)) => { | ||
| serde_json::from_str::<Vec<Option<T>>>(&value).map_err(de::Error::custom)? |
There was a problem hiding this comment.
When a JSON-array setting arrives through the supported encoded-string path with value "null", it is deserialized directly as Vec<Option<T>> and rejected. Environment-originated empty arrays can therefore still prevent configuration application even though literal null arrays are now accepted.
| serde_json::from_str::<Vec<Option<T>>>(&value).map_err(de::Error::custom)? | |
| serde_json::from_str::<Option<Vec<Option<T>>>>(&value) | |
| .map_err(de::Error::custom)? | |
| .unwrap_or_default() |
Was this helpful? React 👍 or 👎
🤖 Bits Code Review · @DataDog review to ask questions · Open Bits AI session
| HashMap::deserialize(de::value::MapAccessDeserializer::new(map)) | ||
| } | ||
|
|
||
| fn visit_unit<E: de::Error>(self) -> Result<Self::Value, E> { |
There was a problem hiding this comment.
The new visitor methods handle literal null maps, but the existing encoded-string path still deserializes "null" directly into a HashMap and rejects it. A map setting such as an environment-originated additional_endpoints=null can still block startup instead of clearing the map.
Was this helpful? React 👍 or 👎
🤖 Bits Code Review · @DataDog review to ask questions · Open Bits AI session
Binary Size Analysis (Agent Data Plane)Baseline: 98ec0b6 · Comparison: 349024f · 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 ( |
2 participants
Summary
Backports #2762 to
releases/1.7.x. The Datadog Agent streamsnullfor an empty or cleared list or map, which ADP previously failed to handle; this treatsnullas an empty collection so cleared settings (e.g.additional_endpoints,histogram_aggregates) are applied instead of rejected. Fixes #2750 on the 1.7 release line.Clean cherry-pick of 6e3f308 with no conflicts.
Test plan
list_de.rs,cast_de.rs,null_collection_tests,system.rs, translatorreplace_tagstest) pass on the 1.7.x base🤖 Generated with Claude Code