IGNORE: base branch combining #5521, #5501

.github/workflows/system-tests.yml

@@ -77,10 +77,7 @@ jobs:
   test:
     needs:
       - build
-    uses: DataDog/system-tests/.github/workflows/system-tests.yml@b38df18ddc8aa6af24e3838cfd91e4232404471b  # Automated: This reference is automatically updated.
-    secrets:
-      TEST_OPTIMIZATION_API_KEY: ${{ secrets.DD_API_KEY }}  # key used to pushed test results to test optim
-      DD_API_KEY: ${{ secrets.DD_API_KEY }}  # key used in tests runs
+    uses: DataDog/system-tests/.github/workflows/system-tests.yml@9457e21e869c18c3c7910057c461cf364f43b379  # Automated: This reference is automatically updated.
     permissions:
       contents: read
       id-token: write
@@ -91,10 +88,10 @@ jobs:
       desired_execution_time: 300  # 5 minutes
       scenarios_groups: tracer_release
       skip_empty_scenarios: true
-      ref: b38df18ddc8aa6af24e3838cfd91e4232404471b  # Automated: This reference is automatically updated.
+      ref: 9457e21e869c18c3c7910057c461cf364f43b379  # Automated: This reference is automatically updated.
       force_execute: ${{ needs.build.outputs.forced_tests }}
       parametric_job_count: 8
-      push_to_test_optimization: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
+      push_to_test_optimization: false  # disabled while API key usage is transitioning to system-tests
 
   complete:
     name: System Tests (complete)

docs/DynamicInstrumentationDevelopment.md

@@ -1,5 +1,104 @@
 # Dynamic Instrumentation Development Guide
 
+## Iseq Lifecycle and GC
+
+### Background
+
+DI line probes work by enabling a `TracePoint` targeted at a specific
+`RubyVM::InstructionSequence` (iseq) and line number. The iseq must cover
+the target line — a whole-file iseq covers all lines, while a per-method
+iseq covers only the method body.
+
+`CodeTracker` maintains a registry mapping file paths to iseqs. The
+`:script_compiled` tracepoint populates this at load time. The
+`backfill_registry` method recovers iseqs for files loaded before tracking
+started by walking object space via `DI.all_iseqs` (C extension).
+
+### Iseq types created when Ruby loads a file
+
+When Ruby loads a file via `require`/`require_relative`, it creates several
+iseq objects:
+
+| Type | Description | `first_lineno` | Created by |
+|------|-------------|-----------------|------------|
+| `:top` | Whole-file iseq, covers all lines | 0 | `rb_iseq_new_top` (require/load) |
+| `:main` | Entry script only (`ruby script.rb`) | 0 | `rb_iseq_new_main` |
+| `:class` | One per class/module body | >= 1 | class/module keyword |
+| `:method` | One per method definition | >= 1 | def keyword |
+| `:rescue`/`:ensure` | Rescue/ensure blocks | >= 1 | rescue/ensure keyword |
+
+`DI.iseq_type` (wraps `rb_iseq_type`, Ruby 3.1+) returns the type as a
+Symbol. On Ruby < 3.1, `first_lineno == 0` identifies whole-file iseqs.
+
+### What survives GC
+
+After file loading completes, iseq objects are subject to garbage collection
+like any other Ruby object. What keeps them alive:
+
+| Type | Survives GC? | Reference holder |
+|------|-------------|------------------|
+| `:method` | **Yes** | Method objects on the class (`UnboundMethod` → iseq) |
+| `:class` | **Unreliable** | May survive via class constant, not guaranteed |
+| `:top` | **No** | Nothing. `$LOADED_FEATURES` stores path strings, not iseqs. |
+| `:rescue`/`:ensure` | **No** | Follow their parent iseq |
+
+The `:top` iseq executes once (defining classes/methods/constants) and is
+then unreferenced. GC can collect it at any time.
+
+In practice:
+- With no GC pressure, iseqs survive indefinitely (not yet collected)
+- With allocation pressure or explicit `GC.start`, `:top` is collected
+- After aggressive GC, typically only `:method` iseqs survive
+
+### Implications for backfill
+
+`backfill_registry` only stores `:top` or `:main` iseqs because they cover
+all lines in the file. Per-method iseqs are filtered out — they cover only
+a subset of lines.
+
+If the `:top` iseq was collected before backfill runs, no whole-file iseq
+exists for that file. This causes `DITargetNotInRegistry` when installing
+a line probe.
+
+**Production:** backfill is best-effort. If the `:top` iseq was already
+collected, line probes on that pre-loaded file won't work via backfill.
+The `:script_compiled` tracepoint (the primary mechanism) is unaffected —
+it captures iseqs at load time before GC can touch them.
+
+### Test pattern: keeping iseqs alive for backfill tests
+
+Tests that load files before code tracking and then test backfill must
+prevent the `:top` iseq from being collected. `GC.disable` alone is
+insufficient across multiple tests — after `deactivate_tracking!` clears
+the registry (the only reference), GC can collect the iseq before the
+next test's backfill.
+
+The correct pattern is to hold a reference in a constant:
+
+```ruby
+# At file load time (before RSpec.describe):
+
+# 1. Disable GC so the :top iseq survives long enough to be captured
+GC.disable
+require_relative "test_class"
+
+# 2. Immediately capture the :top iseq in a constant
+TEST_TOP_ISEQ = Datadog::DI.file_iseqs.find { |i|
+  i.absolute_path&.end_with?("test_class.rb") &&
+    (Datadog::DI.respond_to?(:iseq_type) ? Datadog::DI.iseq_type(i) == :top : i.first_lineno == 0)
+}
+
+# 3. Safe to re-enable GC — the constant holds the reference
+GC.enable
+```
+
+The constant MUST be assigned while GC is still disabled. If GC runs
+between `require_relative` and the assignment, the iseq may already be
+collected.
+
+See `spec/datadog/di/ext/backfill_integration_spec.rb` for a working
+example.
+
 ## Starting the Remote Configuration Worker Manually
 
 Add this to your Rails initializer after `Datadog.configure`:

