-
Notifications
You must be signed in to change notification settings - Fork 166
feat(tracer): add configuration for connection mode #3573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3573 +/- ##
==========================================
- Coverage 61.93% 61.92% -0.02%
==========================================
Files 140 140
Lines 13309 13309
Branches 1762 1762
==========================================
- Hits 8243 8241 -2
- Misses 4275 4278 +3
+ Partials 791 790 -1 see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Benchmarks [ tracer ]Benchmark execution time: 2026-01-19 17:33:59 Comparing candidate commit fc9fcbf in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 192 metrics, 1 unstable metrics. scenario:MessagePackSerializationBench/benchMessagePackSerialization
|
ce20a9e to
2eb0609
Compare
Benchmarks [ profiler ]Benchmark execution time: 2026-01-14 16:24:26 Comparing candidate commit 05ebcae in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 28 metrics, 6 unstable metrics. scenario:php-profiler-timeline-memory-with-profiler-and-timeline
|
34ba942 to
8d17af1
Compare
Signed-off-by: Alexandre Rulleau <[email protected]>
Signed-off-by: Alexandre Rulleau <[email protected]>
Signed-off-by: Alexandre Rulleau <[email protected]>
9429b69 to
f53401c
Compare
Signed-off-by: Alexandre Rulleau <[email protected]>
f53401c to
37ec8f2
Compare
Signed-off-by: Alexandre Rulleau <[email protected]>
06223eb to
cda73e8
Compare
Signed-off-by: Alexandre Rulleau <[email protected]>
|
|
||
| ddtrace_sidecar_shutdown(); | ||
| // Only shutdown sidecar in MSHUTDOWN for non-CLI SAPIs | ||
| // CLI SAPI shuts down in RSHUTDOWN to allow thread joins before ASAN checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does ASAN influence this? ASAN is supposed to run after MSHUTDOWN as well?
I suspect that is related to the timings and the fact that the libdatadog part doesn't properly handle shutdown currently.
And this should in fact always shutdown the sidecar in MSHUTDOWN.
| // Don't try to reconnect in thread mode after fork | ||
| // Let sidecar stay unavailable | ||
| LOG(WARN, "Child process after fork with thread mode: sidecar unavailable"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm, no? Just connect to the parents process socket? At least attempt to.
| LOG(WARN, "pcntl_fork() detected with thread-based sidecar connection. " | ||
| "Thread mode is incompatible with fork and may cause instability. " | ||
| "Consider using subprocess mode (DD_TRACE_SIDECAR_CONNECTION_MODE=subprocess)."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is? Why?
Description
Reviewer checklist