-
Notifications
You must be signed in to change notification settings - Fork 9
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
[crashtracking]: add named socket support back in #722
Conversation
BenchmarksComparisonBenchmark execution time: 2024-11-13 03:12:19 Comparing candidate commit f802124 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #722 +/- ##
==========================================
- Coverage 71.16% 71.06% -0.11%
==========================================
Files 287 287
Lines 42739 42824 +85
==========================================
+ Hits 30416 30431 +15
- Misses 12323 12393 +70
|
…/libdatadog into sanchda/fix_crashtracking_socket
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.
Looks good to me, thanks for adding it back :-)
Don't merge yet, need to verify the thing works end-to-end with a native test |
Signed-off-by: Bob Weinand <[email protected]>
Signed-off-by: Bob Weinand <[email protected]>
Looking good, thank you @bwoebi! I did some local tests and found that it's possible for the collector to lock up indefinitely if the receiver should ever hit a condition where it doesn't drain the unix domain socket (the write will block in the syscall), but I only discovered that by accident (custom receiver implementation with a bug). Except for freezing the receiver, I can't really think of a condition which would allow this to arise in normal code. I have an incoming PR which will push the coordinator into its own forked process, allowing the original process to keep track of its time budget. That will fix the cause of these (speculative!!!) hangs. |
What does this PR do?
In the patches leading up to v14, I had tried to consolidate some of the receiver code paths. In so doing, I accidentally removed the entire ability for a collector to speak with an async receiver.
Sorry, PHP!
Rather than adding the same
ddog_crasht_init_with_unix_socket
as before, this patch adds a newddog_crasht_init_without_receiver
interface. All this does is check that the passedconfig
has a validunix_socket_path
.Motivation
Fixing past mistakes.