diff --git a/lib/datadog/di.rb b/lib/datadog/di.rb index fbe144998aa..78409a75744 100644 --- a/lib/datadog/di.rb +++ b/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). + # Used as a fallback when backtrace_locations returns nil — which happens + # when someone calls Exception#set_backtrace with an Array. + # + # set_backtrace accepts Array 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), 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 diff --git a/lib/datadog/di/probe_notification_builder.rb b/lib/datadog/di/probe_notification_builder.rb index ad6736f43c9..39758153edf 100644 --- a/lib/datadog/di/probe_notification_builder.rb +++ b/lib/datadog/di/probe_notification_builder.rb @@ -56,10 +56,6 @@ def build_executed(context) NANOSECONDS = 1_000_000_000 MILLISECONDS = 1000 - # Matches Ruby backtrace frame format: "/path/file.rb:42:in `method_name'" - # Captures: $1 = file path, $2 = line number, $3 = method name - BACKTRACE_FRAME_PATTERN = /\A(.+):(\d+):in\s+[`'](.+)'\z/ - def build_snapshot(context) probe = context.probe @@ -193,21 +189,66 @@ def serialize_throwable(exception) # The exception class is already reported via the :type field. '' end + # Prefer backtrace_locations (structured Location objects) over + # backtrace (formatted strings that need regex parsing). + # + # However, backtrace_locations returns nil when someone has called + # Exception#set_backtrace with Array — the VM cannot + # reconstruct Location objects from formatted strings. This happens + # in exception wrapping patterns (catch, create new exception, copy + # original's string backtrace via set_backtrace, re-raise). + # In that case, fall back to backtrace strings. + # + # Both accessors use the UnboundMethod trick to bypass subclass + # overrides, consistent with the rest of this method. + # + # If a subclass overrides #backtrace, MRI's raise never stores + # the real backtrace — both paths return nil and stacktrace is []. + # This is unrecoverable without calling customer code. + # See DI::EXCEPTION_BACKTRACE comment for details. + locations = DI::EXCEPTION_BACKTRACE_LOCATIONS.bind(exception).call + stacktrace = if locations + format_backtrace_locations(locations) + else + format_backtrace_strings(DI::EXCEPTION_BACKTRACE.bind(exception).call) + end { type: exception.class.name, message: message, - stacktrace: format_backtrace(exception.backtrace), + stacktrace: stacktrace, } end + # Matches Ruby backtrace frame format: "/path/file.rb:42:in `method_name'" + # Captures: $1 = file path, $2 = line number, $3 = method name + BACKTRACE_FRAME_PATTERN = /\A(.+):(\d+):in\s+[`'](.+)'\z/ + + # Converts backtrace locations into the stack frame format + # expected by the Datadog UI. + # + # Uses Thread::Backtrace::Location objects which provide structured + # path/lineno/label directly, avoiding the round-trip of formatting + # to strings and regex-parsing back. + # + # @param locations [Array] + # @return [Array] + def format_backtrace_locations(locations) + locations.map do |loc| + {fileName: loc.path, function: loc.label, lineNumber: loc.lineno} + end + end + # Parses Ruby backtrace strings into the stack frame format # expected by the Datadog UI. # + # Fallback for when backtrace_locations returns nil (see + # serialize_throwable for details on when this happens). + # # Ruby backtrace format: "/path/file.rb:42:in `method_name'" # # @param backtrace [Array, nil] from Exception#backtrace - # @return [Array, nil] - def format_backtrace(backtrace) + # @return [Array] + def format_backtrace_strings(backtrace) return [] if backtrace.nil? backtrace.map do |frame| diff --git a/sig/datadog/di.rbs b/sig/datadog/di.rbs index 3878d708242..e1e27763969 100644 --- a/sig/datadog/di.rbs +++ b/sig/datadog/di.rbs @@ -10,6 +10,8 @@ module Datadog def self.all_iseqs: () -> Array[RubyVM::InstructionSequence] def self.file_iseqs: () -> Array[RubyVM::InstructionSequence] def self.exception_message: (Exception exception) -> untyped + EXCEPTION_BACKTRACE_LOCATIONS: UnboundMethod + EXCEPTION_BACKTRACE: UnboundMethod def self.component: () -> Component diff --git a/sig/datadog/di/probe_notification_builder.rbs b/sig/datadog/di/probe_notification_builder.rbs index fb14c2c2f34..55552801d0f 100644 --- a/sig/datadog/di/probe_notification_builder.rbs +++ b/sig/datadog/di/probe_notification_builder.rbs @@ -26,7 +26,8 @@ module Datadog def build_snapshot: (Context context) -> Hash[Symbol,untyped] def serialize_throwable: (Exception exception) -> Hash[Symbol, String? | Array[Hash[Symbol, String | Integer | nil]]?] - def format_backtrace: (Array[String]? backtrace) -> Array[Hash[Symbol, String | Integer | nil]] + def format_backtrace_locations: (Array[Thread::Backtrace::Location] locations) -> Array[Hash[Symbol, String | Integer | nil]] + def format_backtrace_strings: (Array[String]? backtrace) -> Array[Hash[Symbol, String | Integer | nil]] def build_snapshot_base: (Context context, ?evaluation_errors: Array[untyped]?, ?captures: untyped?, ?message: String?) -> Hash[Symbol,untyped] diff --git a/spec/datadog/di/ext/exception_backtrace_spec.rb b/spec/datadog/di/ext/exception_backtrace_spec.rb new file mode 100644 index 00000000000..905cb30249f --- /dev/null +++ b/spec/datadog/di/ext/exception_backtrace_spec.rb @@ -0,0 +1,150 @@ +require "datadog/di/spec_helper" + +RSpec.describe 'EXCEPTION_BACKTRACE_LOCATIONS' do + subject(:backtrace) do + Datadog::DI::EXCEPTION_BACKTRACE_LOCATIONS.bind(exception).call + end + + context 'when exception has a backtrace' do + let(:exception) do + raise StandardError, 'test' + rescue => e + e + end + + it 'returns an array of Thread::Backtrace::Location' do + expect(backtrace).to be_an(Array) + expect(backtrace).not_to be_empty + expect(backtrace.first).to be_a(Thread::Backtrace::Location) + expect(backtrace.first.path).to be_a(String) + expect(backtrace.first.lineno).to be_a(Integer) + end + end + + context 'when exception has no backtrace' do + let(:exception) do + StandardError.new('no backtrace') + end + + it 'returns nil' do + expect(backtrace).to be_nil + end + end + + context 'when exception class overrides backtrace_locations method' do + let(:exception_class) do + Class.new(StandardError) do + define_method(:backtrace_locations) do + [] + end + end + end + + let(:exception) do + raise exception_class, 'test' + rescue => e + e + end + + it 'returns the real backtrace, not the overridden one' do + # The UnboundMethod bypasses the subclass override. + expect(backtrace).to be_an(Array) + expect(backtrace).not_to be_empty + expect(backtrace.first).to be_a(Thread::Backtrace::Location) + + # Verify the override exists on the Ruby side. + expect(exception.backtrace_locations).to eq([]) + end + end + + context 'when backtrace was set via set_backtrace with strings' do + let(:exception) do + e = StandardError.new('wrapped') + e.set_backtrace(['/app/foo.rb:10:in `bar\'', '/app/baz.rb:20:in `qux\'']) + e + end + + it 'returns nil for backtrace_locations' do + # set_backtrace with Array causes backtrace_locations to + # return nil — the VM cannot reconstruct Location objects from + # formatted strings. + expect(backtrace).to be_nil + end + end +end + +RSpec.describe 'EXCEPTION_BACKTRACE' do + subject(:backtrace) do + Datadog::DI::EXCEPTION_BACKTRACE.bind(exception).call + end + + context 'when exception has a backtrace' do + let(:exception) do + raise StandardError, 'test' + rescue => e + e + end + + it 'returns an array of strings' do + expect(backtrace).to be_an(Array) + expect(backtrace).not_to be_empty + expect(backtrace.first).to be_a(String) + end + end + + context 'when exception has no backtrace' do + let(:exception) do + StandardError.new('no backtrace') + end + + it 'returns nil' do + expect(backtrace).to be_nil + end + end + + context 'when exception class overrides backtrace method' do + let(:exception_class) do + Class.new(StandardError) do + define_method(:backtrace) do + ['overridden:0:in `fake\''] + end + end + end + + let(:exception) do + raise exception_class, 'test' + rescue => e + e + end + + it 'returns nil — UnboundMethod does NOT bypass overrides for backtrace' do + # Unlike backtrace_locations, the UnboundMethod trick does not bypass + # subclass overrides of Exception#backtrace. MRI's setup_exception + # (eval.c) calls rb_get_backtrace during raise, which detects the + # #backtrace override and calls it. Since the override returns + # non-nil, setup_exception skips storing the real VM backtrace in + # @bt entirely. The C function exc_backtrace then reads @bt (still + # nil from exc_init) and returns nil. + # + # This is acceptable because EXCEPTION_BACKTRACE is only used as a + # fallback when backtrace_locations returns nil (the set_backtrace + # with strings case), where no subclass override is involved. + expect(backtrace).to be_nil + + # Verify the override exists on the Ruby side. + expect(exception.backtrace).to eq(['overridden:0:in `fake\'']) + end + end + + context 'when backtrace was set via set_backtrace with strings' do + let(:exception) do + e = StandardError.new('wrapped') + e.set_backtrace(['/app/foo.rb:10:in `bar\'', '/app/baz.rb:20:in `qux\'']) + e + end + + it 'returns the string backtrace' do + expect(backtrace).to eq(['/app/foo.rb:10:in `bar\'', '/app/baz.rb:20:in `qux\'']) + end + end +end diff --git a/spec/datadog/di/probe_notification_builder_spec.rb b/spec/datadog/di/probe_notification_builder_spec.rb index e3451a44c8c..15f6179d9b3 100644 --- a/spec/datadog/di/probe_notification_builder_spec.rb +++ b/spec/datadog/di/probe_notification_builder_spec.rb @@ -565,6 +565,81 @@ end end + context 'when exception has overridden backtrace method' do + let(:exception_class) do + Class.new(StandardError) do + define_method(:backtrace) do + ['overridden:0:in `fake_method\''] + end + end + end + + let(:exception) do + raise exception_class, 'test' + rescue => e + e + end + + let(:context) do + Datadog::DI::Context.new( + probe: probe, + settings: settings, + serializer: serializer, + target_self: target_self, + serialized_entry_args: {}, + return_value: nil, + duration: 0.1, + exception: exception, + ) + end + + let(:payload) { builder.build_executed(context) } + + it 'uses raw backtrace, not overridden backtrace method' do + throwable = payload.dig(:debugger, :snapshot, :captures, :return, :throwable) + expect(throwable[:stacktrace]).to be_an(Array) + expect(throwable[:stacktrace]).not_to eq( + [{fileName: 'overridden', function: 'fake_method', lineNumber: 0}], + ) + # Verify the override exists on the Ruby side + expect(exception.backtrace).to eq(['overridden:0:in `fake_method\'']) + end + end + + context 'when backtrace was set via set_backtrace with strings' do + let(:exception) do + e = StandardError.new('wrapped') + e.set_backtrace(['/app/foo.rb:10:in `bar\'', '/app/baz.rb:20:in `qux\'']) + e + end + + let(:context) do + Datadog::DI::Context.new( + probe: probe, + settings: settings, + serializer: serializer, + target_self: target_self, + serialized_entry_args: {}, + return_value: nil, + duration: 0.1, + exception: exception, + ) + end + + let(:payload) { builder.build_executed(context) } + + it 'falls back to string backtrace parsing' do + # set_backtrace with Array causes backtrace_locations to + # return nil. serialize_throwable should fall back to parsing the + # string backtrace. + throwable = payload.dig(:debugger, :snapshot, :captures, :return, :throwable) + expect(throwable[:stacktrace]).to eq([ + {fileName: '/app/foo.rb', function: 'bar', lineNumber: 10}, + {fileName: '/app/baz.rb', function: 'qux', lineNumber: 20}, + ]) + end + end + context 'when exception constructor argument is not a string' do let(:exception) { NameError.new(42) }