Merge branch 'sh-backtrace-cleaner-memory' into 'master'

Move backtrace cleaner from Profiler into its own class

Closes #36645

See merge request gitlab-org/gitlab!22264

lib/gitlab/backtrace_cleaner.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BacktraceCleaner
+    IGNORE_BACKTRACES = %w[
+      config/initializers
+      ee/lib/gitlab/middleware/
+      lib/gitlab/correlation_id.rb
+      lib/gitlab/database/load_balancing/
+      lib/gitlab/etag_caching/
+      lib/gitlab/i18n.rb
+      lib/gitlab/metrics/
+      lib/gitlab/middleware/
+      lib/gitlab/performance_bar/
+      lib/gitlab/profiler.rb
+      lib/gitlab/query_limiting/
+      lib/gitlab/request_context.rb
+      lib/gitlab/request_profiler/
+      lib/gitlab/sidekiq_logging/
+      lib/gitlab/sidekiq_middleware/
+      lib/gitlab/sidekiq_status/
+      lib/gitlab/tracing/
+      lib/gitlab/webpack/dev_server_middleware.rb
+    ].freeze
+
+    IGNORED_BACKTRACES_REGEXP = Regexp.union(IGNORE_BACKTRACES).freeze
+
+    def self.clean_backtrace(backtrace)
+      return unless backtrace
+
+      Array(Rails.backtrace_cleaner.clean(backtrace)).reject do |line|
+        line.match(IGNORED_BACKTRACES_REGEXP)
+      end
+    end
+  end
+end

lib/gitlab/bitbucket_import/importer.rb

@@ -42,7 +42,7 @@ module Gitlab
       end
 
       def store_pull_request_error(pull_request, ex)
-        backtrace = Gitlab::Profiler.clean_backtrace(ex.backtrace)
+        backtrace = Gitlab::BacktraceCleaner.clean_backtrace(ex.backtrace)
         error = { type: :pull_request, iid: pull_request.iid, errors: ex.message, trace: backtrace, raw_response: pull_request.raw }
 
         Gitlab::ErrorTracking.log_exception(ex, error)

lib/gitlab/exception_log_formatter.rb

@@ -13,7 +13,7 @@ module Gitlab
       )
 
       if exception.backtrace
-        payload['exception.backtrace'] = Gitlab::Profiler.clean_backtrace(exception.backtrace)
+        payload['exception.backtrace'] = Gitlab::BacktraceCleaner.clean_backtrace(exception.backtrace)
       end
     end
   end

lib/gitlab/git/rugged_impl/use_rugged.rb

@@ -27,7 +27,7 @@ module Gitlab
                 feature: method_name,
                 args: args,
                 duration: duration,
-                backtrace: Gitlab::Profiler.clean_backtrace(caller))
+                backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller))
             end
 
             result

lib/gitlab/gitaly_client.rb

@@ -179,7 +179,7 @@ module Gitlab
       self.query_time += duration
       if Gitlab::PerformanceBar.enabled_for_request?
         add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc,
-                         backtrace: Gitlab::Profiler.clean_backtrace(caller))
+                         backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller))
       end
     end
 
@@ -438,7 +438,7 @@ module Gitlab
     def self.count_stack
       return unless Gitlab::SafeRequestStore.active?
 
-      stack_string = Gitlab::Profiler.clean_backtrace(caller).drop(1).join("\n")
+      stack_string = Gitlab::BacktraceCleaner.clean_backtrace(caller).drop(1).join("\n")
 
       Gitlab::SafeRequestStore[:stack_counter] ||= Hash.new
 

lib/gitlab/profiler.rb

@@ -107,7 +107,7 @@ module Gitlab
 
             super
 
-            Gitlab::Profiler.clean_backtrace(caller).each do |caller_line|
+            Gitlab::BacktraceCleaner.clean_backtrace(caller).each do |caller_line|
               stripped_caller_line = caller_line.sub("#{Rails.root}/", '')
 
               super("  ↳ #{stripped_caller_line}")
@@ -117,14 +117,6 @@ module Gitlab
       end
     end
 
