feat(sidecar)!: introduce TraceData to unify text and binary data#1247
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 7 commits intomainfrom Jan 22, 2026
Merged
feat(sidecar)!: introduce TraceData to unify text and binary data#1247gh-worker-dd-mergequeue-cf854d[bot] merged 7 commits intomainfrom
gh-worker-dd-mergequeue-cf854d[bot] merged 7 commits intomainfrom
Conversation
BenchmarksComparisonBenchmark execution time: 2026-01-22 13:35:57 Comparing candidate commit 66032bf in PR branch Found 10 performance improvements and 12 performance regressions! Performance is the same for 35 metrics, 2 unstable metrics. scenario:benching deserializing traces from msgpack to their internal representation
scenario:credit_card/is_card_number/37828224631000521389798
scenario:credit_card/is_card_number/x371413321323331
scenario:credit_card/is_card_number_no_luhn/ 378282246310005
scenario:credit_card/is_card_number_no_luhn/378282246310005
scenario:credit_card/is_card_number_no_luhn/37828224631000521389798
scenario:credit_card/is_card_number_no_luhn/x371413321323331
scenario:normalization/normalize_name/normalize_name/Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Lo...
scenario:normalization/normalize_name/normalize_name/bad-name
scenario:normalization/normalize_service/normalize_service/A0000000000000000000000000000000000000000000000000...
scenario:normalization/normalize_service/normalize_service/Test Conversion 0f Weird !@#$%^&**() Characters
scenario:single_flag_killswitch/rules-based
CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
Group 14
Group 15
Group 16
Group 17
Group 18
Group 19
BaselineOmitted due to size. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1247 +/- ##
==========================================
+ Coverage 71.08% 71.10% +0.02%
==========================================
Files 417 419 +2
Lines 67035 67067 +32
==========================================
+ Hits 47650 47691 +41
+ Misses 19385 19376 -9
🚀 New features to boost your workflow:
|
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-apple-darwin
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-apple-darwin
x86_64-unknown-linux-gnu
|
bwoebi
commented
Sep 29, 2025
datadog-trace-utils/src/span/mod.rs
Outdated
| } | ||
| /// TraceData implementation using `Bytes` and `BytesString`. | ||
| #[derive(Default, Debug, Clone, PartialEq, Serialize)] | ||
| pub struct TinyData; |
Contributor
Author
There was a problem hiding this comment.
I don't know about the name. If you have better suggestions, feel free.
Contributor
There was a problem hiding this comment.
I think BytesData is better.
bantonsson
reviewed
Sep 30, 2025
datadog-trace-utils/src/span/mod.rs
Outdated
| /// Trait representing a tuple of (Text, Bytes) types used for different underlying data structures. | ||
| /// Note: The functions are internal to the msgpack decoder and should not be used directly: they're | ||
| /// only exposed here due to the inavailability of min_specialization in stable Rust. | ||
| pub trait TraceData: Default + Debug + Clone + PartialEq + Serialize { |
Contributor
There was a problem hiding this comment.
Why the Clone and PartialEq?
Contributor
Author
There was a problem hiding this comment.
PartialEq is necessary because v04::Span is PartialEq. And yeah, generic types need to have all these derives of the context they are used in, otherwise #[derive] doesn't work.
I have no idea though why I added Clone. Removing.
Contributor
Author
There was a problem hiding this comment.
No, Clone is needed for some cases where we have to clone spans (e.g. benchmarks and some tests).
Contributor
There was a problem hiding this comment.
Yeah, would be nice to mark PartialEq and Clone as test convenience things.
Contributor
Author
There was a problem hiding this comment.
How? Like by a comment? or a cfg_attr?
Contributor
There was a problem hiding this comment.
I'm not sure if there's a best practice for conditional trait requirements. You can of course make some other trait which is defined twice behind a #[cfg(test)] gates, one with and one without the extra requirements.
Contributor
There was a problem hiding this comment.
I don't think it's important enough to warrant conditional trait requirements. Was just an observation that it's only used from tests as far as I can see.
Contributor
Author
There was a problem hiding this comment.
Yeah, cfg_attr doesn't work anyway, that'd need at least a feature flag. That's ugly.
bantonsson
reviewed
Sep 30, 2025
bwoebi
force-pushed
the
bob/tracedata
branch
3 times, most recently
from
9bbe018 to
d207172
Compare
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🔗 Commit SHA: c2ae111 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Contributor
VianneyRuhlmann
left a comment
VianneyRuhlmann
left a comment
There was a problem hiding this comment.
Thanks for adding the TraceData trait, it's a nice addition and makes even more sense with V1.
The span::Span was supposed to be a common interface for spans regardless of the encoding. It made sense with v04 and v05 as they have basically the same fields but since v1 has different fields (e.g. attributes vs meta/metrics) we probably won't be able to share a common representation. This also means we will have to reimplement all trace utils for v1 span.
Regarding the changes to the decoder I'm not a fan of having unsafe functions implemented on the SpanBytes. Also I think these changes should be addressed in a separate PR as they're not tied to the addition of TraceData.
| state.serialize_field("array_value", &wrapped_value)?; | ||
| } | ||
| } | ||
| fn get_mut_slice(buf: &mut Self::Bytes) -> &mut &'static [u8]; |
Contributor
There was a problem hiding this comment.
I feel like these functions should be implemented on the type of the buffer rather than on the TraceData type. Maybe in Buffer(B) B shouldn't be T::Bytes but a different type that implements a trait to be deserialized to Span<(T::Text,T::Bytes)>
Contributor
Author
There was a problem hiding this comment.
That's mostly for convenience for now, so that Buffer -> T::Text works trivially. I wanted to not overcomplicate it, unless there's a real need for that. But it should not be overly problematic to change that in future if needed (e.g. you could then add a BufferData trait which holds (Out: TraceData, In: BufferInput) or such). Thanks to this already being T, it would be then a simple find T: TraceData & replace by a new T: BufferData operation at most places and returning T::Out::Text/Bytes then.
Comment on lines
+64
to
+66
| /// `T` is the type used to represent strings in the span, it can be either owned (e.g. BytesString) | ||
| /// or borrowed (e.g. &str). To define a generic function taking any `Span<T>` you can use the | ||
| /// [`SpanValue`] trait: |
Contributor
There was a problem hiding this comment.
Doc needs to be updated to match the change to TraceData
|
|
||
| /// Internal Buffer used to wrap msgpack data for decoding. | ||
| /// Provides a couple accessors to extract data from the buffer. | ||
| pub struct Buffer<T: TraceData>(T::Bytes); |
Contributor
There was a problem hiding this comment.
Why can't we use &[u8] for the buffer ?
Contributor
Author
There was a problem hiding this comment.
We need to at least store the Bytes reference to construct the Bytes/BytesString object.
| @@ -26,7 +26,7 @@ use std::io::Write; | |||
| #[cfg(feature = "compression")] | |||
| use zstd::stream::write::Encoder; | |||
|
|
|||
| #[derive(Debug, Clone)] | |||
| #[derive(Debug)] | |||
Contributor
There was a problem hiding this comment.
The changes to send_data are not linked to TraceData. Could you put them in a separate PR ?
Contributor
Author
There was a problem hiding this comment.
They are, because it relied on Clone on the Span, and TraceData is not meant to be Clone outside of testing. (but these changes are also relatively minor)
| }) | ||
| #[inline] | ||
| fn get_mut_slice<'b>(buf: &'b mut &'a [u8]) -> &'b mut &'static [u8] { | ||
| unsafe { std::mem::transmute::<&'b mut &[u8], &'b mut &'static [u8]>(buf) } |
Contributor
There was a problem hiding this comment.
This function is unsafe and should at least be marked as such.
Contributor
Author
There was a problem hiding this comment.
Yeah, the generic variant on the trait probably should be.
Contributor
Author
My plan is to have a projection with some functions which is compatible with all versions of the spans. Meaning that adding a float attribute will reroute to metrics, everything else to string. That's why moving the span to v04 (and add a new span to v1), then we can add a common projection which routes accordingly, agnostic to the actual backend span used. I definitely want to avoid re-implementing everything. |
Contributor
Author
They sort of are, because the point of this PR is to also make use of TraceData, especially the meta_struct part making use of TraceData::Bytes. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To override this behavior, add the keep-open label or update the PR. |
github-actions
bot
added
the
stale
github-actions
bot
removed
the
stale
bwoebi
force-pushed
the
bob/tracedata
branch
3 times, most recently
from
a033377 to
bc07ed6
Compare
dd-devflow
bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: waiting
mergequeue-status: queued
labels
gh-worker-dd-mergequeue-cf854d
bot
merged commit d430cbd
into
main
41 of 42 checks passed
dd-devflow
bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
hoolioh
added a commit
that referenced
this pull request
Feb 10, 2026
# Release proposal for libdd-tinybytes and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-tinybytes **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-tinybytes-v1.1.0` ### Commits - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: add changelog for every published crate (#1396) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Julio <[email protected]>
This was referenced Feb 16, 2026
This was referenced Feb 18, 2026
paullegranddc
pushed a commit
that referenced
this pull request
Feb 23, 2026
# Release proposal for libdd-data-pipeline and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-common **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-common-v2.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - feat!: enable non-blocking DNS for reqwest (#1558) - feat: unify Azure tags (#1553) - feat(common): add current thread id API (#1569) - refactor!: switch from multipart to multer to resolve deprecation warnings and dependabot alerts (#1540) - feat: single source of truth for headers (fixes issue in profiling with missing headers) (#1493) - refactor!: make reqwest available in common (#1504) ## libdd-ddsketch **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-ddsketch-v1.0.1` ### Commits - chore: remove manual changelog modifications (#1472) - build: update `prost` crates (#1426) - chore: add changelog for every published crate (#1396) ## libdd-trace-protobuf **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-trace-protobuf-v1.1.0` ### Commits - chore: remove manual changelog modifications (#1472) - build: update `prost` crates (#1426) - chore: add changelog for every published crate (#1396) - Handle null span tag values (#1394) ## libdd-dogstatsd-client **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-dogstatsd-client-v1.0.1` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - chore: release libddcommon-v1.1.0 (#1456) - chore: add changelog for every published crate (#1396) - style: fix recent clippy warnings (#1346) ## libdd-telemetry **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-telemetry-v3.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - chore(deps): bump tracing-subscriber to remove regex dep duplicate (#1608) - feat(telemetry)!: add process_tags to Application in telemetry (#1459) - fix(telemetry)!: fix logs payload format [APMSP-2590] (#1498) - feat(appsec): add endpoints collection (#1182) ## libdd-trace-normalization **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-trace-normalization-v1.0.1` ### Commits - Prevent span start overflow panic (#1373) - [CHAOSPLT-932] Add support for internal fuzzing infra (#1372) - chore: add changelog for every published crate (#1396) ## libdd-trace-utils **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-utils-v2.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - feat: unify Azure tags (#1553) - fix(serverless): set hostname on stats from tracer to empty string (#1530) - chore: remove manual changelog modifications (#1472) - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: release libddcommon-v1.1.0 (#1456) - fix(test-agent): undo commenting arg in docker cmd (#1439) - [SLES-2652] Log error details when trace request fails (2) (#1441) - build: update `prost` crates (#1426) - chore(trace-utils): bump the test agent version used for integration tests (#1417) - [Serverless] Skip AAS metadata tagging when span is from API Management (#1409) - chore: add changelog for every published crate (#1396) - Handle null span tag values (#1394) - [SVLS-7934] Log error details when trace request fails (#1392) - Fix trace utils clippy warning (#1397) - feat(trace_utils): Allow sending trace stats using custom HTTP client (#1345) ## libdd-trace-stats **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-trace-stats-v1.0.1` ### Commits - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: add changelog for every published crate (#1396) ## libdd-data-pipeline **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-v2.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - refactor(trace_exporter)!: remove Proxy TraceExporter input mode (#1583) - refactor(libdd-data-pipeline): health metrics (#1433) - feat(data-pipeline)!: include reason for chunks dropped telemetry (#1449) - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: release libddcommon-v1.1.0 (#1456) - chore: prepare libdd-telemetry-v2.0.0 (#1457) - Allow submitting Vec<Vec<Span>> asynchronously (#1302) - test(data-pipeline): handle EINTR in test_health_metrics_disabled (#1430) - chore: add changelog for every published crate (#1396) [APMSP-2590]: https://datadoghq.atlassian.net/browse/APMSP-2590?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com>
dd-octo-sts bot
added a commit
that referenced
this pull request
Feb 25, 2026
# Release proposal for libdd-data-pipeline and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-common **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-common-v2.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - feat!: enable non-blocking DNS for reqwest (#1558) - feat: unify Azure tags (#1553) - feat(common): add current thread id API (#1569) - refactor!: switch from multipart to multer to resolve deprecation warnings and dependabot alerts (#1540) - feat: single source of truth for headers (fixes issue in profiling with missing headers) (#1493) - refactor!: make reqwest available in common (#1504) ## libdd-ddsketch **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-ddsketch-v1.0.1` ### Commits - chore: remove manual changelog modifications (#1472) - build: update `prost` crates (#1426) - chore: add changelog for every published crate (#1396) ## libdd-trace-protobuf **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-trace-protobuf-v1.1.0` ### Commits - chore: remove manual changelog modifications (#1472) - build: update `prost` crates (#1426) - chore: add changelog for every published crate (#1396) - Handle null span tag values (#1394) ## libdd-dogstatsd-client **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-dogstatsd-client-v1.0.1` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - chore: release libddcommon-v1.1.0 (#1456) - chore: add changelog for every published crate (#1396) - style: fix recent clippy warnings (#1346) ## libdd-telemetry **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-telemetry-v3.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - chore(deps): bump tracing-subscriber to remove regex dep duplicate (#1608) - feat(telemetry)!: add process_tags to Application in telemetry (#1459) - fix(telemetry)!: fix logs payload format [APMSP-2590] (#1498) - feat(appsec): add endpoints collection (#1182) ## libdd-trace-normalization **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-trace-normalization-v1.0.1` ### Commits - Prevent span start overflow panic (#1373) - [CHAOSPLT-932] Add support for internal fuzzing infra (#1372) - chore: add changelog for every published crate (#1396) ## libdd-trace-utils **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-utils-v2.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - feat: unify Azure tags (#1553) - fix(serverless): set hostname on stats from tracer to empty string (#1530) - chore: remove manual changelog modifications (#1472) - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: release libddcommon-v1.1.0 (#1456) - fix(test-agent): undo commenting arg in docker cmd (#1439) - [SLES-2652] Log error details when trace request fails (2) (#1441) - build: update `prost` crates (#1426) - chore(trace-utils): bump the test agent version used for integration tests (#1417) - [Serverless] Skip AAS metadata tagging when span is from API Management (#1409) - chore: add changelog for every published crate (#1396) - Handle null span tag values (#1394) - [SVLS-7934] Log error details when trace request fails (#1392) - Fix trace utils clippy warning (#1397) - feat(trace_utils): Allow sending trace stats using custom HTTP client (#1345) ## libdd-trace-stats **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-trace-stats-v1.0.1` ### Commits - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: add changelog for every published crate (#1396) ## libdd-data-pipeline **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-v2.0.0` ### Commits - refactor(ddcommon)!: remove direct dependency on hyper client everywhere in common (#1604) - refactor(trace_exporter)!: remove Proxy TraceExporter input mode (#1583) - refactor(libdd-data-pipeline): health metrics (#1433) - feat(data-pipeline)!: include reason for chunks dropped telemetry (#1449) - feat(sidecar)!: introduce TraceData to unify text and binary data (#1247) - chore: release libddcommon-v1.1.0 (#1456) - chore: prepare libdd-telemetry-v2.0.0 (#1457) - Allow submitting Vec<Vec<Span>> asynchronously (#1302) - test(data-pipeline): handle EINTR in test_health_metrics_disabled (#1430) - chore: add changelog for every published crate (#1396) [APMSP-2590]: https://datadoghq.atlassian.net/browse/APMSP-2590?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also move Span structures to v04, to make space for v1 spans (which will eventually become the new default). TraceData is also going to be used in the V1 implementation, to carry around byte arrays and strings alike, separate from the indexed offsets into the big vector.
This trivially allows directly working on tinybytes data or u8 slices, depending on the input, rather than requiring a copy-conversion at the end for tinybytes input. This is a major performance penalty (benchmarks say about 20% gain - I'm really confused why the regression introduced by #1004 was deemed acceptable).
This also addresses "APMSP-1941 - Replace
Byteswith a wrapper that borrows the underlying".Note that this PR partially reverts #1139 by @shreyamalpani - but instead provides the ability to pass an endpoint directly when sending.
This was done by the deliberate choice to remove Clone from Span, which is quite an expensive operation and should simply never be done. Especially the goal is for v1 spans to possibly store the backing storage for strings alongside the actual TracerPayload (which v1 has), at which point such an operation would be prohibitively expensive.