Skip to content

DI: backfill CodeTracker registry with iseqs for pre-loaded files#5496

Draft
p-datadog wants to merge 20 commits intomasterfrom
di-iseq-backfill
Draft

DI: backfill CodeTracker registry with iseqs for pre-loaded files#5496
p-datadog wants to merge 20 commits intomasterfrom
di-iseq-backfill

Conversation

@p-datadog
Copy link
Copy Markdown
Member

@p-datadog p-datadog commented Mar 23, 2026

What does this PR do?

When CodeTracker starts, backfills the iseq registry with instruction sequences for files that were loaded before tracking began. Also adds a DI.iseq_type C extension (Ruby 3.1+) for precise whole-file iseq detection.

Motivation:

CodeTracker only captures files loaded after DI starts via :script_compiled TracePoint events. Most application code and all gems are loaded at boot time before DI activates, making them invisible to line probes. This change walks the Ruby object space via the all_iseqs C extension (#5111) and recovers whole-file iseqs for already-loaded code.

Implementation:

  • CodeTracker#backfill_registry walks the object space via DI.file_iseqs, filters to whole-file iseqs, and stores them in the registry without overwriting entries from :script_compiled.
  • Whole-file iseq detection uses DI.iseq_type on Ruby 3.1+ (returns :top for require/load, :main for the entry script). On Ruby < 3.1, falls back to first_lineno == 0 — which produces the same result because Ruby's rb_iseq_new_top and rb_iseq_new_main pass INT2FIX(0) while method/class/block definitions get their source line (>= 1).
  • DI.iseq_type wraps the internal rb_iseq_type() function added in Ruby 3.1 (commit 89a02d89 by Koichi Sasada). Guarded behind have_func('rb_iseq_type') in extconf.rb — only compiled when the symbol exists.
  • Backfill runs inside start after the trace point is enabled, so files loaded concurrently are captured by the trace point (backfill won't overwrite them).

Change log entry

Yes. DI/LD: more third-party libraries are now instrumentable in DI/LD.

Additional Notes:

How to test the change?

Unit tests for backfill_registry (filtering, overwrite protection, error boundary, fallback path). Integration test loading a file before tracking starts, then verifying probe installation and snapshot capture work via backfill.

Manually tested in gobo by inspecting the DI code tracer registry. Before:

2026-03-24_19-29-01

After:

2026-03-24_19-29-24 2026-03-24_19-29-33

Manual testing via gobo targeting stdlib:

2026-03-24_20-28-16 2026-03-24_20-28-25

@p-datadog p-datadog added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Mar 23, 2026
@github-actions github-actions bot added the profiling Involves Datadog profiling label Mar 23, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 23, 2026

Typing analysis

Note: Ignored files are excluded from the next sections.

steep:ignore comments

This PR introduces 1 steep:ignore comment.

steep:ignore comments (+1-0)Introduced:
lib/datadog/di/base.rb:109

Untyped methods

This PR introduces 1 partially typed method, and clears 1 partially typed method. It increases the percentage of typed methods from 61.66% to 61.7% (+0.04%).

Partially typed methods (+1-1)Introduced:
sig/datadog/di/code_tracker.rbs:15
└── def iseqs_for_path_suffix: (String suffix) -> untyped
Cleared:
sig/datadog/di/code_tracker.rbs:14
└── def iseqs_for_path_suffix: (String suffix) -> untyped

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept on the line before the definition to remove it from the stats.

@datadog-prod-us1-6
Copy link
Copy Markdown

datadog-prod-us1-6 bot commented Mar 24, 2026

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
Patch Coverage: 99.27%
Overall Coverage: 95.36% (+0.01%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: a058463 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@pr-commenter
Copy link
Copy Markdown

pr-commenter bot commented Mar 24, 2026

Benchmarks

Benchmark execution time: 2026-03-28 03:54:55

Comparing candidate commit a058463 in PR branch di-iseq-backfill with baseline commit 609f934 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 46 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

p-ddsign and others added 6 commits March 27, 2026 14:13
When CodeTracker starts, use the all_iseqs C extension to populate the
registry with instruction sequences for files that were loaded before
tracking began. This enables line probes on third-party code and
application code loaded at boot time.

Only whole-file iseqs (first_lineno == 0) are backfilled — per-method
iseqs require instrumenter changes to select the correct iseq for a
target line and will be supported in a follow-up.

Backfill does not overwrite entries from :script_compiled, which are
authoritative. The C extension availability is checked via
DI.respond_to?(:all_iseqs) so the code gracefully degrades when
the extension is not compiled.

- Added CodeTracker#backfill_registry
- Called from CodeTracker#start after trace point is enabled
- Added RBS signature
- Added tests for backfill behavior and C extension fallback

Co-Authored-By: Claude <noreply@anthropic.com>
- Added rescue block around backfill_registry so failures are
  best-effort (logged + telemetry) rather than propagating
- Replaced all skip-based tests with mock-based tests that exercise
  backfill logic without requiring the compiled C extension
- Added tests for: mixed iseq types, multiple files, error handling,
  suffix/exact lookup on backfilled entries, start ordering
- 27 examples, 0 failures, 0 pending, 0 skipped

Co-Authored-By: Claude <noreply@anthropic.com>
Tests the end-to-end flow: test class loaded before code tracking
starts → CodeTracker#start triggers backfill via all_iseqs C extension
→ iseq recovered from object space → line probe installed on
backfilled iseq → probe fires and captures local variables.

Runs under rake spec:di_with_ext (requires compiled C extension).

Three test cases:
- Probe installs successfully on backfilled iseq
- Probe fires when target line executes
- Snapshot captures local variables from backfilled iseq

Co-Authored-By: Claude <noreply@anthropic.com>
On macOS CI the C extension is compiled, so backfill_registry populates
the CodeTracker registry with pre-loaded files during start. This broke
existing tests that expect the registry to be empty after start or to
contain exactly N explicitly-loaded files.

Fix by stubbing backfill_registry in test contexts that exercise
:script_compiled behavior. Backfill is tested separately in its own
describe blocks.

Affected contexts:
- CodeTracker #start (before block)
- CodeTracker shared context 'when code tracker is running'
- CodeTracker #iseqs_for_path_suffix (around block)
- Instrumenter shared context 'with code tracking'

Co-Authored-By: Claude <noreply@anthropic.com>
…kfill

The backfill filter used first_lineno == 0 to identify whole-file iseqs,
but most whole-file iseqs from all_iseqs have first_lineno == 1. The new
DI.iseq_type method reads the iseq type directly from the Ruby VM struct
and returns a symbol (:top, :method, :block, :class, etc.).

The backfill now filters by type == :top || type == :main, which
correctly identifies whole-file iseqs regardless of first_lineno.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rb_iseq_type is an internal Ruby function that only exists in Ruby 3.1+.
On Ruby 2.7 and 3.0, referencing it causes an undefined symbol error at
load time, crashing the entire C extension (including all_iseqs and
exception_message which work fine on those versions).

Use have_func in extconf.rb to detect rb_iseq_type at compile time, and
wrap the iseq_type function + registration in #ifdef HAVE_RB_ISEQ_TYPE.
The Ruby code in code_tracker.rb already handles the missing method via
DI.respond_to?(:iseq_type) with a first_lineno fallback.

Co-Authored-By: Claude <noreply@anthropic.com>
p-ddsign and others added 13 commits March 27, 2026 14:22
… require

- Add doc comments for rb_iseqw_new and rb_iseqw_to_iseq prototypes in di.c
  (internal Ruby functions used without documentation)
- Add error handling test coverage for backfill_registry: verify logger.debug
  is called with the error message and telemetry.report is called when
  DI.current_component is available
- Add test coverage for the first_lineno == 0 fallback path when iseq_type
  is unavailable (Ruby versions without rb_iseq_type)
- Add missing require "datadog/di/spec_helper" to iseq_type_spec.rb for
  consistency with other ext specs
- Fix skip message: iseq_type availability depends on rb_iseq_type in the
  Ruby runtime, not on the DI C extension

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- di.c: Document that rb_iseq_type was added in Ruby 3.1, explain the
  HAVE_RB_ISEQ_TYPE compile-time guard, and note the fallback path
- code_tracker.rb: Replace "first_lineno == 0" YARD doc with full
  description of both strategies (iseq_type on 3.1+, first_lineno
  heuristic on older Rubies) and their tradeoffs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The YARD doc claimed the first_lineno == 0 fallback "can match top-level
eval iseqs" but this is wrong. InstructionSequence.compile passes
first_lineno = 1 (not 0), and require/load passes INT2FIX(0) in Ruby's
rb_iseq_new_top/rb_iseq_new_main. Both strategies produce the same
result in practice.

Verified by reading Ruby 3.0 source (iseq.c lines 813-822):
  rb_iseq_new_with_opt(ast, name, path, realpath, INT2FIX(0), ...)
  → ISEQ_TYPE_TOP with first_lineno = 0

And compile path (iseq.c line 1064):
  rb_iseq_new_with_opt(&ast->body, label, file, realpath, line, ...)
  → line defaults to INT2FIX(1) for compile/eval

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify idempotency: calling backfill_registry a second time with the
same iseqs doesn't duplicate entries (registry.key? guard).

Also verify that a second call with new iseqs adds them without
overwriting entries from the first call.

Co-Authored-By: Claude <noreply@anthropic.com>
The guard was purely defensive — the C extension is always compiled
when DI is active (enforced by environment_supported? in component.rb).
The rescue block at the bottom of backfill_registry already catches any
exception if file_iseqs fails, making the guard redundant.

Co-Authored-By: Claude <noreply@anthropic.com>
The method is called for side effects only. Without the explicit nil,
the happy path leaked the synchronize return value and the rescue path
leaked the telemetry report return value.

Co-Authored-By: Claude <noreply@anthropic.com>
On older Rubies, accessing an uninitialized instance variable via &.
produces a warning: "instance variable @current_components not
initialized". This triggers loading_spec failures because
datadog/di/preload produces unexpected output.

The variable is accessed by DI.current_component (called from
backfill_registry's error boundary) before any component is added.
Initializing to nil at module level suppresses the warning while
preserving the existing lazy-init behavior in add_current_component.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RSpec's verify_partial_doubles rejects allow(DI).to receive(:iseq_type)
when the method doesn't exist on the module. On Ruby < 3.1,
rb_iseq_type is not available so DI.iseq_type is never defined.

Fix: conditionally stub iseq_type only when it exists. On older Rubies,
let respond_to?(:iseq_type) return false naturally and exercise the
first_lineno == 0 fallback path — which is what production does.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The pre-loaded test class's iseq can be garbage collected before
backfill walks the object space, causing DITargetNotInRegistry. In
production, application code is referenced by live constants/methods
and survives GC. In the test, the iseq is more ephemeral.

Disable GC around activate_tracking! (which calls backfill_registry)
to ensure the iseq is still in the object space when all_iseqs runs.
Re-enable immediately after.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two root causes:

1. code_tracker_spec.rb: iseq_type was stubbed with and_call_original,
   but the C function expects a real RubyVM::InstructionSequence, not a
   test double. Stub returns :top for first_lineno==0, :method otherwise.

2. backfill_integration_spec.rb: The top-level file iseq (first_lineno=0,
   type=:top) is not referenced by any constant or method after loading.
   GC could collect it between require_relative (file load time) and the
   before block's backfill_registry call. Move GC.disable to file level,
   immediately before require_relative, so the iseq survives until
   backfill walks the object space.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The ivar is initialized to nil to avoid Ruby 2.6/2.7 warnings.
RBS type needs to reflect this. Silence false positive on <<
after ||= (Steep doesn't track that ||= guarantees non-nil).

Co-Authored-By: Claude <noreply@anthropic.com>
Root cause: the top-level (:top) iseq for the test class file has no
references after loading completes — only class/method child iseqs
survive via BackfillIntegrationTestClass. The previous approach disabled
GC at file load time and re-enabled it in the before block after
backfill. This protected the first test, but after deactivate_tracking!
cleared the registry (the only reference to the iseq), GC could collect
it before the next test's backfill_registry walked object space.

Fix: capture the top-level iseq in a constant (BACKFILL_TEST_TOP_ISEQ)
immediately after loading, before GC can collect it. The constant keeps
the iseq alive for the lifetime of the process, so backfill_registry
can find it in every test regardless of GC activity.

Verified: 0 failures across 8 consecutive full DI suite runs (714
examples each), vs ~20% failure rate before the fix.

Co-Authored-By: Claude <noreply@anthropic.com>
Add "Iseq Lifecycle and GC" section to DynamicInstrumentationDevelopment.md
covering: iseq types created on file load, which survive GC and why,
implications for backfill_registry, and the correct test pattern for
keeping top-level iseqs alive across tests.

Add cross-reference from code_tracker.rb backfill_registry docstring.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos profiling Involves Datadog profiling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants