feat(profiling): report the foreign SIGSEGV/SIGBUS owner to logs and telemetry - #20589
Draft
vlad-scherbich wants to merge 1 commit into
Draft
vlad-scherbich wants to merge 1 commit into
vlad-scherbich wants to merge 1 commit into
Conversation
…telemetry The owner string the sampler builds only reached stderr, which a customer running in a container usually never sees. A takeover pins the process to the slower syscall-based memory copy for the rest of its life, or stops stack sampling outright when no safe fallback exists, and neither outcome was visible anywhere the profiling team can query. Latch the notice on the sampler, drain it once per process from StackCollector.snapshot(), and emit it as a LOG.warning (LOG.error when sampling stopped) plus a telemetry log. The sampling thread cannot touch Python, hence the latch and the take-once drain rather than logging in place. The telemetry tags carry the normalized owner, whether the handler was already foreign when warmup ended, and lost_signals - sigsegv, sigbus or sigsegv_sigbus. Separating the two signals is the point of that last tag: whether SIGBUS ever changes hands in the field decides whether the handler paths only a SIGBUS can reach are worth keeping, and nothing measures that today.
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
Circular import analysis
|
Contributor
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 6e1ea90 | Docs | View more details | Give us feedback! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What gap this addresses
The owner string #20588 builds only reaches stderr, which nobody running in a container
sees. A takeover pins the process to the slower syscall memory copy for its whole life,
or stops stack sampling outright, and neither outcome is visible anywhere we can query.
Evidence it is real
We concluded the sticky cycle guard in
danger.ccis near-unreachable because re-entryneeds a subsequent
SIGBUSand we had seen zero — but nobody could confirm that,because nothing reports which signal was lost. The
lost_signalstag is what settlesit, and it decides whether the SIGBUS-only handler paths are worth keeping.
What it does
The sampling thread latches the notice on
Samplerinstead of logging in place (itcannot touch Python).
StackCollector.snapshot()drains it once per process and emitsa
LOG.warning—LOG.errorwhen sampling stopped — plus a telemetry log tagged withthe normalized owner, whether the handler was already foreign at end of warmup, and
lost_signals(sigsegv,sigbus, orsigsegv_sigbus). Owner strings are normalizedto a bounded set so the tag cannot carry an unbounded address, and the value is kept
comma-free because
add_logjoins tags on commas.What it does NOT do
telemetry_writer.add_log.