feat: Add FFI for trace exporter#1952
Conversation
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
96342f8 to
303d914
Compare
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 693a1ca | Docs | Datadog PR Page | Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1952 +/- ##
==========================================
+ Coverage 72.76% 72.79% +0.03%
==========================================
Files 458 459 +1
Lines 75789 76106 +317
==========================================
+ Hits 55147 55405 +258
- Misses 20642 20701 +59
🚀 New features to boost your workflow:
|
2b39efc to
2701231
Compare
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
|
|
Review comments addressed. If acceptable, I suggest we merge this one as a first iteration of the FFI API. A new stacked PR based on this one is brewing, which has more contentious API changes coming from the dd-trace-rb side review. |
hoolioh
left a comment
There was a problem hiding this comment.
I just left a comment requesting a change, other than that LGTM overall for a first version.
9d7998b to
5e2089f
Compare
Introduce an opaque `TracerSpan` handle wrapping `Span<BytesData>` and expose it to C callers via four FFI functions: - `ddog_tracer_span_new`: create a span with all scalar fields - `ddog_tracer_span_free`: release the span - `ddog_tracer_span_set_meta`: add a string tag - `ddog_tracer_span_set_metric`: add a numeric tag This enables language tracers (e.g. Ruby) to build spans field-by-field through the C API, bypassing msgpack serialization on the caller side.
Introduce an opaque `TracerTraceChunks` handle wrapping `Vec<Vec<SpanBytes>>` and expose it to C callers via four FFI functions: - `ddog_tracer_trace_chunks_new`: create a container with optional capacity hint - `ddog_tracer_trace_chunks_free`: release the container - `ddog_tracer_trace_chunks_begin_chunk`: start a new trace chunk - `ddog_tracer_trace_chunks_push_span`: move a `TracerSpan` into the current chunk, consuming it Spans are consumed on push to enforce single-ownership and prevent double-use from C callers.
Wire `TracerTraceChunks` to the existing `TraceExporter::send_trace_chunks` method through a new C-callable function. The chunks are consumed on call and the agent response is optionally written to an out-parameter. This completes the span-building FFI surface: callers can now construct spans via `ddog_tracer_span_*`, group them via `ddog_tracer_trace_chunks_*`, and send them via `ddog_trace_exporter_send_trace_chunks` — all without serializing to msgpack on the caller side. The `TraceExporter` type alias in `trace_exporter.rs` is widened to `pub(crate)` to allow cross-module access.
Replace the `match` with identity `Ok` arm with `map_err`, which is the idiomatic Rust combinator for transforming only the error side of a `Result`.
The inner fields are only accessed within the `tracer` module itself, so `pub(crate)` is unnecessarily broad. Default (private) visibility is sufficient since the module owns all access.
`Vec::with_capacity(0)` and `Vec::new()` are identical — both create an empty vec with no heap allocation — so the branch is unnecessary.
…race_chunks`
Replace the `match` with `let Some(...) = ... else { return ... }`,
which is the idiomatic Rust pattern for early-return on a refutable
binding (stabilized in Rust 1.65).
`make_minimal_span` and `make_chunks` have no safety preconditions for their callers — all unsafety is self-contained. Move the `unsafe` from the function signature into internal blocks so the functions present a safe interface.
Extract the span-building logic into an inner closure that returns `Result`, allowing the `?` operator to propagate conversion errors. The outer body calls `inner().err()` to map `Ok(()) -> None` and `Err(e) -> Some(e)`, matching the FFI return type.
Replace `NonNull<Box<T>>` with `&mut MaybeUninit<Box<T>>` for out-parameters in `ddog_tracer_span_new` and `ddog_tracer_trace_chunks_new`. This makes the uninitialized nature of the slot explicit in the type system and allows using `MaybeUninit::write` instead of raw `ptr::write`. Note that `&mut Box<T>` cannot be used here: assignment through `&mut Box<T>` drops the old value first, which is undefined behavior when the slot is uninitialized. `MaybeUninit::write` overwrites without dropping, which is the correct semantics for out-parameters. cbindgen produces identical C signatures (`T **out_handle`) for all three representations.
Accept a capacity hint for pre-allocating the inner span vector, avoiding reallocations when the number of spans is known beforehand.
…egin_chunk` Change the return type from void to `Option<Box<ExporterError>>` and wrap the body in `catch_panic!` with a null handle check, making the function consistent with every other mutating function in the tracer FFI API.
5e2089f to
b9ea727
Compare
|
9d7998b had all CI OK and now post-rebase there's no CI running? EDIT: GitHub Actions incident
|
b1c81ea to
693a1ca
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
Tests failed on this commit b9ba4bb: What to do next?
|
…ons (#2029) # What does this PR do? Two improvements to the tracer FFI surface in `libdd-data-pipeline-ffi`: 1. **Check for nullable pointers.** 2. **Return `ByteSlice` from `ddog_trace_exporter_response_get_body`.** Replace the separate `*const u8` return + `out_len` out-parameter with a single `ByteSlice` return value, matching the existing slice conventions in `libdd-common-ffi`. # Motivation FUP to #1952. C consumers (dd-trace-rb) can now use the existing `read_ddogerr_string_and_drop` / `get_error_details_and_drop` helpers from `datadog_ruby_common.h` instead of custom error handling for tracer construction functions. The `ByteSlice` return avoids an awkward split between return value and out-parameter for the response body. # Additional Notes AI was used to accelerate implementation; all code was reviewed and understood. # How to test the change? ``` cargo test -p libdd-data-pipeline-ffi --lib ``` All 40 tests pass (13 tracer + 3 response + 24 trace_exporter). Co-authored-by: hoolioh <julio.gonzalez@datadoghq.com> Co-authored-by: ekump <edmund.kump@datadoghq.com>
#What does this PR do? Bumps the libdatadog workspace version from 34.0.0 → 35.0.0 (Cargo.toml + regenerated Cargo.lock) to cut the 35.0.0 release. Breaking changes affecting the FFI layer (34.0.0 → 35.0.0) 1. Tracer FFI: response body now returned as ByteSlice (#2029) — feat! - ddog_trace_exporter_response_get_body no longer takes an out_len: *mut usize out-parameter and returns *const u8. It now returns a single ByteSlice value, matching the slice conventions in libdd-common-ffi. - Tracer constructor functions now validate nullable pointers and return ErrorCode::InvalidArgument instead of risking UB; panics across the boundary are caught and surfaced as ErrorCode::Panic. - C consumers (e.g. dd-trace-rb) can now reuse the standard read_ddogerr_string_and_drop / get_error_details_and_drop helpers from datadog_ruby_common.h. - Action for callers: drop the out_len argument and read length/pointer from the returned ByteSlice. 2. Crashtracker: error.threads flattened to []ThreadData (#2054) — fix(crashtracking)! - The crash-report payload now emits error.threads as an optional flat array of thread objects (unified runtime stack schema 1.8) instead of the previous nested threads object. - This is a wire/payload-format change consumed by downstream pipelines and the product UI. - Action for callers: consumers parsing crash reports must update to the 1.8 thread layout. Other breaking changes since 34.0.0 (Rust API, not C ABI) These are !-marked breaking changes in libdd-trace-utils that affect Rust consumers of the library but do not change the C FFI surface or generated headers: - feat(trace-utils)!: introduce VecMap datastructure (#2022) — adds a HashMap replacement optimized for span construction (not yet wired into span fields). - feat(trace-utils)!: add from_string to span text (#2011) — requires From<String> on SpanText and switches SpanSlice to Cow<str> to allow span-field mutation/normalization. Notable new FFI surface (additive) - feat: Add FFI for trace exporter (#1952) — new tracer FFI surface in libdd-data-pipeline-ffi (the change above, #2029, refines it). Co-authored-by: julio.gonzalez <julio.gonzalez@datadoghq.com>
…r trace exporter FFI (#2051) # What does this PR do? Add fork safety and cooperative cancellation support to the trace exporter FFI: 1. **Fork hooks**: Expose `ddog_trace_exporter_before_fork`, `_after_fork_in_parent`, and `_after_fork_in_child` that delegate to the underlying `SharedRuntime`. These allow C callers to coordinate the tokio runtime lifecycle around process forks. 2. **Cancellation token**: Introduce `ddog_trace_exporter_cancel_token_new`, `_cancel`, and `_drop` for managing cancellation tokens. Thread the token into `ddog_trace_exporter_send_trace_chunks` via `tokio::select!` so callers can cooperatively abort in-flight HTTP requests. # Motivation FUP to #1952. Ruby application servers (Puma, Unicorn, Passenger) fork worker processes. The tokio runtime does not survive `fork()` — its threads are not carried over. Without the fork hooks, the trace exporter is dead in child processes. The existing `RUBY_UBF_IO` unblock function on the dd-trace-rb side sends a signal that cannot actually cancel an in-flight Rust HTTP request. The cancellation token enables cooperative abort: when Ruby interrupts a thread (shutdown, `Thread#kill`), the UBF cancels the token, which causes `tokio::select!` to abort the send and return promptly. # Additional Notes - Added a `pub fn shared_runtime()` accessor to `TraceExporter` since the field was private. - The cancellation token uses `Handle<CancellationToken>` from `libdd-common-ffi`, same pattern as profiling-ffi, with a `ddog_TraceExporterCancelToken` cbindgen rename to avoid symbol conflicts. - The `cancel` parameter on `send_trace_chunks` is nullable; passing `NULL` preserves existing behavior. AI was used to accelerate implementation; all code was reviewed and understood. # How to test the change? ``` cargo test -p libdd-data-pipeline-ffi --lib ``` 46 tests pass (40 existing + 6 new for fork hooks and cancellation token). Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com> Co-authored-by: VianneyRuhlmann <vianney.ruhlmann@datadoghq.com>


What does this PR do?
Add span-building and trace-sending FFI functions to
libdd-data-pipeline-ffi, enabling language tracers to construct spans field-by-field through the C API instead of passing pre-serialized msgpack.New types:
TracerSpan: opaque handle wrappingSpan<BytesData>TracerTraceChunks: opaque handle wrappingVec<Vec<SpanBytes>>New functions:
ddog_tracer_span_new,_free,_set_meta,_set_metricddog_tracer_trace_chunks_new,_free,_begin_chunk,_push_spanddog_trace_exporter_send_trace_chunksMotivation
The Ruby tracer needs a native trace export path that bypasses Ruby-side msgpack serialization. This provides the FFI surface for it; the corresponding C extension in
dd-trace-rbcalls these functions.Continuation of the prototype in #1661, reworked to stay close to
main: the genericTraceExporter<H>andSharedRuntimeare untouched; this is purely additive.Additional Notes
Spans are consumed on push, chunks are consumed on send — single-ownership enforced at the API level.
libdd-trace-utilsis promoted from dev-dependency to regular dependency since the FFI crate now needsSpanBytesat build time.Sibling PR for Ruby is at: DataDog/dd-trace-rb#5690
How to test the change?
cargo test -p libdd-data-pipeline-ffi --lib— 39 tests, all passing. The existingtrace_exportertests are unaffected.