DI: document pre-loaded file line probe behavior#5637
Draft
p-datadog wants to merge 20 commits into
Draft
Conversation
Infrastructure for the Bootsnap integration test: - Add :bootsnap to DI contrib list in Rakefile - Add gem 'bootsnap' to rails8-mysql2 appraisals (ruby 3.2-4.0) - Add di:bootsnap Matrixfile entry (rails8-mysql2, ruby 3.2+) Bootsnap is a standard Rails companion gem. Adding it to the rails8 appraisal does not affect other tests — it's an extra gem in the bundle that's only exercised by spec/datadog/di/contrib/bootsnap/. Co-Authored-By: Claude <noreply@anthropic.com>
When a file's whole-file (:top) iseq has been garbage collected, per-method iseqs from all_iseqs can still be used to target line probes. This covers 86% of files that were previously untargetable. Changes: - backfill_registry stores per-method iseqs in per_method_registry (grouped by path) instead of discarding them - New iseq_for_line(suffix, line) method tries whole-file iseq first, then searches per-method iseqs for one whose trace_points include the target line - Instrumenter uses iseq_for_line when available, falls back to iseqs_for_path_suffix for compatibility Verified: 37 code_tracker tests pass, lint clean, types clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Loads a test class, GCs the top iseq, then verifies that the backfill finds the surviving method iseq and a line probe can be installed, fired, and captures local variables through it. Precondition checks skip the test if GC didn't collect the top iseq or if the C extension is unavailable. Verified: 3 integration tests pass (install, fire, capture locals). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The throwable now includes a stacktrace array (from the C extension commit). Also update error message assertion for the new raise_if_probe_in_loaded_features format. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The raise_if_probe_in_loaded_features now reports whether per-method iseqs exist or not, instead of the generic "not in code tracker registry" message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Distinguish between "has per-method iseqs but none cover this line" and "has no surviving iseqs at all". Include the target line number in the error. Helps users understand why a line probe failed and whether the file is partially targetable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The method gained line_no and code_tracker parameters in the per-method iseq support commit, but the RBS declaration was not updated. Co-Authored-By: Claude <noreply@anthropic.com>
On Ruby < 3.1, DI.iseq_type is not defined (the C extension gates it with have_func for rb_iseq_type). RSpec's partial double verification rejects stubs for undefined methods, causing 10 failures in the iseq_for_line and per-method iseq tests. Fix: guard iseq_type stubs with `if Datadog::DI.respond_to?(:iseq_type)`, matching the existing pattern in backfill_registry tests (lines 246-255). On Ruby < 3.1, the code under test falls back to first_lineno == 0. Co-Authored-By: Claude <noreply@anthropic.com>
The all_iseqs C extension is compiled by the di_with_ext CI task. Its absence means a broken build, not an unsupported platform. Tests should fail loudly if the extension is missing, not skip silently. The iseq_type skip (Ruby < 3.1) is retained — that is a genuine platform limitation. Co-Authored-By: Claude <noreply@anthropic.com>
Verifies the full DI stack works when compile_file was called on a file but the compile_file iseqs were GC'd (no reference held). The per-method iseq fallback installs and fires the probe correctly. Documents a known limitation: if compile_file child iseqs survive in object space (reference held), they are indistinguishable from require-produced children and backfill may register the wrong one, causing probes to silently not fire. Co-Authored-By: Claude <noreply@anthropic.com>
The backfill_registry docstring said "per-method iseqs require instrumenter changes and will be supported in a follow-up." This PR is that follow-up — the comment is now stale. Co-Authored-By: Claude <noreply@anthropic.com>
…ent section The section said "Code loaded before the tracer initializes cannot be instrumented with line probes." After #5496 (whole-file iseq backfill) and this PR (per-method iseq fallback), that limitation no longer exists. Remove the section entirely rather than rewrite it. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Address review comment: compile_file-produced :top iseqs with first_lineno != 0 fell through the whole_file check into per_method_registry. Probes targeting these iseqs fail because targeted TracePoints are bound to the specific iseq object. - Fixed in lib/datadog/di/code_tracker.rb:90 (type guard in else branch) - Added test in spec/datadog/di/code_tracker_spec.rb Co-Authored-By: Claude <noreply@anthropic.com>
Address review comment: iseq_for_line matched any trace point event at the target line, including :call. Since the instrumenter subscribes to [:line, :return, :b_return], a :call-only line would cause TracePoint#enable to raise. Now filters to executable event types so the probe goes to pending state instead of erroring. - Fixed in lib/datadog/di/code_tracker.rb:275 (event type filter) - Added tests in spec/datadog/di/code_tracker_spec.rb Co-Authored-By: Claude <noreply@anthropic.com>
The second entry (with jruby) overwrote the first. Keep only the entry without jruby, matching the reviewer's suggestion and the format of adjacent entries. Co-Authored-By: Claude <noreply@anthropic.com>
- Comment 1 (compile_file guard): replace MRI jargon and "known limitation" with concrete user impact (probe silently produces no snapshots) and practical likelihood on Ruby < 3.1 - Comment 2 (event filter): clarify that the `def` line example refers to the defined method's own iseq, not the enclosing scope Co-Authored-By: Claude <noreply@anthropic.com>
State the consequence explicitly: probe installs but silently never fires. Replace vague pronoun and jargon with concrete referents. Co-Authored-By: Claude <noreply@anthropic.com>
RubyVM::InstructionSequence is needed to find the API docs. "Compiles source to bytecode without executing it" explains what they do without assuming MRI internals knowledge. Co-Authored-By: Claude <noreply@anthropic.com>
User doc: add "Pre-Loaded Code" section explaining what works (line probes inside methods), what doesn't (top-level code, setup-only files), troubleshooting (move probe inside a method body), and Ruby version note. Dev doc: rewrite "Implications for backfill" to cover the design rationale for per-method fallback (why it exists, what problem it solves, measured impact), the two-registry lookup, event type filtering, and compile_file iseq filtering with Ruby version differences and accepted limitations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 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.
What does this PR do?
Documents how line probes work on pre-loaded files (gems, stdlib, boot code) now that per-method iseq fallback exists.
Motivation:
PR #5501 removed the "Code Tracking Requirement" doc section that said pre-loaded files can't be instrumented with line probes. That's no longer true, but nothing replaced it with accurate documentation of what works, what doesn't, and what to do when a probe doesn't fire.
Change log entry
No. Documentation only.
Additional Notes:
DynamicInstrumentation.md): new "Pre-Loaded Code" section written for app developers — no VM internals, just behavior and troubleshootingDynamicInstrumentationDevelopment.md): rewrites "Implications for backfill" for DI developers — design rationale, measured impact, two-registry lookup, event type filtering, compile_file filtering by Ruby version, accepted limitationsHow to test the change?
Documentation only — read for accuracy and audience fit.