diff --git a/.changeset/tidy-frames-drum.md b/.changeset/tidy-frames-drum.md new file mode 100644 index 0000000..39b07a4 --- /dev/null +++ b/.changeset/tidy-frames-drum.md @@ -0,0 +1,5 @@ +--- +"posthog-ruby": minor +--- + +Use stable project-relative stack frame `filename` values (relative to `Rails.root` or `Dir.pwd`, falling back to the basename outside the project) while preserving the raw path in `abs_path`; dependency and stdlib frames are detected from RubyGems/stdlib install roots, computed once per stacktrace, so `in_app` no longer depends on deploy-path substrings. diff --git a/lib/posthog/exception_capture.rb b/lib/posthog/exception_capture.rb index 5a62f44..08d69d4 100644 --- a/lib/posthog/exception_capture.rb +++ b/lib/posthog/exception_capture.rb @@ -97,8 +97,10 @@ def self.build_single_exception_from_data(title, message, backtrace, mechanism: def self.build_stacktrace(backtrace) return nil unless backtrace && !backtrace.empty? + root = project_root + roots = dependency_roots(root) frames = backtrace.first(50).map do |line| - parse_backtrace_line(line) + parse_backtrace_line(line, project_root: root, dependency_roots: roots) end.compact.reverse { @@ -108,8 +110,10 @@ def self.build_stacktrace(backtrace) end # @param line [String] + # @param project_root [String, nil] Project root used to derive project-relative filenames. + # @param dependency_roots [Array, nil] Cached gem and stdlib roots. # @return [Hash, nil] - def self.parse_backtrace_line(line) + def self.parse_backtrace_line(line, project_root: self.project_root, dependency_roots: nil) match = line.match(RUBY_INPUT_FORMAT) return nil unless match @@ -118,11 +122,11 @@ def self.parse_backtrace_line(line) method_name = match[5] frame = { - 'filename' => File.basename(file), + 'filename' => frame_filename(file, project_root), 'abs_path' => file, 'lineno' => lineno, 'function' => method_name, - 'in_app' => !gem_path?(file), + 'in_app' => !gem_path?(file, dependency_roots || self.dependency_roots(project_root)), 'platform' => 'ruby' } @@ -131,13 +135,71 @@ def self.parse_backtrace_line(line) frame end + # Root directory the application runs from, used to derive stable + # project-relative filenames from per-host deploy paths + # (e.g. `/app/releases/20240101/...`). + # + # @return [String] + def self.project_root + if defined?(::Rails) && ::Rails.respond_to?(:root) && ::Rails.root + ::Rails.root.to_s + else + Dir.pwd + end + rescue StandardError + Dir.pwd + end + + # Stable filename used for fingerprinting: the project-relative path when + # the file lives inside the project root, its basename otherwise. The raw + # absolute path is kept separately in `abs_path`. + # + # @param path [String] + # @param project_root [String, nil] + # @return [String] + def self.frame_filename(path, project_root) + if project_root && !project_root.empty? && path_within?(path, project_root) + relative = path[project_root.length..] + relative = relative[1..] while relative.start_with?(File::SEPARATOR) + return relative unless relative.empty? + end + + File.basename(path) + end + + # Whether the path belongs to an installed gem or the Ruby standard library, + # based on gem install locations rather than path substrings. + # + # @param path [String] + # @param dependency_roots [Array] + # @return [Boolean] + def self.gem_path?(path, dependency_roots = self.dependency_roots) + dependency_roots.any? { |root| path_within?(path, root) } + end + + # @param project_root [String] + # @return [Array] Directories containing installed gems and the Ruby stdlib. + def self.dependency_roots(project_root = self.project_root) + roots = [] + if defined?(Gem) + roots.concat(Gem.path) if Gem.respond_to?(:path) + roots << Gem.default_dir if Gem.respond_to?(:default_dir) + roots.concat(Gem.loaded_specs.each_value.map(&:full_gem_path)) if Gem.respond_to?(:loaded_specs) + end + roots << RbConfig::CONFIG['rubylibprefix'] if defined?(RbConfig) + # The project itself can be a loaded spec (e.g. a gem developed in place, + # or a Rails engine); its frames are still application code. + roots.compact.reject(&:empty?).uniq - [project_root] + rescue StandardError + [] + end + # @param path [String] + # @param root [String] # @return [Boolean] - def self.gem_path?(path) - path.include?('/gems/') || - path.include?('/ruby/') || - path.include?('/.rbenv/') || - path.include?('/.rvm/') + def self.path_within?(path, root) + root = root.chomp(File::SEPARATOR) + path == root || path.start_with?("#{root}#{File::SEPARATOR}") end # @param frame [Hash] diff --git a/spec/posthog/exception_capture_spec.rb b/spec/posthog/exception_capture_spec.rb index 5f51da4..f423439 100644 --- a/spec/posthog/exception_capture_spec.rb +++ b/spec/posthog/exception_capture_spec.rb @@ -20,18 +20,26 @@ module PostHog end it 'identifies gem files correctly' do - gem_line = '/path/to/gems/ruby-3.0.0/lib/ruby/gems/3.0.0/gems/some_gem/lib/some_gem.rb:10:in `gem_method\'' - frame = described_class.parse_backtrace_line(gem_line) + gem_file = File.join(Gem.path.first, 'gems', 'some_gem-1.0.0', 'lib', 'some_gem.rb') + frame = described_class.parse_backtrace_line("#{gem_file}:10:in `gem_method'") + + expect(frame['in_app']).to be false + end + + it 'identifies standard library files correctly' do + stdlib_file = File.join(RbConfig::CONFIG['rubylibdir'], 'json.rb') + frame = described_class.parse_backtrace_line("#{stdlib_file}:10:in `parse'") expect(frame['in_app']).to be false end it 'does not add context lines for non-in_app frames' do - # Use a gem-style path that points to this real file so File.exist? would be true - # but in_app should be false, so context lines should not be added - gem_line = "#{__FILE__}:10:in `gem_method'" - .gsub(%r{/spec/}, '/gems/ruby/spec/') - frame = described_class.parse_backtrace_line(gem_line) + # Use a real file from a loaded gem so File.exist? is true + # but in_app is false, so context lines should not be added + gem_file = File.join(Gem.loaded_specs['rspec-core'].full_gem_path, 'lib', 'rspec', 'core.rb') + expect(File.exist?(gem_file)).to be true + + frame = described_class.parse_backtrace_line("#{gem_file}:10:in `gem_method'") expect(frame['in_app']).to be false expect(frame['context_line']).to be_nil @@ -39,6 +47,30 @@ module PostHog expect(frame['post_context']).to be_nil end + it 'uses project-relative filenames for paths inside the project root' do + line = '/app/releases/20240101/app/models/user.rb:42:in `validate_email\'' + frame = described_class.parse_backtrace_line(line, project_root: '/app/releases/20240101') + + expect(frame['filename']).to eq('app/models/user.rb') + expect(frame['abs_path']).to eq('/app/releases/20240101/app/models/user.rb') + end + + it 'falls back to the basename for paths outside the project root' do + line = '/somewhere/else/app/models/user.rb:42:in `validate_email\'' + frame = described_class.parse_backtrace_line(line, project_root: '/app/releases/20240101') + + expect(frame['filename']).to eq('user.rb') + expect(frame['abs_path']).to eq('/somewhere/else/app/models/user.rb') + end + + it 'derives filenames for real project files relative to the current working directory' do + frame = described_class.parse_backtrace_line("#{File.expand_path(__FILE__)}:10:in `app_method'") + + expect(frame['filename']).to eq('spec/posthog/exception_capture_spec.rb') + expect(frame['abs_path']).to eq(File.expand_path(__FILE__)) + expect(frame['in_app']).to be true + end + it 'adds context lines for in_app frames' do # Use a real in_app path so File.exist? is true and in_app is true app_line = "#{__FILE__}:10:in `app_method'" @@ -111,6 +143,19 @@ def test_method_that_throws expect(stacktrace['frames'][0]['filename']).to eq('action_controller.rb') expect(stacktrace['frames'][1]['filename']).to eq('user.rb') end + + it 'computes dependency roots once for all frames' do + backtrace = [ + '/app/app/models/user.rb:42:in `validate_email\'', + '/app/app/controllers/users_controller.rb:10:in `show\'' + ] + + expect(described_class).to receive(:dependency_roots).once.and_return([]) + + stacktrace = described_class.build_stacktrace(backtrace) + + expect(stacktrace['frames'].length).to eq(2) + end end describe '#build_parsed_exception' do