feat(crashtracking)!: collect all threads#1878
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. |
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 6a86bed | Docs | Datadog PR Page | Give us feedback! |
6187d0a to
3f70266
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1878 +/- ##
==========================================
- Coverage 71.73% 71.60% -0.14%
==========================================
Files 434 437 +3
Lines 70395 71112 +717
==========================================
+ Hits 50497 50918 +421
- Misses 19898 20194 +296
🚀 New features to boost your workflow:
|
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
|
e96cb1a to
42d4665
Compare
25853ed to
a4b82cb
Compare
e58c6c4 to
ddd4ab6
Compare
e91789b to
cc21b35
Compare
…:DataDog/libdatadog into gyuheon0h/PROF-14275-collect-all-threads
86a4b60 to
a4ac74d
Compare
|
@codex r |
|
To use Codex here, create a Codex account and connect to github. |
a4ac74d to
3d3e060
Compare
3d3e060 to
dacc179
Compare
yannham
left a comment
There was a problem hiding this comment.
I'm less familiar with the libunwind part, but overall I was able to follow thanks to good comments and the syscall/ptrace business looks sound AFAICT 👍 just left minor/pedantic comments.
…ace, small cleanups
0e9d1d8 to
084db2f
Compare
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results📦
|
58a0dd0 to
4abe584
Compare
Can you update the PR description, I did not see where you read the thread name from |
@gleocadie That was an earlier iteration. fixed |
3008bf4 to
6a86bed
Compare
gleocadie
left a comment
There was a problem hiding this comment.
LGTM
super nice :excited:
…or collect all thread config (#1959) # What does this PR do? [feat(crashtracking)!: collect all threads](#1878) added the functionality to collect all threads. We should expose the config vals for this # Motivation What inspired you to submit this pull request? # Additional Notes Anything else we should know when reviewing? # How to test the change? ``` pub struct Config<'a> { pub additional_files: Slice<'a, CharSlice<'a>>, /// If true, the receiver will collect stack traces for all threads in the crashing process /// (not just the crashing thread) using ptrace-based remote unwinding. pub collect_all_threads: bool, *******THIS IS NEW******* pub create_alt_stack: bool, pub demangle_names: bool, /// The endpoint to send the crash report to (can be a file://). /// If None, the crashtracker will infer the agent host from env variables. pub endpoint: EndpointConfig<'a>, /// Maximum number of non-crashing threads to collect when `collect_all_threads` is true. /// If 0, uses the default (`libdd_crashtracker::default_max_threads()`). pub max_threads: usize, *******THIS IS NEW******* /// Optional filename for a unix domain socket if the receiver is used asynchonously pub optional_unix_socket_filename: CharSlice<'a>, pub resolve_frames: StacktraceCollection, /// The set of signals we should be registered for. /// If empty, use the default set. pub signals: Slice<'a, i32>, /// Timeout in milliseconds before the signal handler starts tearing things down to return. /// If 0, uses the default timeout as specified in /// `libdd_crashtracker::shared::constants::DD_CRASHTRACK_DEFAULT_TIMEOUT`. Otherwise, uses /// the specified timeout value. /// This is given as a uint32_t, but the actual timeout needs to fit inside of an i32 (max /// 2^31-1). This is a limitation of the various interfaces used to guarantee the timeout. pub timeout_ms: u32, pub use_alt_stack: bool, } ```

What does this PR do?
When a process crashes the crashtracker now captures a full stack trace for every live non-crashing thread and includes them in the crash report under error.threads.
Previously only the crashing thread's stack was reported. Customers had no visibility into what other threads were doing at the time of the crash, making it much harder to diagnose race conditions, deadlocks, and cross-thread interactions.
Thread collection runs entirely in the receiver process after it has finished reading the crash pipe. Because the crashed process stays alive in its signal handler until the receiver calls
finish(), all threads remain valid ptrace targets for the entire collection window.The receiver:
/proc/<pid>/task/PTRACE_SEIZE+PTRACE_INTERRUPT_UPT_create/unw_init_remote/unw_step_remote) to walk each thread's stackCrashtrackerConfiguration::set_collect_all_threads(true)and bounded by a max of 256. The user can also set a lower boundary usingCrashtrackerConfiguration::set_max_threads(). The collection timeout is the remaining slice of the receiver's deadline after the crash pipe is fully read, so the total receiver lifetime is always bounded byconfig.timeout()Breaking changes
Updated data schema to 1.7; there is now a
Threadsobject.Errors intake upload still sends an array of
ThreadData, as that is the schema that the enrichment pipeline expectsCrashtrackerConfignow hasAdditional notes
One enhancement we can do is to collect even the crashing thread in the receiver along with all threads, since enumerating all threads touches the crashing thread also. We can do this by checking for the crashing thread first in the receiver, then checking all threads.
However, this will be explored in a future PR/investigation, as this is changing the core crashing thread unwinding logic
How to test the change?
Run a crash with a program with multiple threads, with all thread collection turned on.
Bin tests also added to test e2e flow