Merge branch 'remove-sentry-processors-flag' into 'master'

Remove sentry_processors_before_send feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59565

changelogs/unreleased/remove-sentry-processors-flag.yml

+---
+title: Make Sentry processors for GitLab-internal error tracking compatible with new
+  version of Sentry gem
+merge_request: 59565
+author:
+type: other

config/feature_flags/development/sentry_processors_before_send.yml

----
-name: sentry_processors_before_send
-introduced_by_url:
-rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/849#processors
-milestone: '13.11'
-type: development
-group: team::Scalability
-default_enabled: false

lib/gitlab/error_tracking.rb

@@ -31,9 +31,6 @@ module Gitlab
 
           # Sanitize fields based on those sanitized from Rails.
           config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s)
-          config.processors << ::Gitlab::ErrorTracking::Processor::SidekiqProcessor
-          config.processors << ::Gitlab::ErrorTracking::Processor::GrpcErrorProcessor
-          config.processors << ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor
 
           # Sanitize authentication headers
           config.sanitize_http_headers = %w[Authorization Private-Token]

lib/gitlab/error_tracking/processor/context_payload_processor.rb

@@ -3,21 +3,12 @@
 module Gitlab
   module ErrorTracking
     module Processor
-      class ContextPayloadProcessor < ::Raven::Processor
+      module ContextPayloadProcessor
         # This processor is added to inject application context into Sentry
         # events generated by Sentry built-in integrations. When the
         # integrations are re-implemented and use Gitlab::ErrorTracking, this
         # processor should be removed.
-        def process(payload)
-          return payload if ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
-
-          context_payload = Gitlab::ErrorTracking::ContextPayloadGenerator.generate(nil, {})
-          payload.deep_merge!(context_payload)
-        end
-
         def self.call(event)
-          return event unless ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
-
           Gitlab::ErrorTracking::ContextPayloadGenerator.generate(nil, {}).each do |key, value|
             event.public_send(key).deep_merge!(value) # rubocop:disable GitlabSecurity/PublicSend
           end

lib/gitlab/error_tracking/processor/grpc_error_processor.rb

@@ -3,22 +3,11 @@
 module Gitlab
   module ErrorTracking
     module Processor
-      class GrpcErrorProcessor < ::Raven::Processor
+      module GrpcErrorProcessor
         DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)')
 
-        def process(payload)
-          return payload if ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
-
-          self.class.process_first_exception_value(payload)
-          self.class.process_custom_fingerprint(payload)
-
-          payload
-        end
-
         class << self
           def call(event)
-            return event unless ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
-
             process_first_exception_value(event)
             process_custom_fingerprint(event)
 
@@ -27,8 +16,9 @@ module Gitlab
 
           # Sentry can report multiple exceptions in an event. Sanitize
           # only the first one since that's what is used for grouping.
-          def process_first_exception_value(event_or_payload)
-            exceptions = exceptions(event_or_payload)
+          def process_first_exception_value(event)
+            # Better in new version, will be event.exception.values
+            exceptions = event.instance_variable_get(:@interfaces)[:exception]&.values
 
             return unless exceptions.is_a?(Array)
 
@@ -36,18 +26,21 @@ module Gitlab
 
             return unless valid_exception?(exception)
 
-            exception_type, raw_message = type_and_value(exception)
+            raw_message = exception.value
 
-            return unless exception_type&.start_with?('GRPC::')
+            return unless exception.type&.start_with?('GRPC::')
             return unless raw_message.present?
 
             message, debug_str = split_debug_error_string(raw_message)
 
-            set_new_values!(event_or_payload, exception, message, debug_str)
+            # Worse in new version, no setter! Have to poke at the
+            # instance variable
+            exception.value = message if message
+            event.extra[:grpc_debug_error_string] = debug_str if debug_str
           end
 
           def process_custom_fingerprint(event)
-            fingerprint = fingerprint(event)
+            fingerprint = event.fingerprint
 
             return event unless custom_grpc_fingerprint?(fingerprint)
 
@@ -71,61 +64,14 @@ module Gitlab
             [match[1], match[2]]
           end
 
-          # The below methods can be removed once we remove the
-          # sentry_processors_before_send feature flag, and we can
-          # assume we always have an Event object
-          def exceptions(event_or_payload)
-            case event_or_payload
-            when Raven::Event
-              # Better in new version, will be event_or_payload.exception.values
-              event_or_payload.instance_variable_get(:@interfaces)[:exception]&.values
-            when Hash
-              event_or_payload.dig(:exception, :values)
-            end
-          end
-
           def valid_exception?(exception)
             case exception
             when Raven::SingleExceptionInterface
               exception&.value
-            when Hash
-              true
             else
               false
             end
           end
-
-          def type_and_value(exception)
-            case exception
-            when Raven::SingleExceptionInterface
-              [exception.type, exception.value]
-            when Hash
-              exception.values_at(:type, :value)
-            end
-          end
-
-          def set_new_values!(event_or_payload, exception, message, debug_str)
-            case event_or_payload
-            when Raven::Event
-              # Worse in new version, no setter! Have to poke at the
-              # instance variable
-              exception.value = message if message
-              event_or_payload.extra[:grpc_debug_error_string] = debug_str if debug_str
-            when Hash
-              exception[:value] = message if message
-              extra = event_or_payload[:extra] || {}
-              extra[:grpc_debug_error_string] = debug_str if debug_str
-            end
-          end
-
-          def fingerprint(event_or_payload)
-            case event_or_payload
-            when Raven::Event
-              event_or_payload.fingerprint
-            when Hash
-              event_or_payload[:fingerprint]
-            end
-          end
         end
       end
     end

lib/gitlab/error_tracking/processor/sidekiq_processor.rb