ext/libdatadog_api/di.c

@@ -3,7 +3,12 @@
 #include "datadog_ruby_common.h"
 
 // Prototypes for Ruby functions declared in internal Ruby headers.
+// rb_iseqw_new wraps an internal iseq pointer into a Ruby-visible
+// RubyVM::InstructionSequence object.
 VALUE rb_iseqw_new(const void *iseq);
+// rb_iseqw_to_iseq unwraps a RubyVM::InstructionSequence object back
+// to its internal iseq pointer.
+const void *rb_iseqw_to_iseq(VALUE iseqw);
 int rb_objspace_internal_object_p(VALUE obj);
 void rb_objspace_each_objects(
     int (*callback)(void *start, void *end, size_t stride, void *data),
@@ -70,10 +75,53 @@ static VALUE exception_message(DDTRACE_UNUSED VALUE _self, VALUE exception) {
   return rb_ivar_get(exception, id_mesg);
 }
 
+// rb_iseq_type was added in Ruby 3.1 (commit 89a02d89 by Koichi Sasada,
+// 2021-12-19). It returns the iseq type as a Symbol. On Ruby < 3.1 this
+// function does not exist, so have_func('rb_iseq_type') in extconf.rb
+// gates compilation. When unavailable, backfill_registry falls back to
+// the first_lineno == 0 heuristic.
+#ifdef HAVE_RB_ISEQ_TYPE
+VALUE rb_iseq_type(const void *iseq);
+
+/*
+ * call-seq:
+ *   DI.iseq_type(iseq) -> Symbol
+ *
+ * Returns the type of an InstructionSequence as a symbol by calling
+ * the internal rb_iseq_type() function (available since Ruby 3.1).
+ *
+ * This method is only defined when rb_iseq_type is detected at compile
+ * time via have_func in extconf.rb. On Ruby < 3.1 it is not available
+ * and callers must use an alternative (e.g. first_lineno heuristic).
+ *
+ * Possible return values: :top, :method, :block, :class, :rescue,
+ * :ensure, :eval, :main, :plain.
+ *
+ * :top and :main represent whole-file iseqs (from require/load and the
+ * entry point script respectively). Other types represent sub-file
+ * constructs (method definitions, class bodies, blocks, etc.).
+ *
+ * Used by CodeTracker#backfill_registry to distinguish whole-file iseqs
+ * from per-method/block/class iseqs when populating the registry from
+ * the object space.
+ *
+ * @param iseq [RubyVM::InstructionSequence] The instruction sequence
+ * @return [Symbol] The iseq type
+ */
+static VALUE iseq_type(DDTRACE_UNUSED VALUE _self, VALUE iseq_val) {
+  const void *iseq = rb_iseqw_to_iseq(iseq_val);
+  if (!iseq) return Qnil;
+  return rb_iseq_type(iseq);
+}
+#endif
+
 void di_init(VALUE datadog_module) {
   id_mesg = rb_intern("mesg");
 
   VALUE di_module = rb_define_module_under(datadog_module, "DI");
   rb_define_singleton_method(di_module, "all_iseqs", all_iseqs, 0);
   rb_define_singleton_method(di_module, "exception_message", exception_message, 1);
+#ifdef HAVE_RB_ISEQ_TYPE
+  rb_define_singleton_method(di_module, "iseq_type", iseq_type, 1);
+#endif
 }

ext/libdatadog_api/extconf.rb

@@ -89,6 +89,8 @@ def skip_building_extension!(reason)
 # When requiring, we need to use the exact same string, including the version and the platform.
 EXTENSION_NAME = "libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}".freeze
 
+have_func('rb_iseq_type')
+
 create_makefile(EXTENSION_NAME)
 
 # rubocop:enable Style/GlobalVars

lib/datadog/di.rb

@@ -11,6 +11,71 @@ module Datadog
   module DI
     INSTRUMENTED_COUNTERS_LOCK = Mutex.new
 
+    # Captured at load time from Exception itself (not a subclass).
+    # Used to bypass subclass overrides of backtrace_locations.
+    #
+    # This does NOT protect against monkeypatching Exception#backtrace_locations
+    # before dd-trace-rb loads — in that case we'd capture the monkeypatch.
+    # The practical threat model is customer subclasses overriding the method:
+    #
+    #   class MyError < StandardError
+    #     def backtrace_locations; []; end
+    #   end
+    #
+    # The UnboundMethod bypasses subclass overrides: bind(exception).call
+    # always dispatches to the original Exception implementation.
+    #
+    # Note: if the subclass overrides #backtrace (not #backtrace_locations),
+    # MRI's setup_exception skips storing the VM backtrace entirely — both
+    # @bt and @bt_locations stay nil. In that case this UnboundMethod also
+    # returns nil. See EXCEPTION_BACKTRACE comment and
+    # docs/ruby/exception-backtrace-internals.md in claude-projects for the
+    # full MRI analysis.
+    EXCEPTION_BACKTRACE_LOCATIONS = Exception.instance_method(:backtrace_locations)
+
+    # Same UnboundMethod trick for Exception#backtrace (Array<String>).
+    # Used as a fallback when backtrace_locations returns nil — which happens
+    # when someone calls Exception#set_backtrace with an Array<String>.
+    #
+    # set_backtrace accepts Array<String> or nil. When called with strings,
+    # it replaces the VM-level backtrace: backtrace returns the new strings,
+    # but backtrace_locations returns nil because the VM cannot reconstruct
+    # Location objects from formatted strings. This occurs in exception
+    # wrapping patterns where a library catches an exception, creates a new
+    # one, and copies the original's string backtrace onto it via
+    # set_backtrace before re-raising.
+    #
+    # Ruby 3.4+ also allows set_backtrace(Array<Location>), which preserves
+    # backtrace_locations — but older Rubies and most existing code use
+    # the string form.
+    #
+    # LIMITATION: Unlike EXCEPTION_BACKTRACE_LOCATIONS, this UnboundMethod
+    # does NOT bypass subclass overrides of #backtrace. When a subclass
+    # overrides #backtrace, MRI's setup_exception (eval.c) calls the
+    # override via rb_get_backtrace, gets a non-nil result, and skips
+    # storing the real VM backtrace in @bt and @bt_locations entirely.
+    # The C function exc_backtrace then reads @bt (still nil from
+    # exc_init) and returns nil.
+    #
+    # By contrast, setup_exception only checks for #backtrace overrides,
+    # not #backtrace_locations overrides. So when only backtrace_locations
+    # is overridden, the real backtrace IS stored, and the UnboundMethod
+    # for backtrace_locations reads it directly from @bt_locations.
+    #
+    # This limitation is acceptable because this constant is only used as
+    # a fallback when backtrace_locations returns nil. In the common
+    # set_backtrace-with-strings case, no subclass override is involved
+    # and the fallback works. If a subclass does override #backtrace AND
+    # set_backtrace was called, set_backtrace writes to @bt via C
+    # regardless of overrides, so the fallback still works.
+    #
+    # The only unrecoverable case: a subclass overrides #backtrace, the
+    # exception is raised normally, and set_backtrace is never called.
+    # Both @bt and @bt_locations are nil — the real backtrace was never
+    # stored by raise. DI reports an empty stacktrace (type and message
+    # are still reported).
+    EXCEPTION_BACKTRACE = Exception.instance_method(:backtrace)
+
     class << self
       def enabled?
         Datadog.configuration.dynamic_instrumentation.enabled

lib/datadog/di/base.rb

@@ -17,6 +17,11 @@ module Datadog
   module DI
     LOCK = Mutex.new
 
+    # Initialize to avoid "instance variable not initialized" warning
+    # on Ruby 2.6/2.7 when current_component is called before any
+    # component is added (e.g. from backfill_registry's error boundary).
+    @current_components = nil
+
     class << self
       attr_reader :code_tracker
 
@@ -101,7 +106,7 @@ def current_component
       def add_current_component(component)
         LOCK.synchronize do
           @current_components ||= []
-          @current_components << component
+          @current_components << component # steep:ignore NoMethod
         end
       end