libdatadog update to c79d783f#3983
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f826cc728b
ℹ️ 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".
| @@ -1 +1 @@ | |||
| Subproject commit 6760faaeeda1cfcf634410105f93cf7149265592 | |||
| Subproject commit c79d783f79f4a2d1e637906f3323600c6e2b5b17 | |||
There was a problem hiding this comment.
Regenerate the sidecar FFI signature
This libdatadog bump changes ddog_sidecar_session_set_config by inserting retry_interval_milliseconds after flush_interval_milliseconds, but the checked-in components-rs/sidecar.h and the call in ext/sidecar.c still use the old argument list. Because C compiles against the stale header, this will not be caught at compile time; at runtime the new Rust FFI function will read every argument after the flush interval in the wrong slot, so normal sidecar startup can misconfigure intervals/sizes and eventually interpret non-pointer values as strings or callbacks. Please regenerate/update the header and pass the new retry interval at the call site as part of this bump.
Useful? React with 👍 / 👎.
| AssignmentReason::Static => REASON_STATIC, | ||
| AssignmentReason::TargetingMatch => REASON_TARGETING_MATCH, | ||
| AssignmentReason::Split => REASON_SPLIT, | ||
| AssignmentReason::Default => REASON_DEFAULT, |
There was a problem hiding this comment.
Handle invalid flag configs as defaults
The new libdatadog revision also adds EvaluationError::FlagConfigurationInvalid, which get_assignment can return for a requested flag whose per-flag config is invalid/unsupported; upstream FFE FFI maps that case to DEFAULT with no error so callers get their supplied default. This wrapper only added the new assignment reason, while the error match below still sends the new error through _ => (ERROR_GENERAL, REASON_ERROR), so those flags will now surface as evaluation errors instead of default evaluations. Please add an explicit arm for the new error variant.
Useful? React with 👍 / 👎.
|
Benchmarks [ tracer ]Benchmark execution time: 2026-06-15 10:38:27 Comparing candidate commit b26baf8 in PR branch Found 0 performance improvements and 3 performance regressions! Performance is the same for 191 metrics, 0 unstable metrics.
|
122e2ec to
3a44de2
Compare
Automated update by CI pipeline https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-php/-/pipelines/118698656 Full CI result: ❌ 14 job(s) failed
d3d1bae to
9388ac2
Compare
Automated update by CI pipeline https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-php/-/pipelines/118728668 Full CI result: ❌ 608 job(s) failed
Summary
Automated update of the libdatadog submodule to the latest HEAD.
$LIBDATADOG_PINNED_SHAc79d783f79f4a2d1e637906f3323600c6e2b5b17Full CI result: ❌ 608 job(s) failed
CI pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-php/-/pipelines/118728668
libdatadog Integration Report
libdatadog SHA: c79d783f79f4a2d1e637906f3323600c6e2b5b17
Analysis date: 2026-06-15
Overall status
A single libdatadog FFI signature change accounts for all 608 failing jobs. It was an
ABI mismatch that compiled cleanly but corrupted the stack at runtime, crashing the PHP
process on essentially every request. Adapting our checked-in C header and the one call
site fixes it.
Build & test summary
The delta for this update is small.
masterpins libdatadog6760faae(green); thisbranch (
bot/libdatadog-latest) bumps it toc79d783f, adding exactly 6 commits:c79d783ffix(ffe): honor shared fixture result metadata (#2109)AssignmentReason::Defaultenum variant — already adapted by the bot incomponents-rs/ffe.rs. 0 FFE jobs failed.bdc0a629chore(crashtracking)!: remove frame count field (#2114)with_experimental_frame_count; we never call it. No effect.8907887cfix(trace-utils): mark decoded span maps as deduped (#2110)aceec12bfeat(sidecar): add retry interval configuration (#2106)a21b946afeat(trace-utils)!: change buffer implementation (#2055)change_bufferis dd-trace-js-only (we don't use it); the only PHP-visible change is additiveVecMap::{clear,drain}. No effect.da8cbcb8feat(shared-runtime)!: use weak waker in trigger (#2050)Key observation: none of the 608 traces contained a Rust/C compilation error or a
Rust panic/backtrace — every one was a hard
SIGSEGV(exit 139 / signal 11 / core dump)in the
phpprocess itself, across all three sub-products:phpsegfaults immediately on running the test suite (e.g.test_distributed_tracingsegfaults even withdatadog.trace.sidecar_trace_sender=0).fails; core dumps collected (
core.php.*). The appsec extension.phptsuite itselfpasses 343/346, failing only
client_init_sidecar.phpt— the one test that exercisessidecar session setup.
run-tests.phpsegfaults on startup.A universal crash on the path common to all three (sidecar session initialization), with
no compile error and no panic, is the fingerprint of an FFI ABI mismatch.
Non-trivial changes made
Root cause: stale cbindgen header for
ddog_sidecar_session_set_configlibdatadog #2106 inserted a new parameter into the FFI function, between
flush_interval_millisecondsandremote_config_poll_interval_millis:The C headers in
components-rs/*.hare checked-in artifacts, regenerated only by themake cbindgentarget — not during the extension build. The bot bumped the submodule butdid not regenerate them, so
components-rs/sidecar.hstill declared the old signature, andext/sidecar.cstill called it with the old argument list. Because the stale prototype andthe call site agreed, the C code compiled with no error.
At runtime, however, the linked libdatadog symbol reads arguments per the new ABI. Every
argument from the
retry_intervalslot onward is shifted by one — including the pointer/slicearguments that follow (
log_level,log_path, the remote-config product/capability pointers,the notify function pointer, the tags vector, hostname/service). libdatadog dereferences these
misaligned (garbage) pointers during session setup →
SIGSEGV. Since session config is set upon essentially every process/request, this crashes everything that loads
ddtrace.so(which isall tracer, appsec and profiler jobs).
Fix (2 files):
components-rs/sidecar.h— addeduint32_t retry_interval_milliseconds,afteruint32_t flush_interval_milliseconds,in theddog_sidecar_session_set_configprototype, matching what
make cbindgenwould regenerate.ext/sidecar.c— passed a value for the new parameter at the call site. Used100(milliseconds), which preserves prior behavior: before [BUG] Multiple unique TypeError's are being grouped as the same and have the same fingerprint #2106 the sidecar used
RetryStrategy::default()whosedelay_mswasDuration::from_millis(100). After thechange the value is threaded through as
RetryStrategy::new(5, retry_interval, Exponential, None), so100reproduces the previous default exactly. (dd-trace-php hasno user-facing config for this yet; a dedicated
DD_*setting can be wired up separatelyif desired.)
This was the only header-facing API change in the entire delta — verified by diffing
all
extern "C"/#[no_mangle]/#[repr(C)]surfaces between6760faaeandc79d783f;the only hit is
retry_interval_milliseconds.Identified libdatadog issues
None identified. The crash was caused by our stale checked-in header, not a libdatadog
defect. (#2106 is a legitimate, intentional FFI addition; the breakage is on the
integration side because the bot's automated bump does not run
make cbindgen.)Flaky / ignored failures
None. All 608 failures share the single root cause above and are expected to clear once the
sidecar config ABI is realigned. Notable downstream symptoms that are not separate issues:
assert resp.statusCode() == 200failures — secondaryeffects of the php-fpm worker crashing mid-request.
harness retries against a continuously-crashing worker until the job times out.
/cc @bwoebi