Merge branch 'track-sql-queries-sentry' into 'master'

Fix Sentry tracking of SQL queries

See merge request gitlab-org/gitlab!50413

lib/gitlab/error_tracking.rb

@@ -111,8 +111,8 @@ module Gitlab
       private
 
       def before_send(event, hint)
-        event = add_context_from_exception_type(event, hint)
-        event = custom_fingerprinting(event, hint)
+        inject_context_for_exception(event, hint[:exception])
+        custom_fingerprinting(event, hint[:exception])
 
         event
       end
@@ -123,7 +123,6 @@ module Gitlab
         end
 
         extra = sanitize_request_parameters(extra)
-        inject_sql_query_into_extra(exception, extra)
 
         if sentry && Raven.configuration.server
           Raven.capture_exception(exception, tags: default_tags, extra: extra)
@@ -150,12 +149,6 @@ module Gitlab
         filter.filter(parameters)
       end
 
-      def inject_sql_query_into_extra(exception, extra)
-        return unless exception.is_a?(ActiveRecord::StatementInvalid)
-
-        extra[:sql] = PgQuery.normalize(exception.sql.to_s)
-      end
-
       def sentry_dsn
         return unless Rails.env.production? || Rails.env.development?
         return unless Gitlab.config.sentry.enabled
@@ -183,9 +176,17 @@ module Gitlab
         {}
       end
 
-      # Debugging for https://gitlab.com/gitlab-org/gitlab-foss/issues/57727
-      def add_context_from_exception_type(event, hint)
-        if ActiveModel::MissingAttributeError === hint[:exception]
+      # Group common, mostly non-actionable exceptions by type and message,
+      # rather than cause
+      def custom_fingerprinting(event, ex)
+        return event unless CUSTOM_FINGERPRINTING.include?(ex.class.name)
+
+        event.fingerprint = [ex.class.name, ex.message]
+      end
+
+      def inject_context_for_exception(event, ex)
+        case ex
+        when ActiveModel::MissingAttributeError # Debugging for https://gitlab.com/gitlab-org/gitlab/-/issues/26751
           columns_hash = ActiveRecord::Base
                             .connection
                             .schema_cache
@@ -193,21 +194,11 @@ module Gitlab
                             .transform_values { |v| v.map(&:first) }
 
           event.extra.merge!(columns_hash)
+        when ActiveRecord::StatementInvalid
+          event.extra[:sql] = PgQuery.normalize(ex.sql.to_s)
+        else
+          inject_context_for_exception(event, ex.cause) if ex.cause.present?
         end
-
-        event
-      end
-
-      # Group common, mostly non-actionable exceptions by type and message,
-      # rather than cause
-      def custom_fingerprinting(event, hint)
-        ex = hint[:exception]
-
-        return event unless CUSTOM_FINGERPRINTING.include?(ex.class.name)
-
-        event.fingerprint = [ex.class.name, ex.message]
-
-        event
       end
     end
   end

spec/lib/gitlab/error_tracking_spec.rb

@@ -236,7 +236,7 @@ RSpec.describe Gitlab::ErrorTracking do
 
     context 'the exception implements :sentry_extra_data' do
       let(:extra_info) { { event: 'explosion', size: :massive } }
-      let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info, backtrace: caller) }
+      let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info, backtrace: caller, cause: nil) }
 
       it 'includes the extra data from the exception in the tracking information' do
         track_exception
@@ -247,7 +247,7 @@ RSpec.describe Gitlab::ErrorTracking do
     end
 
     context 'the exception implements :sentry_extra_data, which returns nil' do
-      let(:exception) { double(message: 'bang!', sentry_extra_data: nil, backtrace: caller) }
+      let(:exception) { double(message: 'bang!', sentry_extra_data: nil, backtrace: caller, cause: nil) }
       let(:extra) { { issue_url: issue_url } }
 
       it 'just includes the other extra info' do
@@ -287,10 +287,23 @@ RSpec.describe Gitlab::ErrorTracking do
       let(:exception) { ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1') }
 
       it 'injects the normalized sql query into extra' do
+        allow(Raven.client.transport).to receive(:send_event) do |event|
+          expect(event.extra).to include(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')
+        end
+
         track_exception
+      end
+    end
 
-        expect(Raven).to have_received(:capture_exception)
-          .with(exception, a_hash_including(extra: a_hash_including(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')))
+    context 'when the `ActiveRecord::StatementInvalid` is wrapped in another exception' do
+      let(:exception) { RuntimeError.new(cause: ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1')) }
+
+      it 'injects the normalized sql query into extra' do
+        allow(Raven.client.transport).to receive(:send_event) do |event|
+          expect(event.extra).to include(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')
+        end
+
+        track_exception
       end
     end
   end