Fix #3135: Force flushing on shutdown of entry point processes#3398
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3398 +/- ##
==========================================
- Coverage 61.87% 61.74% -0.13%
==========================================
Files 141 141
Lines 12481 12481
Branches 1630 1630
==========================================
- Hits 7722 7706 -16
- Misses 4038 4054 +16
Partials 721 721 see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Benchmarks [ tracer ]Benchmark execution time: 2025-09-22 15:40:53 Comparing candidate commit 31e13fc in PR branch Found 1 performance improvements and 7 performance regressions! Performance is the same for 185 metrics, 1 unstable metrics. scenario:MessagePackSerializationBench/benchMessagePackSerialization
scenario:PDOBench/benchPDOOverhead
scenario:PDOBench/benchPDOOverheadWithDBM
scenario:PHPRedisBench/benchRedisOverhead
scenario:SamplingRuleMatchingBench/benchRegexMatching1-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching2-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching3-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching4-opcache
|
1edd421 to
4d81d94
Compare
4d81d94 to
0edd29d
Compare
Also avoid retrying a trace flush, that's pretty much pointless. Signed-off-by: Bob Weinand <[email protected]>
Signed-off-by: Bob Weinand <[email protected]>
0edd29d to
31e13fc
Compare
| dd_signal_data.uc = uc; | ||
|
|
||
| if (ddtrace_sidecar) { | ||
| // Spawn a thread using clone() to perform sidecar cleanup asynchronously to avoid async unsafeness in the signal handler |
There was a problem hiding this comment.
I don't understand. We're supposed to be forcing a flush because the entry point (or a child of it) is shutting down. But this makes a thread... why would this have a realistic chance to complete before the process exits?
Presumably the profiler should so something similar, but it's less dire (only the most recent partial minute is lost, probably).
There was a problem hiding this comment.
If the thread continues executing and regularly shuts down - there's not a problem. (The signal handler resuming basically just continues normal execution.)
All this does is slightly deferring the signal delivery by the time it needs to flush the traces, essentially. dd_call_prev_handler only calls the prev handler after flushing.
Does that make sense?
4 participants
Cgroup roots as well as container root processes generally cause the container to terminate on end. This currently drops all pending traces on the floor.
Enforce a synchronous flush here, by default for PID=1 and PPID=1.