Skip to content

Commit 84f04ce

Browse files
solnicsolnic
committed
WIP
1 parent 7755dab commit 84f04ce

File tree

6 files changed

+210
-20
lines changed

6 files changed

+210
-20
lines changed

‎sentry-rails/benchmarks/db_query_source_benchmark.rb‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# frozen_string_literal: true
2+
3+
require 'benchmark/ips'
4+
require 'benchmark/memory'
5+
require 'debug'
6+
7+
require "sentry-rails"
8+
9+
require_relative "../spec/dummy/test_rails_app/config/application"
10+
require_relative "../spec/support/test_helper"
11+
12+
include Sentry::Rails::TestHelper
13+
14+
Sentry::TestRailsApp.load_test_schema
15+
16+
# Benchmark workload: perform various DB queries
17+
def run_queries
18+
Sentry.with_scope do |scope|
19+
transaction = Sentry.start_transaction(name: "benchmark", op: "benchmark")
20+
scope.set_span(transaction)
21+
22+
# Mix of different query types
23+
Post.count
24+
Post.first
25+
Post.where(title: "Post 1").first
26+
Post.order(created_at: :desc).limit(5).to_a
27+
Post.find_by(title: "Post 5")
28+
29+
transaction.finish
30+
end
31+
end
32+
33+
Benchmark.ips do |x|
34+
x.config(warmup: 2)
35+
36+
x.report("disabled") {
37+
app = make_basic_app do |config|
38+
config.traces_sample_rate = 1.0
39+
config.rails.enable_db_query_source = false
40+
end
41+
42+
1000.times { run_queries }
43+
44+
app.cleanup!
45+
}
46+
47+
x.report("enabled, threshold=0ms") {
48+
app = make_basic_app do |config|
49+
config.traces_sample_rate = 1.0
50+
config.rails.enable_db_query_source = true
51+
config.rails.db_query_source_threshold_ms = 0.001
52+
end
53+
54+
1000.times { run_queries }
55+
56+
app.cleanup!
57+
}
58+
59+
x.compare!
60+
end
61+
62+
Benchmark.memory do |x|
63+
x.report("disabled") do
64+
app = make_basic_app do |config|
65+
config.traces_sample_rate = 1.0
66+
config.rails.enable_db_query_source = false
67+
end
68+
69+
1000.times { run_queries }
70+
71+
app.cleanup!
72+
end
73+
74+
x.report("enabled, threshold=0ms") do
75+
app = make_basic_app do |config|
76+
config.traces_sample_rate = 1.0
77+
config.rails.enable_db_query_source = true
78+
config.rails.db_query_source_threshold_ms = 0.001
79+
end
80+
81+
1000.times { run_queries }
82+
83+
app.cleanup!
84+
end
85+
86+
x.compare!
87+
end

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

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,21 @@ class ActiveRecordSubscriber < AbstractSubscriber
1717
class_attribute :backtrace_cleaner, default: (ActiveSupport::BacktraceCleaner.new.tap do |cleaner|
1818
cleaner.add_silencer { |line| line.include?("sentry-ruby/lib") || line.include?("sentry-rails/lib") }
1919
end)
20+
21+
CLEAN_FRAME_CACHE_SIZE = 1000
22+
23+
@clean_frame_cache = {}
24+
@backtrace_cache = {}
2025
end
2126

2227
class << self
2328
def subscribe!
24-
record_query_source = SUPPORT_SOURCE_LOCATION && Sentry.configuration.rails.enable_db_query_source
25-
query_source_threshold = Sentry.configuration.rails.db_query_source_threshold_ms
26-
2729
subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
2830
next if EXCLUDED_EVENTS.include? payload[:name]
2931

