feat(trace-utils): add v1-native agentless JSON encoder brick - #2370
feat(trace-utils): add v1-native agentless JSON encoder brick#2370anais-raison wants to merge 9 commits into
Conversation
Isolated brick for APMSP-2812: adds encode_payload_from_v1 and its v1::Span-native helpers (collect_attrs_v1, flatten_attr_into_v1, encode_span_link_v1, encode_span_event_v1) alongside the existing v0.4 agentless encoder. Not wired into any live send path yet.
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results📦
|
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 7d3deb7 | Docs | View more details | Give us feedback! |
BenchmarksComparisonBenchmark execution time: 2026-09-03 13:10:10 Comparing candidate commit 7d3deb7 in PR branch Found 1 performance improvements and 2 performance regressions! Performance is the same for 137 metrics, 0 unstable metrics.
|
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fbc4b65b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| chunk: &TraceChunk<T>, | ||
| metadata: &TracerMetadata, | ||
| ) -> Result<S::Ok, S::Error> { | ||
| let mut map = ser.serialize_map(None)?; |
There was a problem hiding this comment.
Nit: I think we can pre-compute the size here, avoiding useless allocations: (!metadata.env.is_empty() as usize) + (....) + 1
There was a problem hiding this comment.
@yannham I looked into this but actually for serde_json, the len passed to serialize_map doesn't actually help. The len only matter in the special-case of len == Some(0) (where it closes the object immediately). For any other value it takes the exact same path as None and starts writing fields one by one directly to the output writer. There's no length-prefixed header, no upfront Vec::with_capacity sized from the lenght and Compound::Map doesn't store the length either. So Some(3) vs None produce byte-identical output, no allocation avoided (verified that with a small test comparing the two to be 100% sure).
There was a problem hiding this comment.
Ah, I think I had msgpack in head, but you're right that it doesn't make a difference for JSON which is streaming-style anyway. It could though if in the future agentless move from JSON to a different encoding, so I would say if it's trivial to pre-compute (just re-order some declaration), it's a sanity step that doesn't cost much to do anyway. If it's too complicated or if it does mangle the code, then it's probably not worth it 👍
| )?; | ||
| map.end() | ||
| } | ||
| other => encode_event_scalar_v1(ser, other), |
There was a problem hiding this comment.
It's a matter of taste, but I wonder if we shouldn't just inline encode_event_scalar_v1 here. It's still relatively small and avoid an unreachable. Though one could argue that keep smaller function is better, so no strong opinion either way
3 participants
What does this PR do?
Adds a v1-native equivalent of the existing v0.4 agentless JSON encoder, as groundwork for making
v1::Span/v1::TraceChunkthe exporter's canonical internal type:agentless_encoder::encode_payload_from_v1and its helpers (collect_attrs_v1,flatten_attr_into_v1,encode_span_link_v1,encode_span_event_v1) (libdd-trace-utils)Nothing is wired into the live pipeline yet.
Motivation
Part of APMSP-2812: migration of the exporter from v0.4 to v1 with isolated bricks first before one final breaking PR that will handle the actual swap.
Additional Notes
Pure addition, no behavior change — these functions aren't called anywhere yet, so there's no regression risk.