-
Notifications
You must be signed in to change notification settings - Fork 10
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
Crashtracker receiver is spawned on crash #692
Conversation
BenchmarksComparisonBenchmark execution time: 2024-10-31 20:22:14 Comparing candidate commit 2bfef1b in PR branch Found 1 performance improvements and 1 performance regressions! Performance is the same for 49 metrics, 2 unstable metrics. scenario:benching deserializing traces from msgpack to their internal representation
scenario:tags/replace_trace_tags
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. |
…datadog into sanchda/crashtracking_safety
How does this interact with the PHP sidecar? |
It should not. In a future PR I think many of us hope to harmonize the two implementations, but there are a few questions about how packaging would work in that universe. Right now I think the priority is to fix the outstanding defects, provide a sane assessment of risk for the runtimes which will rely on this implementation of crashtracking, and then harmonize over a more relaxed timeline. |
Curiously, I see here that vfork availability isn't what I thought it was on macos. Will check that out after I finish writing some documents and tests. |
…datadog into sanchda/crashtracking_safety
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #692 +/- ##
==========================================
- Coverage 71.73% 70.56% -1.18%
==========================================
Files 271 281 +10
Lines 41143 41838 +695
==========================================
+ Hits 29514 29523 +9
- Misses 11629 12315 +686
|
…datadog into sanchda/crashtracking_safety
👋 adding @bwoebi and @kevingosse as subject matter experts. I'd really appreciate a review of the systems interactions in |
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.
nice
Co-authored-by: Daniel Schwartz-Narbonne <[email protected]>
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.
The strategy looks sound to me.
What does this PR do?
Changes the way the crashtracker receiver works
This PR isn't yet complete, as it requires more tests and an RFC. But please feel free to comment on what you see!
Motivation
Some runtimes actually call
waitpid(-1,...
anticipating an ECHILD, which is incompatible with the idea of a long-lived receiver process. We also wanted to improve some of the safety around how signals work.Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.
APMLP-286