-    def self.clean_backtrace(backtrace)
-      return unless backtrace
-
-      Array(Rails.backtrace_cleaner.clean(backtrace)).reject do |line|
-        line.match(Regexp.union(IGNORE_BACKTRACES))
-      end
-    end
-
     def self.with_custom_logger(logger)
       original_colorize_logging = ActiveSupport::LogSubscriber.colorize_logging
       original_activerecord_logger = ActiveRecord::Base.logger

lib/gitlab/sidekiq_logging/exception_handler.rb

@@ -18,7 +18,7 @@ module Gitlab
           data.merge!(job_data) if job_data.present?
         end
 
-        data[:error_backtrace] = Gitlab::Profiler.clean_backtrace(job_exception.backtrace) if job_exception.backtrace.present?
+        data[:error_backtrace] = Gitlab::BacktraceCleaner.clean_backtrace(job_exception.backtrace) if job_exception.backtrace.present?
 
         Sidekiq.logger.warn(data)
       end

lib/peek/views/active_record.rb

@@ -32,7 +32,7 @@ module Peek
               detail_store << {
                 duration: finish - start,
                 sql: data[:sql].strip,
-                backtrace: Gitlab::Profiler.clean_backtrace(caller)
+                backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller)
               }
             end
           end

lib/peek/views/redis_detailed.rb

@@ -23,7 +23,7 @@ module Gitlab
         detail_store << {
           cmd: args.first,
           duration: duration,
-          backtrace: ::Gitlab::Profiler.clean_backtrace(caller)
+          backtrace: ::Gitlab::BacktraceCleaner.clean_backtrace(caller)
         }
       end
 

spec/initializers/lograge_spec.rb

@@ -112,7 +112,7 @@ describe 'lograge', type: :request do
 
         expect(log_data['exception.class']).to eq('RuntimeError')
         expect(log_data['exception.message']).to eq('bad request')
-        expect(log_data['exception.backtrace']).to eq(Gitlab::Profiler.clean_backtrace(backtrace))
+        expect(log_data['exception.backtrace']).to eq(Gitlab::BacktraceCleaner.clean_backtrace(backtrace))
       end
     end
   end

spec/lib/gitlab/backtrace_cleaner_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BacktraceCleaner do
+  describe '.clean_backtrace' do
+    it 'uses the Rails backtrace cleaner' do
+      backtrace = []
+
+      expect(Rails.backtrace_cleaner).to receive(:clean).with(backtrace)
+
+      described_class.clean_backtrace(backtrace)
+    end
+
+    it 'removes lines from IGNORE_BACKTRACES' do
+      backtrace = [
+        "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'",
+        "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'",
+        "lib/gitlab/gitaly_client.rb:280:in `block in migrate'",
+        "lib/gitlab/metrics/influx_db.rb:103:in `measure'",
+        "lib/gitlab/gitaly_client.rb:278:in `migrate'",
+        "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'",
+        "lib/gitlab/git/commit.rb:66:in `find'",
+        "app/models/repository.rb:1047:in `find_commit'",
+        "lib/gitlab/metrics/instrumentation.rb:159:in `block in find_commit'",
+        "lib/gitlab/metrics/method_call.rb:36:in `measure'",
+        "lib/gitlab/metrics/instrumentation.rb:159:in `find_commit'",
+        "app/models/repository.rb:113:in `commit'",
+        "lib/gitlab/i18n.rb:50:in `with_locale'",
+        "lib/gitlab/middleware/multipart.rb:95:in `call'",
+        "lib/gitlab/request_profiler/middleware.rb:14:in `call'",
+        "ee/lib/gitlab/database/load_balancing/rack_middleware.rb:37:in `call'",
+        "ee/lib/gitlab/jira/middleware.rb:15:in `call'"
+      ]
+
+      expect(described_class.clean_backtrace(backtrace))
+        .to eq([
+                 "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'",
+                 "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'",
+                 "lib/gitlab/gitaly_client.rb:280:in `block in migrate'",
+                 "lib/gitlab/gitaly_client.rb:278:in `migrate'",
+                 "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'",
+                 "lib/gitlab/git/commit.rb:66:in `find'",
+                 "app/models/repository.rb:1047:in `find_commit'",
+                 "app/models/repository.rb:113:in `commit'",
+                 "ee/lib/gitlab/jira/middleware.rb:15:in `call'"
+               ])
+    end
+  end
+end

spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb

@@ -39,7 +39,7 @@ describe Gitlab::GrapeLogging::Loggers::ExceptionLogger do
         before do
           current_backtrace = caller
           allow(exception).to receive(:backtrace).and_return(current_backtrace)
-          expected['exception.backtrace'] = Gitlab::Profiler.clean_backtrace(current_backtrace)
+          expected['exception.backtrace'] = Gitlab::BacktraceCleaner.clean_backtrace(current_backtrace)
         end
 
         it 'includes the backtrace' do

spec/lib/gitlab/profiler_spec.rb

@@ -120,51 +120,6 @@ describe Gitlab::Profiler do
     end
   end
 
-  describe '.clean_backtrace' do
-    it 'uses the Rails backtrace cleaner' do
-      backtrace = []
-
-      expect(Rails.backtrace_cleaner).to receive(:clean).with(backtrace)
-
-      described_class.clean_backtrace(backtrace)
-    end
-
-    it 'removes lines from IGNORE_BACKTRACES' do
-      backtrace = [
-        "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'",
-        "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'",
-        "lib/gitlab/gitaly_client.rb:280:in `block in migrate'",
-        "lib/gitlab/metrics/influx_db.rb:103:in `measure'",
-        "lib/gitlab/gitaly_client.rb:278:in `migrate'",
-        "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'",
-        "lib/gitlab/git/commit.rb:66:in `find'",
-        "app/models/repository.rb:1047:in `find_commit'",
-        "lib/gitlab/metrics/instrumentation.rb:159:in `block in find_commit'",
-        "lib/gitlab/metrics/method_call.rb:36:in `measure'",
-        "lib/gitlab/metrics/instrumentation.rb:159:in `find_commit'",
-        "app/models/repository.rb:113:in `commit'",
-        "lib/gitlab/i18n.rb:50:in `with_locale'",
-        "lib/gitlab/middleware/multipart.rb:95:in `call'",
-        "lib/gitlab/request_profiler/middleware.rb:14:in `call'",
-        "ee/lib/gitlab/database/load_balancing/rack_middleware.rb:37:in `call'",
-        "ee/lib/gitlab/jira/middleware.rb:15:in `call'"
-      ]
-
-      expect(described_class.clean_backtrace(backtrace))
-        .to eq([
-                 "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'",
-                 "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'",
-                 "lib/gitlab/gitaly_client.rb:280:in `block in migrate'",
-                 "lib/gitlab/gitaly_client.rb:278:in `migrate'",
-                 "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'",
-                 "lib/gitlab/git/commit.rb:66:in `find'",
-                 "app/models/repository.rb:1047:in `find_commit'",
-                 "app/models/repository.rb:113:in `commit'",
-                 "ee/lib/gitlab/jira/middleware.rb:15:in `call'"
-               ])
-    end
-  end
-
   describe '.with_custom_logger' do
     context 'when the logger is set' do
       it 'uses the replacement logger for the duration of the block' do

spec/lib/gitlab/sidekiq_logging/exception_handler_spec.rb

@@ -33,7 +33,7 @@ describe Gitlab::SidekiqLogging::ExceptionHandler do
         error_class: 'RuntimeError',
         error_message: exception_message,
         context: 'Test',
-        error_backtrace: Gitlab::Profiler.clean_backtrace(backtrace)
+        error_backtrace: Gitlab::BacktraceCleaner.clean_backtrace(backtrace)
       )
 
       expect(logger).to receive(:warn).with(expected_data)

spec/support/helpers/query_recorder.rb

@@ -16,7 +16,7 @@ module ActiveRecord
 
     def show_backtrace(values)
       Rails.logger.debug("QueryRecorder SQL: #{values[:sql]}")
-      Gitlab::Profiler.clean_backtrace(caller).each { |line| Rails.logger.debug("   --> #{line}") }
+      Gitlab::BacktraceCleaner.clean_backtrace(caller).each { |line| Rails.logger.debug("   --> #{line}") }
     end
 
     def callback(name, start, finish, message_id, values)