Skip to content

Commit 160e343

Browse files
solnicsolnic
committed
fix(rails): improve AR tracing performance
refs #2595
1 parent 7755dab commit 160e343

File tree

4 files changed

+197
-110
lines changed

4 files changed

+197
-110
lines changed

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

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ class ActiveRecordSubscriber < AbstractSubscriber
1414
SUPPORT_SOURCE_LOCATION = ActiveSupport::BacktraceCleaner.method_defined?(:clean_frame)
1515

1616
if SUPPORT_SOURCE_LOCATION
17-
class_attribute :backtrace_cleaner, default: (ActiveSupport::BacktraceCleaner.new.tap do |cleaner|
17+
backtrace_cleaner = ActiveSupport::BacktraceCleaner.new.tap do |cleaner|
1818
cleaner.add_silencer { |line| line.include?("sentry-ruby/lib") || line.include?("sentry-rails/lib") }
19-
end)
19+
end
20+
21+
class_attribute :backtrace_cleaner, default: backtrace_cleaner.freeze
2022
end
2123

2224
class << self
@@ -62,43 +64,21 @@ def subscribe!
6264
span.set_data(Span::DataConventions::SERVER_PORT, db_config[:port]) if db_config[:port]
6365
span.set_data(Span::DataConventions::SERVER_SOCKET_ADDRESS, db_config[:socket]) if db_config[:socket]
6466

65-
next unless record_query_source
66-
6767
# 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
68+
if record_query_source && duration >= query_source_threshold
69+
backtrace_line = Backtrace.source_location(backtrace_cleaner)
70+
71+
if backtrace_line
72+
span.set_data(Span::DataConventions::FILEPATH, backtrace_line.file) if backtrace_line.file
73+
span.set_data(Span::DataConventions::LINENO, backtrace_line.number) if backtrace_line.number
74+
span.set_data(Span::DataConventions::FUNCTION, backtrace_line.method) if backtrace_line.method
75+
# Only JRuby has namespace in the backtrace
76+
span.set_data(Span::DataConventions::NAMESPACE, backtrace_line.module_name) if backtrace_line.module_name
77+
end
8078
end
8179
end
8280
end
8381
end
84-
85-
# Thread.each_caller_location is an API added in Ruby 3.2 that doesn't always collect the entire stack like
86-
# Kernel#caller or #caller_locations do. See https://github.com/rails/rails/pull/49095 for more context.
87-
if SUPPORT_SOURCE_LOCATION && Thread.respond_to?(:each_caller_location)
88-
def query_source_location
89-
Thread.each_caller_location do |location|
90-
frame = backtrace_cleaner.clean_frame(location)
91-
return frame if frame
92-
end
93-
nil
94-
end
95-
else
96-
# Since Sentry is mostly used in production, we don't want to fallback to the slower implementation
97-
# and adds potentially big overhead to the application.
98-
def query_source_location
99-
nil
100-
end
101-
end
10282
end
10383
end
10484
end

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

Lines changed: 48 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,12 @@
11
# frozen_string_literal: true
22

33
require "rubygems"
4+
require "concurrent/map"
5+
require "sentry/backtrace/line"
46

