Skip to content

feat!: Align tracer FFI error and response types with common conventions#2029

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 14 commits into
mainfrom
lloeki/ffi-for-trace-exporter-improved
May 29, 2026
Merged

feat!: Align tracer FFI error and response types with common conventions#2029
gh-worker-dd-mergequeue-cf854d[bot] merged 14 commits into
mainfrom
lloeki/ffi-for-trace-exporter-improved

Conversation

@lloeki
Copy link
Copy Markdown
Member

@lloeki lloeki commented May 22, 2026

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).

@datadog-official
Copy link
Copy Markdown

datadog-official Bot commented May 22, 2026

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 90.59%
Overall Coverage: 72.93% (+0.03%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: ed5becf | Docs | Datadog PR Page | Give us feedback!

@lloeki lloeki changed the base branch from main to lloeki/ffi-for-trace-exporter May 22, 2026 12:24
@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter-improved branch from 405e95f to dc8aeb9 Compare May 22, 2026 12:42
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

Clippy Allow Annotation Report

Comparing clippy allow annotations between branches:

  • Base Branch: origin/main
  • PR Branch: origin/lloeki/ffi-for-trace-exporter-improved

Summary by Rule

Rule Base Branch PR Branch Change

Annotation Counts by File

File Base Branch PR Branch Change

Annotation Stats by Crate

Crate Base Branch PR Branch Change
clippy-annotation-reporter 5 5 No change (0%)
datadog-ffe-ffi 1 1 No change (0%)
datadog-ipc 21 21 No change (0%)
datadog-live-debugger 6 6 No change (0%)
datadog-live-debugger-ffi 10 10 No change (0%)
datadog-profiling-replayer 4 4 No change (0%)
datadog-remote-config 3 3 No change (0%)
datadog-sidecar 57 57 No change (0%)
libdd-common 13 13 No change (0%)
libdd-common-ffi 12 12 No change (0%)
libdd-data-pipeline 5 5 No change (0%)
libdd-ddsketch 2 2 No change (0%)
libdd-dogstatsd-client 1 1 No change (0%)
libdd-profiling 13 13 No change (0%)
libdd-telemetry 20 20 No change (0%)
libdd-tinybytes 4 4 No change (0%)
libdd-trace-normalization 2 2 No change (0%)
libdd-trace-obfuscation 3 3 No change (0%)
libdd-trace-stats 1 1 No change (0%)
libdd-trace-utils 13 13 No change (0%)
Total 196 196 No change (0%)

About This Report

This 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.

@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter-improved branch from dc8aeb9 to b3784be Compare May 22, 2026 12:50
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 22, 2026

Codecov Report

❌ Patch coverage is 90.58824% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.93%. Comparing base (f7d471d) to head (ed5becf).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2029      +/-   ##
==========================================
+ Coverage   72.90%   72.93%   +0.02%     
==========================================
  Files         460      460              
  Lines       76396    76463      +67     
==========================================
+ Hits        55696    55765      +69     
+ Misses      20700    20698       -2     
Components Coverage Δ
libdd-crashtracker 65.32% <ø> (+0.09%) ⬆️
libdd-crashtracker-ffi 36.82% <ø> (ø)
libdd-alloc 98.77% <ø> (ø)
libdd-data-pipeline 85.65% <90.58%> (+0.04%) ⬆️
libdd-data-pipeline-ffi 76.17% <90.58%> (+0.47%) ⬆️
libdd-common 79.89% <ø> (ø)
libdd-common-ffi 74.41% <ø> (ø)
libdd-telemetry 73.37% <ø> (+0.02%) ⬆️
libdd-telemetry-ffi 31.36% <ø> (ø)
libdd-dogstatsd-client 82.64% <ø> (ø)
datadog-ipc 76.22% <ø> (ø)
libdd-profiling 81.68% <ø> (-0.02%) ⬇️
libdd-profiling-ffi 64.79% <ø> (ø)
libdd-sampling 97.46% <ø> (ø)
datadog-sidecar 29.19% <ø> (ø)
datdog-sidecar-ffi 10.17% <ø> (ø)
spawn-worker 48.86% <ø> (ø)
libdd-tinybytes 93.80% <ø> (ø)
libdd-trace-normalization 81.71% <ø> (ø)
libdd-trace-obfuscation 87.30% <ø> (ø)
libdd-trace-protobuf 68.25% <ø> (ø)
libdd-trace-utils 88.94% <ø> (ø)
libdd-tracer-flare 86.88% <ø> (ø)
libdd-log 74.83% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter branch from 9d7998b to 5e2089f Compare May 22, 2026 13:13
@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter-improved branch from b3784be to 34f795a Compare May 22, 2026 13:13
@lloeki lloeki marked this pull request as ready for review May 26, 2026 10:53
@lloeki lloeki requested review from a team as code owners May 26, 2026 10:53
@lloeki
Copy link
Copy Markdown
Member Author

lloeki commented May 26, 2026

There are some CI failures but I'm opening this for review.

@lloeki lloeki requested review from hoolioh and yannham May 26, 2026 10:54
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1425e682b1

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread libdd-data-pipeline-ffi/src/tracer.rs Outdated
@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter branch from 5e2089f to b9ea727 Compare May 26, 2026 11:05
@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter-improved branch from 1425e68 to 3bd0ef7 Compare May 26, 2026 11:05
@ekump ekump force-pushed the lloeki/ffi-for-trace-exporter-improved branch from 8ed5a05 to e6df3ba Compare May 26, 2026 17:00
@ekump ekump requested review from a team as code owners May 26, 2026 17:00
@dd-octo-sts
Copy link
Copy Markdown
Contributor

dd-octo-sts Bot commented May 26, 2026

Artifact Size Benchmark Report

aarch64-alpine-linux-musl
Artifact Baseline Commit Change
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.a 82.94 MB 82.95 MB +.01% (+11.78 KB) 🔍
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.so 7.63 MB 7.63 MB 0% (0 B) 👌
aarch64-unknown-linux-gnu
Artifact Baseline Commit Change
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.so 10.25 MB 10.25 MB +0% (+240 B) 👌
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.a 94.02 MB 94.03 MB +.01% (+12.66 KB) 🔍
libdatadog-x64-windows
Artifact Baseline Commit Change
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.dll 24.54 MB 24.54 MB +0% (+2.50 KB) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.lib 83.96 KB 83.96 KB 0% (0 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.pdb 178.23 MB 178.26 MB +.01% (+24.00 KB) 🔍
/libdatadog-x64-windows/debug/static/datadog_profiling_ffi.lib 915.25 MB 915.28 MB +0% (+38.09 KB) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.dll 8.03 MB 8.03 MB +.02% (+2.00 KB) 🔍
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.lib 83.96 KB 83.96 KB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.pdb 23.77 MB 23.78 MB +.03% (+8.00 KB) 🔍
/libdatadog-x64-windows/release/static/datadog_profiling_ffi.lib 47.43 MB 47.43 MB +.01% (+8.97 KB) 🔍
libdatadog-x86-windows
Artifact Baseline Commit Change
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.dll 21.27 MB 21.28 MB +0% (+1.50 KB) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.lib 85.29 KB 85.29 KB 0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.pdb 182.23 MB 182.26 MB +.01% (+32.00 KB) 🔍
/libdatadog-x86-windows/debug/static/datadog_profiling_ffi.lib 908.37 MB 908.44 MB +0% (+66.93 KB) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.dll 6.20 MB 6.20 MB +.02% (+1.50 KB) 🔍
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.lib 85.29 KB 85.29 KB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.pdb 25.48 MB 25.49 MB +.03% (+8.00 KB) 🔍
/libdatadog-x86-windows/release/static/datadog_profiling_ffi.lib 45.07 MB 45.08 MB +.01% (+7.55 KB) 🔍
x86_64-alpine-linux-musl
Artifact Baseline Commit Change
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.a 73.95 MB 73.96 MB +.01% (+9.76 KB) 🔍
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.so 8.52 MB 8.53 MB +.04% (+4.00 KB) 🔍
x86_64-unknown-linux-gnu
Artifact Baseline Commit Change
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.a 89.36 MB 89.37 MB +.01% (+10.47 KB) 🔍
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.so 10.36 MB 10.36 MB +0% (+192 B) 👌

Base automatically changed from lloeki/ffi-for-trace-exporter to main May 26, 2026 18:05
@ekump ekump force-pushed the lloeki/ffi-for-trace-exporter-improved branch from e6df3ba to 49384c0 Compare May 26, 2026 19:09
Comment thread libdd-data-pipeline-ffi/src/tracer.rs Outdated
Comment thread libdd-data-pipeline-ffi/src/response.rs
Comment thread libdd-data-pipeline-ffi/src/tracer.rs Outdated
Copy link
Copy Markdown
Contributor

@yannham yannham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @hoolioh, and in general we should keep the catch_panic

Comment thread libdd-data-pipeline-ffi/src/response.rs Outdated
Comment thread libdd-data-pipeline-ffi/src/tracer.rs Outdated
lloeki added 2 commits May 28, 2026 13:35
Switch tracer span and chunk construction functions from
`Option<Box<ExporterError>>` to `MaybeError` (`Option<ddog_Error>`).
These functions only produce error messages (the error code is never
used by callers), so the simpler `ddog_Error` type is sufficient.

This lets C consumers use the existing `read_ddogerr_string_and_drop`
and `get_error_details_and_drop` helpers instead of custom error
handling.

`send_trace_chunks` retains `ExporterError` because callers need
the error code for HTTP response classification.
Replace the separate `*const u8` return + `out_len` out-parameter with
a single `ByteSlice` return value. This bundles pointer and length
into one FFI-safe struct, matching the existing slice conventions in
libdd-common-ffi and avoiding the awkward split between return value
and out-parameter.
lloeki and others added 6 commits May 28, 2026 13:35
Wrap the `fields` parameter in `Option` so the function can check for
null and return an error instead of undefined behavior. cbindgen
produces the same C signature (`const ddog_TracerSpanFields *`).
@hoolioh hoolioh changed the title feat: Align tracer FFI error and response types with common conventions feat!: Align tracer FFI error and response types with common conventions May 28, 2026
@lloeki lloeki force-pushed the lloeki/ffi-for-trace-exporter-improved branch from 10ceab4 to ce54277 Compare May 28, 2026 11:51
@hoolioh hoolioh self-requested a review May 28, 2026 12:14
Copy link
Copy Markdown
Contributor

@yannham yannham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some nits, but LGTM otherwise 👍

Comment thread libdd-data-pipeline-ffi/src/tracer.rs Outdated
@hoolioh
Copy link
Copy Markdown
Contributor

hoolioh commented May 29, 2026

/merge

@gh-worker-devflow-routing-ef8351
Copy link
Copy Markdown

gh-worker-devflow-routing-ef8351 Bot commented May 29, 2026

View all feedbacks in Devflow UI.

2026-05-29 09:16:25 UTC ℹ️ Start processing command /merge


2026-05-29 09:16:32 UTC ℹ️ MergeQueue: waiting for PR to be ready

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.
It will be added to the queue as soon as checks pass and/or get approvals. View in MergeQueue UI.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2026-05-29 09:30:15 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in main is approximately 49m (p90).


2026-05-29 10:12:59 UTC ℹ️ MergeQueue: This merge request was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants