-
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.91% 61.91%
=======================================
Files 140 140
Lines 13312 13312
Branches 1762 1762
=======================================
Hits 8242 8242
Misses 4280 4280
Partials 790 790 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Benchmarks [ tracer ]Benchmark execution time: 2026-01-22 16:18:20 Comparing candidate commit c59bfda in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 184 metrics, 4 unstable metrics. scenario:MessagePackSerializationBench/benchMessagePackSerialization
scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching1
scenario:SamplingRuleMatchingBench/benchRegexMatching2
scenario:SamplingRuleMatchingBench/benchRegexMatching3
scenario:SamplingRuleMatchingBench/benchRegexMatching4
|
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
|
06223eb to
cda73e8
Compare
ext/ddtrace.c
Outdated
|
|
||
| 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.
ext/ddtrace.c
Outdated
| // 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.
ext/handlers_pcntl.c
Outdated
| 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?
391826e to
2332634
Compare
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
2332634 to
c59bfda
Compare
Description
Reviewer checklist