57
module Sentry
68
# @api private
79
class Backtrace
8-
# Handles backtrace parsing line by line
9-
class Line
10-
RB_EXTENSION = ".rb"
11-
# regexp (optional leading X: on windows, or JRuby9000 class-prefix)
12-
RUBY_INPUT_FORMAT = /
13-
^ \s* (?: [a-zA-Z]: | uri:classloader: )? ([^:]+ | <.*>):
14-
(\d+)
15-
(?: :in\s('|`)(?:([\w:]+)\#)?([^']+)')?$
16-
/x
17-
18-
# org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:170)
19-
JAVA_INPUT_FORMAT = /^([\w$.]+)\.([\w$]+)\(([\w$.]+):(\d+)\)$/
20-
21-
# The file portion of the line (such as app/models/user.rb)
22-
attr_reader :file
23-
24-
# The line number portion of the line
25-
attr_reader :number
26-
27-
# The method of the line (such as index)
28-
attr_reader :method
29-
30-
# The module name (JRuby)
31-
attr_reader :module_name
32-
33-
attr_reader :in_app_pattern
34-
35-
# Parses a single line of a given backtrace
36-
# @param [String] unparsed_line The raw line from +caller+ or some backtrace
37-
# @return [Line] The parsed backtrace line
38-
def self.parse(unparsed_line, in_app_pattern = nil)
39-
ruby_match = unparsed_line.match(RUBY_INPUT_FORMAT)
40-
41-
if ruby_match
42-
_, file, number, _, module_name, method = ruby_match.to_a
43-
file.sub!(/\.class$/, RB_EXTENSION)
44-
module_name = module_name
45-
else
46-
java_match = unparsed_line.match(JAVA_INPUT_FORMAT)
47-
_, module_name, method, file, number = java_match.to_a
48-
end
49-
new(file, number, method, module_name, in_app_pattern)
50-
end
51-
52-
def initialize(file, number, method, module_name, in_app_pattern)
53-
@file = file
54-
@module_name = module_name
55-
@number = number.to_i
56-
@method = method
57-
@in_app_pattern = in_app_pattern
58-
end
59-
60-
def in_app
61-
return false unless in_app_pattern
62-
63-
if file =~ in_app_pattern
64-
true
65-
else
66-
false
67-
end
68-
end
69-
70-
# Reconstructs the line in a readable fashion
71-
def to_s
72-
"#{file}:#{number}:in `#{method}'"
73-
end
74-
75-
def ==(other)
76-
to_s == other.to_s
77-
end
78-
79-
def inspect
80-
"<Line:#{self}>"
81-
end
82-
end
83-
8410
# holder for an Array of Backtrace::Line instances
8511
attr_reader :lines
8612