@@ -5,7 +5,7 @@ require 'set'
 module Gitlab
   module ErrorTracking
     module Processor
-      class SidekiqProcessor < ::Raven::Processor
+      module SidekiqProcessor
         FILTERED_STRING = '[FILTERED]'
 
         class << self
@@ -42,8 +42,6 @@ module Gitlab
           end
 
           def call(event)
-            return event unless ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
-
             sidekiq = event&.extra&.dig(:sidekiq)
 
             return event unless sidekiq
@@ -64,29 +62,6 @@ module Gitlab
             event
           end
         end
-
-        def process(value, key = nil)
-          return value if ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
-
-          sidekiq = value.dig(:extra, :sidekiq)
-
-          return value unless sidekiq
-
-          sidekiq = sidekiq.deep_dup
-          sidekiq.delete(:jobstr)
-
-          # 'args' in this hash => from Gitlab::ErrorTracking.track_*
-          # 'args' in :job => from default error handler
-          job_holder = sidekiq.key?('args') ? sidekiq : sidekiq[:job]
-
-          if job_holder['args']
-            job_holder['args'] = self.class.filter_arguments(job_holder['args'], job_holder['class']).to_a
-          end
-
-          value[:extra][:sidekiq] = sidekiq
-
-          value
-        end
       end
     end
   end

spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb

@@ -3,7 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do
-  shared_examples 'processing an exception' do
+  describe '.call' do
+    let(:event) { Raven::Event.new(payload) }
+    let(:result_hash) { described_class.call(event).to_hash }
+
     before do
       allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator|
         allow(generator).to receive(:generate).and_return(
@@ -37,30 +40,4 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do
                     sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] })
     end
   end
-
-  describe '.call' do
-    let(:event) { Raven::Event.new(payload) }
-    let(:result_hash) { described_class.call(event).to_hash }
-
-    it_behaves_like 'processing an exception'
-
-    context 'when followed by #process' do
-      let(:result_hash) { described_class.new.process(described_class.call(event).to_hash) }
-
-      it_behaves_like 'processing an exception'
-    end
-  end
-
-  describe '#process' do
-    let(:event) { Raven::Event.new(payload) }
-    let(:result_hash) { described_class.new.process(event.to_hash) }
-
-    context 'with sentry_processors_before_send disabled' do
-      before do
-        stub_feature_flags(sentry_processors_before_send: false)
-      end
-
-      it_behaves_like 'processing an exception'
-    end
-  end
 end

spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb

@@ -3,7 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do
-  shared_examples 'processing an exception' do
+  describe '.call' do
+    let(:event) { Raven::Event.from_exception(exception, data) }
+    let(:result_hash) { described_class.call(event).to_hash }
+
     context 'when there is no GRPC exception' do
       let(:exception) { RuntimeError.new }
       let(:data) { { fingerprint: ['ArgumentError', 'Missing arguments'] } }
@@ -56,30 +59,4 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do
       end
     end
   end
-
-  describe '.call' do
-    let(:event) { Raven::Event.from_exception(exception, data) }
-    let(:result_hash) { described_class.call(event).to_hash }
-
-    it_behaves_like 'processing an exception'
-
-    context 'when followed by #process' do
-      let(:result_hash) { described_class.new.process(described_class.call(event).to_hash) }
-
-      it_behaves_like 'processing an exception'
-    end
-  end
-
-  describe '#process' do
-    let(:event) { Raven::Event.from_exception(exception, data) }
-    let(:result_hash) { described_class.new.process(event.to_hash) }
-
-    context 'with sentry_processors_before_send disabled' do
-      before do
-        stub_feature_flags(sentry_processors_before_send: false)
-      end
-
-      it_behaves_like 'processing an exception'
-    end
-  end
 end

spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb

@@ -94,7 +94,10 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do
     end
   end
 
-  shared_examples 'processing an exception' do
+  describe '.call' do
+    let(:event) { Raven::Event.new(wrapped_value) }
+    let(:result_hash) { described_class.call(event).to_hash }
+
     context 'when there is Sidekiq data' do
       let(:wrapped_value) { { extra: { sidekiq: value } } }
 
@@ -168,30 +171,4 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do
       end
     end
   end
-
-  describe '.call' do
-    let(:event) { Raven::Event.new(wrapped_value) }
-    let(:result_hash) { described_class.call(event).to_hash }
-
-    it_behaves_like 'processing an exception'
-
-    context 'when followed by #process' do
-      let(:result_hash) { described_class.new.process(described_class.call(event).to_hash) }
-
-      it_behaves_like 'processing an exception'
-    end
-  end
-
-  describe '#process' do
-    let(:event) { Raven::Event.new(wrapped_value) }
-    let(:result_hash) { described_class.new.process(event.to_hash) }
-
-    context 'with sentry_processors_before_send disabled' do
-      before do
-        stub_feature_flags(sentry_processors_before_send: false)
-      end
-
-      it_behaves_like 'processing an exception'
-    end
-  end
 end

spec/lib/gitlab/error_tracking_spec.rb

@@ -217,7 +217,7 @@ RSpec.describe Gitlab::ErrorTracking do
     end
   end
 
-  shared_examples 'event processors' do
+  context 'event processors' do
     subject(:track_exception) { described_class.track_exception(exception, extra) }
 
     before do
@@ -312,20 +312,4 @@ RSpec.describe Gitlab::ErrorTracking do
       end
     end
   end
-
-  context 'with sentry_processors_before_send enabled' do
-    before do
-      stub_feature_flags(sentry_processors_before_send: true)
-    end
-
-    include_examples 'event processors'
-  end
-
-  context 'with sentry_processors_before_send disabled' do
-    before do
-      stub_feature_flags(sentry_processors_before_send: false)
-    end
-
-    include_examples 'event processors'
-  end
 end