fix(crashtracking): stop resolving thread symbols with libunwind in the receiver - #2361
Conversation
📚 Documentation Check Results📦
|
df1aff7 to
ab7cbb3
Compare
🔒 Cargo Deny Results📦
|
BenchmarksComparisonBenchmark execution time: 2026-08-18 22:33:14 Comparing candidate commit 97f8eba in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.
|
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 97f8eba | 1787091806 | taegyunkim/crashtracker-receiver-symbolization-fix |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| receiver_entry_point/report/2644 | execution_time | 3.537ms | 3.569ms ± 0.018ms | 3.566ms ± 0.010ms | 3.577ms | 3.602ms | 3.614ms | 3.655ms | 2.49% | 1.310 | 3.323 | 0.49% | 0.001ms | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| receiver_entry_point/report/2644 | execution_time | [3.566ms; 3.571ms] or [-0.069%; +0.069%] | None | None | None |
Baseline
Baseline benchmark details
Group 1
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | aab513c | 1787075760 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| receiver_entry_point/report/2644 | execution_time | 3.547ms | 3.574ms ± 0.017ms | 3.572ms ± 0.009ms | 3.581ms | 3.605ms | 3.627ms | 3.641ms | 1.92% | 1.173 | 1.906 | 0.47% | 0.001ms | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| receiver_entry_point/report/2644 | execution_time | [3.572ms; 3.576ms] or [-0.065%; +0.065%] | None | None | None |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 97f8eba | Docs | View more details | Give us feedback! |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR removes in-receiver symbol name resolution during ptrace unwinding (capturing only IP/SP), and improves post-processing symbolization by adding an ELF/offset fallback when the target process has already exited.
Changes:
- Stop resolving frame symbol names via
unw_get_proc_name_remoteduring ptrace collection; capture raw addresses only. - Update thread context streaming/capture APIs to drop the “resolve frames” parameter.
- Enhance
StackFrame::resolve_namesto fall back from/proc/<pid>symbolization to ELF + virtual offset, and add a regression test for “process already exited”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| libdd-crashtracker/src/receiver/receive_report.rs | Removes passing the “resolve frames” setting into thread collection after collector no longer supports per-frame symbolization. |
| libdd-crashtracker/src/receiver/ptrace_collector.rs | Removes remote proc-name symbolization and updates thread-capture APIs accordingly. |
| libdd-crashtracker/src/crash_info/stacktrace.rs | Adds ELF/offset fallback symbolization and a test ensuring symbolization works after the process exits. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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
|
7bc664d to
52a3c44
Compare
…he receiver The receiver called unw_get_proc_name_remote() for every frame of every thread it collected over ptrace. libunwind then searched the target process's ELF symbol tables, and a fault inside _Uelf64_lookup_symbol_closeness killed the receiver. Because thread collection runs before the report is built and uploaded, the crash report was lost entirely: dd-trace-py telemetry for 4.13.x shows 5,492 receiver crashes with that exact signature in five days, 98.9% of them in the libunwind symbol lookup, against 47 delivered application crash reports for the worst-affected service. The names were also redundant. CrashInfo::enrich_callstacks runs blazesym over every thread stack under the same EnabledWithSymbolsInReceiver setting and overwrites StackFrame::function, so libunwind's result was discarded. Drop the call. blazesym alone could not cover the sidecar receiver, which outlives the crashing process and so loses /proc/<pid>: add an ELF-file fallback to StackFrame::resolve_names using the path and virtual offset that normalize_ip already recorded, which needs nothing from the live process. This also recovers symbol names for the crashing thread's own stack in that case. Verified with the existing bin_tests: all 45 crashtracker integration tests pass, including the three multi-thread collection tests that assert worker function names in error.threads. Without the ELF fallback, test_crash_tracking_sidecar_multi_thread_collection fails with "failed to open proc maps file /proc/<pid>/maps".
52a3c44 to
f30b297
Compare
# What does this PR do? This RFC was out of date. Especially with [fix(crashtracking): stop resolving thread symbols with libunwind in the receiver](#2361), we should update the doc with the new changes + other tweaks that have been made in the past that has not been reflected in the documentation # Motivation What inspired you to submit this pull request? # Additional Notes I enumerated the differences manually, had claude verify, and update the doc. I went through it again manually to make sure all was correct # How to test the change? Describe here in detail how the change can be validated.
Summary
Keep libunwind for remote stack walking, stop using it for redundant and crash-prone ELF symbol lookup, and let blazesym do the one symbolization pass with an ELF fallback when the application is already gone.
Why?
The crashtracker uses a separate receiver process because the crashing application's signal handler cannot safely parse binaries, collect other thread stacks, or upload a report itself.
When all-thread collection is enabled, the receiver ptrace-attaches each application thread and uses libunwind's remote API to walk its stack. Stack walking and symbolization are separate operations:
The second operation ran once per frame per thread inside
unwind_remote_thread. For affected processes, libunwind segfaulted while searching an ELF symbol table:This destroys the original crash report. Thread collection runs before
builder.build()andasync_upload_to_endpoint, so a receiver that crashes here never uploads the application's report.The risky lookup was also redundant on successful paths.
CrashInfo::enrich_callstackssubsequently asks blazesym to symbolize the same thread frames and overwrites the libunwind function name. If blazesym failed, the libunwind name previously remained as an accidental fallback, but retaining that fallback meant risking the entire report. An uploaded report with unresolved addresses is preferable to losing all crash data.How did we detect this?
The Python receiver was unintentionally auto-instrumenting itself in affected SSI and
ddtrace-runenvironments. The parent application'sPYTHONPATHincludedddtrace/bootstrap, and that path was forwarded to the receiver. On startup, the receiver ran the full ddtrace preload and started its own nested crashtracker.That nested crashtracker did not cause the libunwind fault, but it made the fault observable:
The resulting reports identify
_dd_crashtracker_receiveras the crashed process and contain instrumentation threads such asTelemetryWriter,RemoteConfigPol,RemoteConfigSub, andSignalUploader.In a 120-hour dd-trace-py 4.13.x window we found 5,492 receiver crashes. 5,431 (98.9%) contain the libunwind
get_proc_name/_Uelf64_lookup_symbol_closenessstack. The worst-affected customer produced 5,389 receiver crashes against only 47 delivered application crash reports.Representative receiver crash and matching events in Datadog Logs
Without the receiver's accidental self-instrumentation, the libunwind crash and application report loss would still occur, but the receiver death would be largely silent.
dd-trace-py #19735 fixes that separate auto-instrumentation bug by removing the exact
ddtrace/bootstrapdirectory from the receiver's inheritedPYTHONPATHwhile preserving the injected ddtrace package path. It prevents the receiver from starting tracing, profiling, and a nested crashtracker. That PR intentionally remains draft until this libdatadog fix lands, because merging it first would remove the telemetry that exposes these receiver deaths without preventing the deaths or recovering the lost application reports.How does this PR fix the problem?
unw_init_remote,unw_get_reg_remote, andunw_step_remoteto collect every thread's instruction and stack pointers.unw_get_proc_name_remoteis no longer called, so the receiver does not enter the faulting ELF symbol-table search.CrashInfo::enrich_callstacksremains responsible for converting addresses into function names, files, and line numbers./proc/<pid>. The sidecar receiver can outlive the application, so this PR falls back to the ELFpathand virtualrelative_addressalready recorded bynormalize_ip. This needs no live process.log_messages; and the receiver still uploads the report.How did we test this?
A standalone reproducer and fundamental ELF parser hardening are available in DataDog/libdatadog-libunwind #14, with the underlying fork change in DataDog/libunwind #3. The reproducer ptrace-stops a helper in a valid shared library, corrupts its on-disk GNU hash metadata after loading, and calls
unw_get_proc_name_remote()on that frame. Before the parser fix it terminates with SIGSEGV in the same call chain observed in telemetry; after the fix it returns-UNW_ENOINFO.Local Linux x86_64 results:
worker_fn_0andworker_fn_1are present inerror.threads[].stack.frames[].function, proving blazesym still names remotely collected thread frames withoutunw_get_proc_name_remote.cargo clippypassed.test_symbolization_after_process_exit. It normalizes a frame while the process is available, then symbolizes against a nonexistent process and verifies that the recorded ELF path and virtual offset recovermy_function.test_crash_tracking_sidecar_multi_thread_collectionfail withfailed to open proc maps file /proc/<pid>/maps, confirming that the fallback covers a real path rather than an artificial unit-test condition.