fix(crashtracking): isolate receiver from bootstrap instrumentation - #19735
Closed
taegyunkim wants to merge 2 commits into
Closed
taegyunkim wants to merge 2 commits into
taegyunkim wants to merge 2 commits into
Conversation
taegyunkim
added
the
changelog/no-changelog
Contributor
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 2 jobs - 2 passed on retry 🔗 Commit SHA: baeb209 | Docs | Datadog PR Page | Give us feedback! |
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
Circular import analysis
|
taegyunkim
removed
the
changelog/no-changelog
taegyunkim
changed the title
BenchmarksBenchmark execution time: 2026-08-17 20:50:27 Comparing candidate commit baeb209 in PR branch Found 0 performance improvements and 8 performance regressions! Performance is the same for 610 metrics, 10 unstable metrics.
|
gyuheon0h
pushed a commit
to DataDog/libdatadog
that referenced
this pull request
Aug 19, 2026
…he receiver (#2361) # 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: ```text unw_get_reg_remote / unw_step_remote -> collect instruction and stack pointers unw_get_proc_name_remote -> search the target's ELF symbol tables for a function name ``` The second operation ran once per frame per thread inside `unwind_remote_thread`. For affected processes, libunwind segfaulted while searching an ELF symbol table: ```text unw_get_proc_name_remote -> _Ux86_64_get_proc_name -> _Ux86_64_get_proc_name_by_ip -> _Uelf64_get_proc_name -> _Uelf64_get_proc_name_in_image -> _Uelf64_lookup_symbol_closeness # receiver crashes here ``` This destroys the original crash report. Thread collection runs before `builder.build()` and `async_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_callstacks` subsequently 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-run` environments. The parent application's `PYTHONPATH` included `ddtrace/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: ```text Application -> Receiver R1 processes the application crash -> R1 crashes in _Uelf64_lookup_symbol_closeness -> R1's nested crashtracker starts Receiver R2 -> R2 uploads R1's crash report ``` The resulting reports identify `_dd_crashtracker_receiver` as the crashed process and contain instrumentation threads such as `TelemetryWriter`, `RemoteConfigPol`, `RemoteConfigSub`, and `SignalUploader`. 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_closeness` stack. 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](https://app.datadoghq.com/logs?query=service%3Ainstrumentation-telemetry-data%2A%20%28%40tags.severity%3Acrash%20OR%20severity%3Acrash%20OR%20signum%3A%2A%20OR%20%40error.is_crash%3Atrue%29%20%40lib_language%3A%2A%20%40tracer_version%3A%2A%20-%40tags.crash_runtime%3Atrue%20%40org_id%3A%2A%20%40error.stack.frames.function%3A_Ux86_64_get_proc_name&agg_m=count&agg_m_source=base&agg_t=count&clustering_pattern_field_path=message&cols=host%2Cservice&event=AwAAAaAN2Q4H5mHhTgAAABhBYUFOMlE0SEFBQW83Ukt0YlUxRkxBQVIAAAAkMDFhMDBkZGYtOTZkNC00YWU3LWFhN2UtMzNhMjYwYzQxZTIxAAIv6g&fromUser=true&messageDisplay=inline&refresh_mode=paused&storage=hot&stream_sort=time%2Cdesc&viz=stream&from_ts=1786334400000&to_ts=1786939199999&live=false) 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](DataDog/dd-trace-py#19735) fixes that separate auto-instrumentation bug by removing the exact `ddtrace/bootstrap` directory from the receiver's inherited `PYTHONPATH` while 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? 1. **Keep remote stack walking.** The receiver still uses `unw_init_remote`, `unw_get_reg_remote`, and `unw_step_remote` to collect every thread's instruction and stack pointers. 2. **Remove receiver-side libunwind symbol lookup.** `unw_get_proc_name_remote` is no longer called, so the receiver does not enter the faulting ELF symbol-table search. 3. **Use blazesym once.** `CrashInfo::enrich_callstacks` remains responsible for converting addresses into function names, files, and line numbers. 4. **Handle an exited target process.** Process-based blazesym symbolization requires `/proc/<pid>`. The sidecar receiver can outlive the application, so this PR falls back to the ELF `path` and virtual `relative_address` already recorded by `normalize_ip`. This needs no live process. 5. **Degrade instead of losing the report.** If both blazesym paths fail, the frame retains its IP, SP, and any normalized build ID/path/relative address; the symbolization error is added to `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](DataDog/libdatadog-libunwind#14), with the underlying fork change in [DataDog/libunwind #3](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`. ```bash cargo run -p bin_tests --bin prebuild cargo test -p bin_tests --test crashtracker_bin_test cargo test -p libdd-crashtracker --features generate-unit-test-files --lib cargo clippy -p libdd-crashtracker --all-targets ``` Local Linux x86_64 results: - **45/45 crashtracker integration tests passed.** This includes the three multi-thread collection tests that assert `worker_fn_0` and `worker_fn_1` are present in `error.threads[].stack.frames[].function`, proving blazesym still names remotely collected thread frames without `unw_get_proc_name_remote`. - **161 unit tests passed, 1 ignored.** - `cargo clippy` passed. - Added `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 recover `my_function`. - Disabling the ELF fallback makes the new unit test fail and also makes `test_crash_tracking_sidecar_multi_thread_collection` fail with `failed to open proc maps file /proc/<pid>/maps`, confirming that the fallback covers a real path rather than an artificial unit-test condition.
Contributor
Author
|
Closing in favor of #20084 |
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
1 participant
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.
Description
Prevents the Python crashtracker receiver from auto-instrumenting itself when the parent process was started through Single Step Instrumentation or
ddtrace-run.Receiver construction now removes only the exact
<ddtrace package>/bootstrapentry derived from the receiver script path while preserving the rest ofPYTHONPATH, including the selected ddtrace package location.LD_LIBRARY_PATHandDYLD_LIBRARY_PATHinheritance are unchanged.The regression captures the generated receiver environment and starts its configured Python interpreter. It verifies both that
ddtrace.internal.native._nativeremains importable and thatddtrace.bootstrap.preloadis not loaded.This fixes receiver self-instrumentation, not the separate libdatadog receiver segfault in remote libunwind symbol resolution.
Testing
bfe6e1c5:ddtrace.bootstrap.preloadwas present insys.modules.dd-gitlab/core/crashtracker 1/2failed as required.baeb209e:scripts/run-tests --venv 1ef26c5 -- -s -- tests/crashtracker/test_crashtracker.py -k receiver_pythonpath_isolation -vvpassed.scripts/run-tests --venv 1ef26c5 -- -s -- tests/crashtracker/test_crashtracker.py -vvpassed all 28 tests on Python 3.13.scripts/lint typing -- ddtrace/internal/core/crashtracking.pyandscripts/lint checkspassed.dd-gitlab/core/crashtrackershards passed.Risks
Do not merge this PR before the separate libdatadog receiver crash is fixed, or before equivalent receiver self-failure telemetry is added. The nested crashtracker currently makes that receiver segfault observable. Removing self-instrumentation alone would make the existing crash-report loss silent.
The code change is narrowly scoped to exact-match filtering of the receiver's bootstrap directory from
PYTHONPATH.Additional Notes
The same bootstrap propagation occurs with SSI and
ddtrace-run, so the fix is not SSI-specific.