@@ -100,6 +26,52 @@ def self.parse(backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_cal
10026
new(lines)
10127
end
10228

29+
# Thread.each_caller_location is an API added in Ruby 3.2 that doesn't always collect
30+
# the entire stack like Kernel#caller or #caller_locations do.
31+
#
32+
# @see https://github.com/rails/rails/pull/49095 for more context.
33+
if Thread.respond_to?(:each_caller_location)
34+
def self.source_location(backtrace_cleaner)
35+
cache = (line_cache[backtrace_cleaner.__id__] ||= Concurrent::Map.new)
36+
37+
Thread.each_caller_location do |location|
38+
frame_key = [location.absolute_path, location.lineno]
39+
cached_value = cache[frame_key]
40+
41+
next if cached_value == :skip
42+
43+
if cached_value
44+
return cached_value
45+
else
46+
cleaned_frame = backtrace_cleaner.clean_frame(location)
47+
48+
if cleaned_frame
49+
line = Line.from_source_location(location)
50+
cache[frame_key] = line
51+
52+
return line
53+
else
54+
cache[frame_key] = :skip
55+
56+
next
57+
end
58+
end
59+
end
60+
end
61+
62+
def self.line_cache
63+
@line_cache ||= Concurrent::Map.new
64+
end
65+
else
66+
# Since Sentry is mostly used in production, we don't want to fallback
67+
# to the slower implementation and adds potentially big overhead to the
68+
# application.
69+
def self.source_location(*)
70+
nil
71+
end
72+
end
73+
74+
10375
def initialize(lines)
10476
@lines = lines
10577
end

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# frozen_string_literal: true
2+
3+
module Sentry
4+
# @api private
5+
class Backtrace
6+
# Handles backtrace parsing line by line
7+
class Line
8+
RB_EXTENSION = ".rb"
9+
# regexp (optional leading X: on windows, or JRuby9000 class-prefix)
10+
RUBY_INPUT_FORMAT = /
11+
^ \s* (?: [a-zA-Z]: | uri:classloader: )? ([^:]+ | <.*>):
12+
(\d+)
13+
(?: :in\s('|`)(?:([\w:]+)\#)?([^']+)')?$
14+
/x
15+
16+
# org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:170)
17+
JAVA_INPUT_FORMAT = /^([\w$.]+)\.([\w$]+)\(([\w$.]+):(\d+)\)$/
18+
19+
# The file portion of the line (such as app/models/user.rb)
20+
attr_reader :file
21+
22+
# The line number portion of the line
23+
attr_reader :number
24+
25+
# The method of the line (such as index)
26+
attr_reader :method
27+
28+
# The module name (JRuby)
29+
attr_reader :module_name
30+
31+
attr_reader :in_app_pattern
32+
33+
# Parses a single line of a given backtrace
34+
# @param [String] unparsed_line The raw line from +caller+ or some backtrace
35+
# @return [Line] The parsed backtrace line
36+
def self.parse(unparsed_line, in_app_pattern = nil)
37+
ruby_match = unparsed_line.match(RUBY_INPUT_FORMAT)
38+
39+
if ruby_match
40+
_, file, number, _, module_name, method = ruby_match.to_a
41+
file.sub!(/\.class$/, RB_EXTENSION)
42+
module_name = module_name
43+
else
44+
java_match = unparsed_line.match(JAVA_INPUT_FORMAT)
45+
_, module_name, method, file, number = java_match.to_a
46+
end
47+
new(file, number, method, module_name, in_app_pattern)
48+
end
49+
50+
# Creates a Line from a Thread::Backtrace::Location object
51+
# This is more efficient than converting to string and parsing with regex
52+
# @param [Thread::Backtrace::Location] location The location object
53+
# @param [Regexp, nil] in_app_pattern Optional pattern to determine if the line is in-app
54+
# @return [Line] The backtrace line
55+
def self.from_source_location(location, in_app_pattern = nil)
56+
file = location.absolute_path
57+
number = location.lineno
58+
method = location.base_label
59+
60+
label = location.label
61+
index = label.index("#") || label.index(".")
62+
module_name = label[0, index] if index
63+
64+
new(file, number, method, module_name, in_app_pattern)
65+
end
66+
67+
def initialize(file, number, method, module_name, in_app_pattern)
68+
@file = file
69+
@module_name = module_name
70+
@number = number.to_i
71+
@method = method
72+
@in_app_pattern = in_app_pattern
73+
end
74+
75+
def in_app
76+
return false unless in_app_pattern
77+
78+
if file =~ in_app_pattern
79+
true
80+
else
81+
false
82+
end
83+
end
84+
85+
# Reconstructs the line in a readable fashion
86+
def to_s
87+
"#{file}:#{number}:in `#{method}'"
88+
end
89+
90+
def ==(other)
91+
to_s == other.to_s
92+
end
93+
94+
def inspect
95+
"<Line:#{self}>"
96+
end
97+
end
98+
end
99+
end

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,40 @@
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+
location = caller_locations.first
48+
line = described_class.from_source_location(location, in_app_pattern)
49+
50+
expect(line).to be_a(described_class)
51+
expect(line.file).to be_a(String)
52+
expect(line.number).to be_a(Integer)
53+
expect(line.method).to be_a(String)
54+
expect(line.in_app_pattern).to eq(in_app_pattern)
55+
end
56+
57+
it "extracts file, line number, and method correctly" do
58+
location = caller_locations.first
59+
line = described_class.from_source_location(location)
60+
61+
expect(line.file).to eq(location.absolute_path)
62+
expect(line.number).to eq(location.lineno)
63+
expect(line.method).to eq(location.base_label)
64+
end
65+
66+
it "extracts module name from label when present", when: { ruby_version?: [:>=, "3.4"] } do
67+
location = caller_locations.first
68+
line = described_class.from_source_location(location)
69+
70+
expect(line.module_name).to be_a(String)
71+
end
72+
73+
it "skips module name from label when present", when: { ruby_version?: [:<, "3.4"] } do
74+
location = caller_locations.first
75+
line = described_class.from_source_location(location)
76+
77+
expect(line.module_name).to be(nil)
78+
end
79+
end
4480
end

0 commit comments

Comments
 (0)