32+
record_query_source = SUPPORT_SOURCE_LOCATION && Sentry.configuration.rails.enable_db_query_source
33+
query_source_threshold = Sentry.configuration.rails.db_query_source_threshold_ms
34+
3035
record_on_current_span(
3136
op: SPAN_PREFIX + event_name,
3237
origin: SPAN_ORIGIN,
@@ -62,21 +67,17 @@ def subscribe!
6267
span.set_data(Span::DataConventions::SERVER_PORT, db_config[:port]) if db_config[:port]
6368
span.set_data(Span::DataConventions::SERVER_SOCKET_ADDRESS, db_config[:socket]) if db_config[:socket]
6469

65-
next unless record_query_source
66-
6770
# both duration and query_source_threshold are in ms
68-
next unless duration >= query_source_threshold
69-
70-
source_location = query_source_location
71-
72-
if source_location
73-
backtrace_line = Sentry::Backtrace::Line.parse(source_location)
74-
75-
span.set_data(Span::DataConventions::FILEPATH, backtrace_line.file) if backtrace_line.file
76-
span.set_data(Span::DataConventions::LINENO, backtrace_line.number) if backtrace_line.number
77-
span.set_data(Span::DataConventions::FUNCTION, backtrace_line.method) if backtrace_line.method
78-
# Only JRuby has namespace in the backtrace
79-
span.set_data(Span::DataConventions::NAMESPACE, backtrace_line.module_name) if backtrace_line.module_name
71+
if record_query_source && duration >= query_source_threshold
72+
backtrace_line = query_source_location
73+
74+
if backtrace_line
75+
span.set_data(Span::DataConventions::FILEPATH, backtrace_line.file) if backtrace_line.file
76+
span.set_data(Span::DataConventions::LINENO, backtrace_line.number) if backtrace_line.number
77+
span.set_data(Span::DataConventions::FUNCTION, backtrace_line.method) if backtrace_line.method
78+
# Only JRuby has namespace in the backtrace
79+
span.set_data(Span::DataConventions::NAMESPACE, backtrace_line.module_name) if backtrace_line.module_name
80+
end
8081
end
8182
end
8283
end
@@ -87,11 +88,35 @@ def subscribe!
8788
if SUPPORT_SOURCE_LOCATION && Thread.respond_to?(:each_caller_location)
8889
def query_source_location
8990
Thread.each_caller_location do |location|
90-
frame = backtrace_cleaner.clean_frame(location)
91-
return frame if frame
91+
if should_include_location?(location)
92+
return cached_backtrace_line(location)
93+
end
9294
end
9395
nil
9496
end
97+
98+
def should_include_location?(location)
99+
cache_key = [location.absolute_path, location.lineno]
100+
101+
cached_result = @clean_frame_cache[cache_key]
102+
return cached_result unless cached_result.nil?
103+
104+
# Check if backtrace_cleaner would include this frame
105+
frame = backtrace_cleaner.clean_frame(location)
106+
should_include = !frame.nil?
107+
108+
if @clean_frame_cache.size < CLEAN_FRAME_CACHE_SIZE
109+
@clean_frame_cache[cache_key] = should_include
110+
end
111+
112+
should_include
113+
end
114+
115+
def cached_backtrace_line(location)
116+
cache_key = [location.absolute_path, location.lineno]
117+
118+
@backtrace_cache[cache_key] ||= Sentry::Backtrace::Line.from_source_location(location)
119+
end
95120
else
96121
# Since Sentry is mostly used in production, we don't want to fallback to the slower implementation
97122
# and adds potentially big overhead to the application.

‎sentry-rails/spec/support/test_helper.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# frozen_string_literal: true
22

33
require "sentry/test_helper"
4-
require "dummy/test_rails_app/config/application"
4+
require_relative "../dummy/test_rails_app/config/application"
55

66
module Sentry
77
module Rails

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,33 @@ def self.parse(unparsed_line, in_app_pattern = nil)
4949
new(file, number, method, module_name, in_app_pattern)
5050
end
5151

52+
# Creates a Line from a Thread::Backtrace::Location object
53+
# This is more efficient than converting to string and parsing with regex
54+
# @param [Thread::Backtrace::Location] location The location object
55+
# @param [Regexp, nil] in_app_pattern Optional pattern to determine if the line is in-app
56+
# @return [Line] The backtrace line
57+
def self.from_source_location(location, in_app_pattern = nil)
58+
# Directly extract information from Thread::Backtrace::Location object
59+
# instead of converting to string and parsing with regex
60+
file = location.absolute_path || location.path
61+
number = location.lineno
62+
63+
# In Ruby 3.4+, label includes class name like "ClassName#method_name"
64+
# base_label is just the method name without the class
65+
method = location.base_label || location.label
66+
67+
# Extract module/class name from label if present (Ruby 3.4+)
68+
# Format: "ClassName#method_name" or "ClassName.method_name"
69+
label = location.label
70+
module_name = if label && label.include?("#")
71+
label.split("#", 2).first
72+
elsif label && label.include?(".")
73+
label.split(".", 2).first
74+
end
75+
76+
new(file, number, method, module_name, in_app_pattern)
77+
end
78+
5279
def initialize(file, number, method, module_name, in_app_pattern)
5380
@file = file
5481
@module_name = module_name

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# frozen_string_literal: true
2+
3+
module Sentry
4+
# A minimal transport for benchmarking that does nothing
5+
# This avoids network overhead and focuses on SDK performance
6+
class BenchmarkTransport < Transport
7+
def send_data(data, options = {})
8+
# no-op
9+
end
10+
11+
def send_envelope(envelope)
12+
# no-op
13+
end
14+
end
15+
end
16+

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,39 @@
4141
expect(line.in_app).to eq(false)
4242
end
4343
end
44+
45+
describe ".from_source_location", skip: !Thread.respond_to?(:each_caller_location) do
46+
it "creates a Line from Thread::Backtrace::Location" do
47+
# Get a location from the current call stack
48+
location = caller_locations.first
49+
line = described_class.from_source_location(location, in_app_pattern)
50+
51+
# Verify the line was created with correct types
52+
expect(line).to be_a(described_class)
53+
expect(line.file).to be_a(String)
54+
expect(line.number).to be_a(Integer)
55+
expect(line.method).to be_a(String)
56+
expect(line.in_app_pattern).to eq(in_app_pattern)
57+
end
58+
59+
it "extracts file, line number, and method correctly" do
60+
location = caller_locations.first
61+
line = described_class.from_source_location(location)
62+
63+
# Verify that the extracted values match the location object
64+
expect(line.file).to eq(location.absolute_path || location.path)
65+
expect(line.number).to eq(location.lineno)
66+
expect(line.method).to eq(location.base_label || location.label)
67+
end
68+
69+
it "extracts module name from label when present" do
70+
location = caller_locations.first
71+
line = described_class.from_source_location(location)
72+
73+
# Module name extraction depends on Ruby version and context
74+
# In Ruby 3.4+, label includes class name for methods defined in classes
75+
# The module_name should be a string if present, or nil
76+
expect(line.module_name).to be_a(String).or be_nil
77+
end
78+
end
4479
end

0 commit comments

Comments
 (0)