Ruby skip native resume#80
Draft
Gandem wants to merge 19 commits into
Draft
Conversation
* Avoid using the GITHUB_TOKEN for pr creation * fixing repo reference
Force-pushing the datadog branch makes old commit hashes unreachable, breaking go.mod replace directives that reference them by pseudo-version.
- Compare `mergeable` against the MERGEABLE enum string instead of the boolean `false`, so CONFLICTING/UNKNOWN PRs are correctly rejected. - Guard workflow dispatch to the `rebase-on-upstream` branch so an approved PR from an unrelated branch cannot force-push `datadog`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…open-telemetry#1400) (#79) Co-authored-by: Christos Kalkanis <christos.kalkanis@elastic.co>
Replace the `systemProcess.fileToMapping` map (populated as a side effect of IterateMappings, then consulted by OpenELF) with an explicit `OpenELFMapping(*RawMapping)` method on the Process interface. The mapping is now passed directly, removing the temporal coupling between the two methods and the path-keyed lookup. In processmanager.newFrameMapping, a small `mappingELFOpener` adapter routes opens of the bound mapping's path to `OpenELFMapping` while forwarding everything else (e.g. .gnu_debuglink targets) to `OpenELF`. This also fixes some issues with `fileToMapping`: - The first iteration after process creation missed the map_files route because `sp.fileToMapping` was assigned only at the end of `IterateMappings` and subsequent iterations could use a stale RawMapping (from the previous iteration). - Avoid path-keyed lookup that could pick the wrong file in some very rare cases (open-telemetry#812).
When the Ruby unwinder encounters a cfunc (C-implemented Ruby method), it normally transitions back to the native unwinder to unwind through the C code, then re-enters the Ruby unwinder when it detects the Ruby interpreter loop. Each such round-trip costs 2-3 tail calls out of a budget of 29, so a Ruby stack with 10+ cfuncs can easily truncate. This adds a skip_native_resume flag to RubyProcInfo that, when set, pushes cfunc frames inline without transitioning to the native unwinder. This trades native frames within cfuncs (e.g., the C call chain inside Array#sort) for complete Ruby-level traces without truncation. The flag is controlled by OTEL_EBPF_RUBY_SKIP_NATIVE_RESUME (default: false). Set to 'true' to enable.
- Correct inline comment that incorrectly stated 'Default to skipping' when the actual default is false (requires explicit opt-in) - Add note that skipNativeResume() is evaluated per-process at attach time
When skip_native_resume is true, the end-of-stack check should stop instead of resuming native unwinding. The native unwinder could encounter the JIT anonymous mapping and re-enter the Ruby unwinder, causing incorrect frames. Check skip_native_resume at end-of-stack to prevent resuming native unwinding when the flag is set.
Regenerates tracer.ebpf.{amd64,arm64} against the datadog branch HEAD
after cherry-picking PR open-telemetry#1335 (ruby: add skip_native_resume).
This comment has been minimized.
This comment has been minimized.
Collapse the skip_native_resume ternary onto one line to satisfy
clang-format-17, and regenerate tracer.ebpf.{amd64,arm64}.
Per Benoit Daloze: defaulting to true avoids having to plumb the env var through DOE deployments. Users can still opt out via OTEL_EBPF_RUBY_SKIP_NATIVE_RESUME=false.
This reverts commit 0c1ea86.
In walk_ruby_stack, when skip_native_resume is set and we reach the end of the Ruby VM stack, the inner for-loop set *next_unwinder to PROG_UNWIND_STOP but only `goto save_state` on PROG_UNWIND_NATIVE. The loop continued with stack_ptr unchanged, re-reading the same cfp on every iteration, and the natural for-loop exit then overwrote *next_unwinder with PROG_UNWIND_RUBY — causing the Ruby unwinder to tail-call itself repeatedly until the trace buffer overflowed with stack_length_exceeded. Reproduced locally with OTEL_EBPF_RUBY_SKIP_NATIVE_RESUME=true against the arm64 ruby-4.0.1-loop coredump: before the fix, the unwinder returned ~150 duplicated "<main>+0 in /tmp/loop.rb:30" frames and aborted. After the fix, every Ruby (ISEQ + cfunc) frame is pushed exactly once, matching the goal of avoiding the Ruby<->native tail-call round-trips without truncating the stack. The original (skip_native_resume=false) path is unchanged: all 17 Ruby arm64 coredump fixtures still pass.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
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
5 participants
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.
No description provided.