WIP

Author: solnic

sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb

@@ -14,24 +14,21 @@ class ActiveRecordSubscriber < AbstractSubscriber
         SUPPORT_SOURCE_LOCATION = ActiveSupport::BacktraceCleaner.method_defined?(:clean_frame)
 
         if SUPPORT_SOURCE_LOCATION
-          class_attribute :backtrace_cleaner, default: (ActiveSupport::BacktraceCleaner.new.tap do |cleaner|
+          backtrace_cleaner = ActiveSupport::BacktraceCleaner.new.tap do |cleaner|
             cleaner.add_silencer { |line| line.include?("sentry-ruby/lib") || line.include?("sentry-rails/lib") }
-          end)
-
-          CLEAN_FRAME_CACHE_SIZE = 1000
+          end
 
-          @clean_frame_cache = {}
-          @backtrace_cache = {}
+          class_attribute :backtrace_cleaner, default: backtrace_cleaner.freeze
         end
 
         class << self
           def subscribe!
             subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
-              next if EXCLUDED_EVENTS.include? payload[:name]
-
               record_query_source = SUPPORT_SOURCE_LOCATION && Sentry.configuration.rails.enable_db_query_source
               query_source_threshold = Sentry.configuration.rails.db_query_source_threshold_ms
 
+              next if EXCLUDED_EVENTS.include? payload[:name]
+
               record_on_current_span(
                 op: SPAN_PREFIX + event_name,
                 origin: SPAN_ORIGIN,
@@ -69,7 +66,7 @@ def subscribe!
 
                 # both duration and query_source_threshold are in ms
                 if record_query_source && duration >= query_source_threshold
-                  backtrace_line = query_source_location
+                  backtrace_line = Backtrace.source_location(backtrace_cleaner)
 
                   if backtrace_line
                     span.set_data(Span::DataConventions::FILEPATH, backtrace_line.file) if backtrace_line.file
@@ -82,48 +79,6 @@ def subscribe!
               end
             end
           end
-
-          # Thread.each_caller_location is an API added in Ruby 3.2 that doesn't always collect the entire stack like
-          # Kernel#caller or #caller_locations do. See https://github.com/rails/rails/pull/49095 for more context.
-          if SUPPORT_SOURCE_LOCATION && Thread.respond_to?(:each_caller_location)
-            def query_source_location
-              Thread.each_caller_location do |location|
-                if should_include_location?(location)
-                  return cached_backtrace_line(location)
-                end
-              end
-              nil
-            end
-
-            def should_include_location?(location)
-              cache_key = [location.absolute_path, location.lineno]
-
-              cached_result = @clean_frame_cache[cache_key]
-              return cached_result unless cached_result.nil?
-
-              # Check if backtrace_cleaner would include this frame
-              frame = backtrace_cleaner.clean_frame(location)
-              should_include = !frame.nil?
-
-              if @clean_frame_cache.size < CLEAN_FRAME_CACHE_SIZE
-                @clean_frame_cache[cache_key] = should_include
-              end
-
-              should_include
-            end
-
-            def cached_backtrace_line(location)
-              cache_key = [location.absolute_path, location.lineno]
-
-              @backtrace_cache[cache_key] ||= Sentry::Backtrace::Line.from_source_location(location)
-            end
-          else
-            # Since Sentry is mostly used in production, we don't want to fallback to the slower implementation
-            # and adds potentially big overhead to the application.
-            def query_source_location
-              nil
-            end
-          end
         end
       end
     end

sentry-ruby/lib/sentry/backtrace.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "rubygems"
+require "concurrent/map"
 require "sentry/backtrace/line"
 
 module Sentry
@@ -25,6 +26,52 @@ def self.parse(backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_cal
       new(lines)
     end
 
+    # Thread.each_caller_location is an API added in Ruby 3.2 that doesn't always collect
+    # the entire stack like Kernel#caller or #caller_locations do.
+    #
+    # @see https://github.com/rails/rails/pull/49095 for more context.
+    if Thread.respond_to?(:each_caller_location)
+      def self.source_location(backtrace_cleaner)
+        cache = line_cache.fetch_or_store(backtrace_cleaner.__id__) { Concurrent::Map.new }
+
+        Thread.each_caller_location do |location|
+          frame_key = [location.absolute_path, location.lineno].freeze
+          cached_value = cache[frame_key]
+
+          next if cached_value == :skip
+
+          if cached_value
+            return cached_value
+          else
+            cleaned_frame = backtrace_cleaner.clean_frame(location)
+
+            if cleaned_frame
+              line = Line.from_source_location(location)
+              cache[frame_key] = line
+
+              return line
+            else
+              cache[frame_key] = :skip
+
+              next
+            end
+          end
+        end
+      end
+
+      def self.line_cache
+        @line_cache ||= Concurrent::Map.new
+      end
+    else
+      # Since Sentry is mostly used in production, we don't want to fallback
+      # to the slower implementation and adds potentially big overhead to the
+      # application.
+      def self.source_location(*)
+        nil
+      end
+    end
+
+
     def initialize(lines)
       @lines = lines
     end

sentry-ruby/lib/sentry/backtrace/line.rb

@@ -53,23 +53,13 @@ def self.parse(unparsed_line, in_app_pattern = nil)
       # @param [Regexp, nil] in_app_pattern Optional pattern to determine if the line is in-app
       # @return [Line] The backtrace line
       def self.from_source_location(location, in_app_pattern = nil)
-        # Directly extract information from Thread::Backtrace::Location object
-        # instead of converting to string and parsing with regex
-        file = location.absolute_path || location.path
+        file = location.absolute_path
         number = location.lineno
+        method = location.base_label
 
-        # In Ruby 3.4+, label includes class name like "ClassName#method_name"
-        # base_label is just the method name without the class
-        method = location.base_label || location.label
-
-        # Extract module/class name from label if present (Ruby 3.4+)
-        # Format: "ClassName#method_name" or "ClassName.method_name"
         label = location.label
-        module_name = if label && label.include?("#")
-          label.split("#", 2).first
-        elsif label && label.include?(".")
-          label.split(".", 2).first
-        end
+        index = label.index("#") || label.index(".")
+        module_name = label[0, index] if index
 
         new(file, number, method, module_name, in_app_pattern)
       end

sentry-ruby/lib/sentry/benchmarks/benchmark_transport.rb

@@ -13,4 +13,3 @@ def send_envelope(envelope)
     end
   end
 end
-

sentry-ruby/spec/sentry/backtrace/lines_spec.rb

@@ -44,11 +44,9 @@
 
   describe ".from_source_location", skip: !Thread.respond_to?(:each_caller_location) do
     it "creates a Line from Thread::Backtrace::Location" do
-      # Get a location from the current call stack
       location = caller_locations.first
       line = described_class.from_source_location(location, in_app_pattern)
 
-      # Verify the line was created with correct types
       expect(line).to be_a(described_class)
       expect(line.file).to be_a(String)
       expect(line.number).to be_a(Integer)
@@ -60,20 +58,23 @@
       location = caller_locations.first
       line = described_class.from_source_location(location)
 
-      # Verify that the extracted values match the location object
-      expect(line.file).to eq(location.absolute_path || location.path)
+      expect(line.file).to eq(location.absolute_path)
       expect(line.number).to eq(location.lineno)
-      expect(line.method).to eq(location.base_label || location.label)
+      expect(line.method).to eq(location.base_label)
     end
 
-    it "extracts module name from label when present" do
+    it "extracts module name from label when present", when: { ruby_version?: [:>=, "3.4"] } do
       location = caller_locations.first
       line = described_class.from_source_location(location)
 
-      # Module name extraction depends on Ruby version and context
-      # In Ruby 3.4+, label includes class name for methods defined in classes
-      # The module_name should be a string if present, or nil
-      expect(line.module_name).to be_a(String).or be_nil
+      expect(line.module_name).to be_a(String)
+    end
+
+    it "skips module name from label when present", when: { ruby_version?: [:<, "3.4"] } do
+      location = caller_locations.first
+      line = described_class.from_source_location(location)
+
+      expect(line.module_name).to be(nil)
     end
   end
 end