Commit 72c44c2f authored by Dmitry Gruzd's avatar Dmitry Gruzd

Merge branch 'move-sentry-processors-to-before-send' into 'master'

Move Sentry processors to before_send hook [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55648
parents c307eab8 5bdcdb97
---
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
...@@ -16,6 +16,12 @@ module Gitlab ...@@ -16,6 +16,12 @@ module Gitlab
Rack::Timeout::RequestTimeoutException Rack::Timeout::RequestTimeoutException
].freeze ].freeze
PROCESSORS = [
::Gitlab::ErrorTracking::Processor::SidekiqProcessor,
::Gitlab::ErrorTracking::Processor::GrpcErrorProcessor,
::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor
].freeze
class << self class << self
def configure def configure
Raven.configure do |config| Raven.configure do |config|
...@@ -97,7 +103,9 @@ module Gitlab ...@@ -97,7 +103,9 @@ module Gitlab
inject_context_for_exception(event, hint[:exception]) inject_context_for_exception(event, hint[:exception])
custom_fingerprinting(event, hint[:exception]) custom_fingerprinting(event, hint[:exception])
event PROCESSORS.reduce(event) do |processed_event, processor|
processor.call(processed_event)
end
end end
def process_exception(exception, sentry: false, logging: true, extra:) def process_exception(exception, sentry: false, logging: true, extra:)
......
...@@ -9,9 +9,21 @@ module Gitlab ...@@ -9,9 +9,21 @@ module Gitlab
# integrations are re-implemented and use Gitlab::ErrorTracking, this # integrations are re-implemented and use Gitlab::ErrorTracking, this
# processor should be removed. # processor should be removed.
def process(payload) def process(payload)
return payload if ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
context_payload = Gitlab::ErrorTracking::ContextPayloadGenerator.generate(nil, {}) context_payload = Gitlab::ErrorTracking::ContextPayloadGenerator.generate(nil, {})
payload.deep_merge!(context_payload) payload.deep_merge!(context_payload)
end 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
event
end
end end
end end
end end
......
...@@ -6,41 +6,50 @@ module Gitlab ...@@ -6,41 +6,50 @@ module Gitlab
class GrpcErrorProcessor < ::Raven::Processor class GrpcErrorProcessor < ::Raven::Processor
DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)') DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)')
def process(value) def process(payload)
process_first_exception_value(value) return payload if ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
process_custom_fingerprint(value)
value 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)
event
end end
# Sentry can report multiple exceptions in an event. Sanitize # Sentry can report multiple exceptions in an event. Sanitize
# only the first one since that's what is used for grouping. # only the first one since that's what is used for grouping.
def process_first_exception_value(value) def process_first_exception_value(event_or_payload)
exceptions = value.dig(:exception, :values) exceptions = exceptions(event_or_payload)
return unless exceptions.is_a?(Array) return unless exceptions.is_a?(Array)
entry = exceptions.first exception = exceptions.first
return unless entry.is_a?(Hash) return unless valid_exception?(exception)
exception_type = entry[:type] exception_type, raw_message = type_and_value(exception)
raw_message = entry[:value]
return unless exception_type&.start_with?('GRPC::') return unless exception_type&.start_with?('GRPC::')
return unless raw_message.present? return unless raw_message.present?
message, debug_str = split_debug_error_string(raw_message) message, debug_str = split_debug_error_string(raw_message)
entry[:value] = message if message set_new_values!(event_or_payload, exception, message, debug_str)
extra = value[:extra] || {}
extra[:grpc_debug_error_string] = debug_str if debug_str
end end
def process_custom_fingerprint(value) def process_custom_fingerprint(event)
fingerprint = value[:fingerprint] fingerprint = fingerprint(event)
return value unless custom_grpc_fingerprint?(fingerprint) return event unless custom_grpc_fingerprint?(fingerprint)
message, _ = split_debug_error_string(fingerprint[1]) message, _ = split_debug_error_string(fingerprint[1])
fingerprint[1] = message if message fingerprint[1] = message if message
...@@ -61,6 +70,63 @@ module Gitlab ...@@ -61,6 +70,63 @@ module Gitlab
[match[1], match[2]] [match[1], match[2]]
end 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
end end
end end
......
...@@ -8,7 +8,8 @@ module Gitlab ...@@ -8,7 +8,8 @@ module Gitlab
class SidekiqProcessor < ::Raven::Processor class SidekiqProcessor < ::Raven::Processor
FILTERED_STRING = '[FILTERED]' FILTERED_STRING = '[FILTERED]'
def self.filter_arguments(args, klass) class << self
def filter_arguments(args, klass)
args.lazy.with_index.map do |arg, i| args.lazy.with_index.map do |arg, i|
case arg case arg
when Numeric when Numeric
...@@ -23,7 +24,7 @@ module Gitlab ...@@ -23,7 +24,7 @@ module Gitlab
end end
end end
def self.permitted_arguments_for_worker(klass) def permitted_arguments_for_worker(klass)
@permitted_arguments_for_worker ||= {} @permitted_arguments_for_worker ||= {}
@permitted_arguments_for_worker[klass] ||= @permitted_arguments_for_worker[klass] ||=
begin begin
...@@ -33,14 +34,40 @@ module Gitlab ...@@ -33,14 +34,40 @@ module Gitlab
end end
end end
def self.loggable_arguments(args, klass) def loggable_arguments(args, klass)
Gitlab::Utils::LogLimitedArray Gitlab::Utils::LogLimitedArray
.log_limited_array(filter_arguments(args, klass)) .log_limited_array(filter_arguments(args, klass))
.map(&:to_s) .map(&:to_s)
.to_a .to_a
end 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
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'] = filter_arguments(job_holder['args'], job_holder['class']).to_a
end
event.extra[:sidekiq] = sidekiq
event
end
end
def process(value, key = nil) def process(value, key = nil)
return value if ::Feature.enabled?(:sentry_processors_before_send, default_enabled: :yaml)
sidekiq = value.dig(:extra, :sidekiq) sidekiq = value.dig(:extra, :sidekiq)
return value unless sidekiq return value unless sidekiq
......
# frozen_string_literal: true # frozen_string_literal: true
require 'fast_spec_helper' require 'spec_helper'
RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do
subject(:processor) { described_class.new } shared_examples 'processing an exception' do
before do before do
allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator| allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator|
allow(generator).to receive(:generate).and_return( allow(generator).to receive(:generate).and_return(
...@@ -15,31 +14,53 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do ...@@ -15,31 +14,53 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do
end end
end end
it 'merges the context payload into event payload' do let(:payload) do
payload = { {
user: { ip_address: '127.0.0.1' }, user: { ip_address: '127.0.0.1' },
tags: { priority: 'high' }, tags: { priority: 'high' },
extra: { sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } } extra: { sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } }
} }
end
processor.process(payload) it 'merges the context payload into event payload', :aggregate_failures do
expect(result_hash[:user]).to eq(ip_address: '127.0.0.1', username: 'root')
expect(payload).to eql( expect(result_hash[:tags])
user: { .to eq(priority: 'high',
ip_address: '127.0.0.1',
username: 'root'
},
tags: {
priority: 'high',
locale: 'en', locale: 'en',
program: 'test', program: 'test',
feature_category: 'feature_a', feature_category: 'feature_a',
correlation_id: 'cid' correlation_id: 'cid')
},
extra: { expect(result_hash[:extra])
some_info: 'info', .to include(some_info: 'info',
sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } 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
end end
...@@ -3,73 +3,83 @@ ...@@ -3,73 +3,83 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do
describe '#process' do shared_examples 'processing an exception' do
subject { described_class.new }
context 'when there is no GRPC exception' do context 'when there is no GRPC exception' do
let(:exception) { RuntimeError.new }
let(:data) { { fingerprint: ['ArgumentError', 'Missing arguments'] } } let(:data) { { fingerprint: ['ArgumentError', 'Missing arguments'] } }
it 'leaves data unchanged' do it 'leaves data unchanged' do
expect(subject.process(data)).to eq(data) expect(result_hash).to include(data)
end end
end end
context 'when there is a GPRC exception with a debug string' do context 'when there is a GPRC exception with a debug string' do
let(:exception) { GRPC::DeadlineExceeded.new('Deadline Exceeded', {}, '{"hello":1}') }
let(:data) do let(:data) do
{ {
exception: {
values: [
{
type: "GRPC::DeadlineExceeded",
value: "4:DeadlineExceeded. debug_error_string:{\"hello\":1}"
}
]
},
extra: { extra: {
caller: 'test' caller: 'test'
}, },
fingerprint: [ fingerprint: [
"GRPC::DeadlineExceeded", 'GRPC::DeadlineExceeded',
"4:Deadline Exceeded. debug_error_string:{\"created\":\"@1598938192.005782000\",\"description\":\"Error received from peer unix:/home/git/gitalypraefect.socket\",\"file\":\"src/core/lib/surface/call.cc\",\"file_line\":1055,\"grpc_message\":\"Deadline Exceeded\",\"grpc_status\":4}" '4:Deadline Exceeded. debug_error_string:{"created":"@1598938192.005782000","description":"Error received from peer unix:/home/git/gitalypraefect.socket","file":"src/core/lib/surface/call.cc","file_line":1055,"grpc_message":"Deadline Exceeded","grpc_status":4}'
] ]
} }
end end
let(:expected) do
{
fingerprint: [
"GRPC::DeadlineExceeded",
"4:Deadline Exceeded."
],
exception: {
values: [
{
type: "GRPC::DeadlineExceeded",
value: "4:DeadlineExceeded."
}
]
},
extra: {
caller: 'test',
grpc_debug_error_string: "{\"hello\":1}"
}
}
end
it 'removes the debug error string and stores it as an extra field' do it 'removes the debug error string and stores it as an extra field' do
expect(subject.process(data)).to eq(expected) expect(result_hash[:fingerprint])
.to eq(['GRPC::DeadlineExceeded', '4:Deadline Exceeded.'])
expect(result_hash[:exception][:values].first)
.to include(type: 'GRPC::DeadlineExceeded', value: '4:Deadline Exceeded.')
expect(result_hash[:extra])
.to include(caller: 'test', grpc_debug_error_string: '{"hello":1}')
end end
context 'with no custom fingerprint' do context 'with no custom fingerprint' do
before do let(:data) do
data.delete(:fingerprint) { extra: { caller: 'test' } }
expected.delete(:fingerprint)
end end
it 'removes the debug error string and stores it as an extra field' do it 'removes the debug error string and stores it as an extra field' do
expect(subject.process(data)).to eq(expected) expect(result_hash).not_to include(:fingerprint)
expect(result_hash[:exception][:values].first)
.to include(type: 'GRPC::DeadlineExceeded', value: '4:Deadline Exceeded.')
expect(result_hash[:extra])
.to include(caller: 'test', grpc_debug_error_string: '{"hello":1}')
end end
end end
end 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 end
...@@ -94,28 +94,37 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do ...@@ -94,28 +94,37 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do
end end
end end
describe '#process' do shared_examples 'processing an exception' do
context 'when there is Sidekiq data' do context 'when there is Sidekiq data' do
let(:wrapped_value) { { extra: { sidekiq: value } } }
shared_examples 'Sidekiq arguments' do |args_in_job_hash: true| shared_examples 'Sidekiq arguments' do |args_in_job_hash: true|
let(:path) { [:extra, :sidekiq, args_in_job_hash ? :job : nil, 'args'].compact } let(:path) { [:extra, :sidekiq, args_in_job_hash ? :job : nil, 'args'].compact }
let(:args) { [1, 'string', { a: 1 }, [1, 2]] } let(:args) { [1, 'string', { a: 1 }, [1, 2]] }
it 'only allows numeric arguments for an unknown worker' do context 'for an unknown worker' do
value = { 'args' => args, 'class' => 'UnknownWorker' } let(:value) do
hash = { 'args' => args, 'class' => 'UnknownWorker' }
value = { job: value } if args_in_job_hash args_in_job_hash ? { job: hash } : hash
end
expect(subject.process(extra_sidekiq(value)).dig(*path)) it 'only allows numeric arguments for an unknown worker' do
expect(result_hash.dig(*path))
.to eq([1, described_class::FILTERED_STRING, described_class::FILTERED_STRING, described_class::FILTERED_STRING]) .to eq([1, described_class::FILTERED_STRING, described_class::FILTERED_STRING, described_class::FILTERED_STRING])
end end
end
it 'allows all argument types for a permitted worker' do context 'for a permitted worker' do
value = { 'args' => args, 'class' => 'PostReceive' } let(:value) do
hash = { 'args' => args, 'class' => 'PostReceive' }
value = { job: value } if args_in_job_hash args_in_job_hash ? { job: hash } : hash
end
expect(subject.process(extra_sidekiq(value)).dig(*path)) it 'allows all argument types for a permitted worker' do
.to eq(args) expect(result_hash.dig(*path)).to eq(args)
end
end end
end end
...@@ -127,39 +136,62 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do ...@@ -127,39 +136,62 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do
include_examples 'Sidekiq arguments', args_in_job_hash: false include_examples 'Sidekiq arguments', args_in_job_hash: false
end end
it 'removes a jobstr field if present' do context 'when a jobstr field is present' do
value = { let(:value) do
{
job: { 'args' => [1] }, job: { 'args' => [1] },
jobstr: { 'args' => [1] }.to_json jobstr: { 'args' => [1] }.to_json
} }
end
expect(subject.process(extra_sidekiq(value))) it 'removes the jobstr' do
.to eq(extra_sidekiq(value.except(:jobstr))) expect(result_hash.dig(:extra, :sidekiq)).to eq(value.except(:jobstr))
end
end end
it 'does nothing with no jobstr' do context 'when no jobstr value is present' do
value = { job: { 'args' => [1] } } let(:value) { { job: { 'args' => [1] } } }
expect(subject.process(extra_sidekiq(value))) it 'does nothing' do
.to eq(extra_sidekiq(value)) expect(result_hash.dig(:extra, :sidekiq)).to eq(value)
end
end end
end end
context 'when there is no Sidekiq data' do context 'when there is no Sidekiq data' do
let(:value) { { tags: { foo: 'bar', baz: 'quux' } } }
let(:wrapped_value) { value }
it 'does nothing' do it 'does nothing' do
value = { expect(result_hash).to include(value)
request: { expect(result_hash.dig(:extra, :sidekiq)).to be_nil
method: 'POST', end
data: { 'key' => 'value' } end
} end
}
describe '.call' do
let(:event) { Raven::Event.new(wrapped_value) }
let(:result_hash) { described_class.call(event).to_hash }
expect(subject.process(value)).to eq(value) 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
end end
def extra_sidekiq(hash) describe '#process' do
{ extra: { sidekiq: hash } } 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 end
end end
...@@ -7,6 +7,7 @@ require 'raven/transports/dummy' ...@@ -7,6 +7,7 @@ require 'raven/transports/dummy'
RSpec.describe Gitlab::ErrorTracking do RSpec.describe Gitlab::ErrorTracking do
let(:exception) { RuntimeError.new('boom') } let(:exception) { RuntimeError.new('boom') }
let(:issue_url) { 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1' } let(:issue_url) { 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1' }
let(:extra) { { issue_url: issue_url, some_other_info: 'info' } }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -42,6 +43,8 @@ RSpec.describe Gitlab::ErrorTracking do ...@@ -42,6 +43,8 @@ RSpec.describe Gitlab::ErrorTracking do
} }
end end
let(:sentry_event) { Gitlab::Json.parse(Raven.client.transport.events.last[1]) }
before do before do
stub_sentry_settings stub_sentry_settings
...@@ -133,8 +136,6 @@ RSpec.describe Gitlab::ErrorTracking do ...@@ -133,8 +136,6 @@ RSpec.describe Gitlab::ErrorTracking do
end end
describe '.track_exception' do describe '.track_exception' do
let(:extra) { { issue_url: issue_url, some_other_info: 'info' } }
subject(:track_exception) { described_class.track_exception(exception, extra) } subject(:track_exception) { described_class.track_exception(exception, extra) }
before do before do
...@@ -195,6 +196,55 @@ RSpec.describe Gitlab::ErrorTracking do ...@@ -195,6 +196,55 @@ RSpec.describe Gitlab::ErrorTracking do
end end
end end
context 'when the error is kind of an `ActiveRecord::StatementInvalid`' 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
track_exception
expect(sentry_event.dig('extra', 'sql')).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')
end
end
context 'when the `ActiveRecord::StatementInvalid` is wrapped in another exception' do
it 'injects the normalized sql query into extra' do
allow(exception).to receive(:cause).and_return(ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1'))
track_exception
expect(sentry_event.dig('extra', 'sql')).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')
end
end
end
shared_examples 'event processors' do
subject(:track_exception) { described_class.track_exception(exception, extra) }
before do
allow(Raven).to receive(:capture_exception).and_call_original
allow(Gitlab::ErrorTracking::Logger).to receive(:error)
end
context 'custom GitLab context when using Raven.capture_exception directly' do
subject(:raven_capture_exception) { Raven.capture_exception(exception) }
it 'merges a default set of tags into the existing tags' do
allow(Raven.context).to receive(:tags).and_return(foo: 'bar')
raven_capture_exception
expect(sentry_event['tags']).to include('correlation_id', 'feature_category', 'foo', 'locale', 'program')
end
it 'merges the current user information into the existing user information' do
Raven.user_context(id: -1)
raven_capture_exception
expect(sentry_event['user']).to eq('id' => -1, 'username' => user.username)
end
end
context 'with sidekiq args' do context 'with sidekiq args' do
context 'when the args does not have anything sensitive' do context 'when the args does not have anything sensitive' do
let(:extra) { { sidekiq: { 'class' => 'PostReceive', 'args' => [1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'] } } } let(:extra) { { sidekiq: { 'class' => 'PostReceive', 'args' => [1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'] } } }
...@@ -211,16 +261,20 @@ RSpec.describe Gitlab::ErrorTracking do ...@@ -211,16 +261,20 @@ RSpec.describe Gitlab::ErrorTracking do
) )
) )
end end
it 'does not filter parameters when sending to Sentry' do
track_exception
expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq([1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'])
end
end end
context 'when the args has sensitive information' do context 'when the args has sensitive information' do
let(:extra) { { sidekiq: { 'class' => 'UnknownWorker', 'args' => ['sensitive string', 1, 2] } } } let(:extra) { { sidekiq: { 'class' => 'UnknownWorker', 'args' => ['sensitive string', 1, 2] } } }
it 'filters sensitive arguments before sending' do it 'filters sensitive arguments before sending and logging' do
track_exception track_exception
sentry_event = Gitlab::Json.parse(Raven.client.transport.events.last[1])
expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2])
expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with(
hash_including( hash_including(
...@@ -234,28 +288,44 @@ RSpec.describe Gitlab::ErrorTracking do ...@@ -234,28 +288,44 @@ RSpec.describe Gitlab::ErrorTracking do
end end
end end
context 'when the error is kind of an `ActiveRecord::StatementInvalid`' do context 'when the error is a GRPC error' do
let(:exception) { ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1') } context 'when the GRPC error contains a debug_error_string value' do
let(:exception) { GRPC::DeadlineExceeded.new('unknown cause', {}, '{"hello":1}') }
it 'injects the normalized sql query into extra' do it 'sets the GRPC debug error string in the Sentry event and adds a custom fingerprint' do
allow(Raven.client.transport).to receive(:send_event) do |event| track_exception
expect(event.extra).to include(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1')
expect(sentry_event.dig('extra', 'grpc_debug_error_string')).to eq('{"hello":1}')
expect(sentry_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.'])
end end
end
context 'when the GRPC error does not contain a debug_error_string value' do
let(:exception) { GRPC::DeadlineExceeded.new }
it 'does not do any processing on the event' do
track_exception track_exception
expect(sentry_event['extra']).not_to include('grpc_debug_error_string')
expect(sentry_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause'])
end
end
end end
end end
context 'when the `ActiveRecord::StatementInvalid` is wrapped in another exception' do context 'with sentry_processors_before_send enabled' do
let(:exception) { RuntimeError.new(cause: ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1')) } before do
stub_feature_flags(sentry_processors_before_send: true)
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 end
track_exception include_examples 'event processors'
end end
context 'with sentry_processors_before_send disabled' do
before do
stub_feature_flags(sentry_processors_before_send: false)
end end
include_examples 'event processors'
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment