Remove Sentry reporting for query limiting
Using Sentry, while useful, poses two problems you have to choose from: 1. All errors are reported separately, making it easy to create issues but also making it next to impossible to see other errors (due to the sheer volume of threshold errors). 2. Errors can be grouped or merged together, reducing the noise. This however also means it's (as far as I can tell) much harder to automatically create GitLab issues from Sentry for the offending controllers. Since both solutions are terrible I decided to go with a third option: not using Sentry for this at all. Instead we'll investigate using Prometheus alerts and Grafana dashboards for this, which has the added benefit of being able to more accurately measure the behaviour over time. Note that throwing errors in test environments is still enabled, and whitelisting is still necessary to prevent that from happening (and that in turn still requires that developers create issues).
This commit is contained in:
parent
4e846c735f
commit
e3bd674e81
|
@ -1,8 +1,7 @@
|
||||||
# Query Count Limits
|
# Query Count Limits
|
||||||
|
|
||||||
Each controller or API endpoint is allowed to execute up to 100 SQL queries. In
|
Each controller or API endpoint is allowed to execute up to 100 SQL queries and
|
||||||
a production environment we'll only log an error in case this threshold is
|
in test environments we'll raise an error when this threshold is exceeded.
|
||||||
exceeded, but in a test environment we'll raise an error instead.
|
|
||||||
|
|
||||||
## Solving Failing Tests
|
## Solving Failing Tests
|
||||||
|
|
||||||
|
|
|
@ -51,13 +51,7 @@ module Gitlab
|
||||||
|
|
||||||
error = ThresholdExceededError.new(error_message)
|
error = ThresholdExceededError.new(error_message)
|
||||||
|
|
||||||
if raise_error?
|
raise(error) if raise_error?
|
||||||
raise(error)
|
|
||||||
else
|
|
||||||
# Raven automatically logs to the Rails log if disabled, thus we don't
|
|
||||||
# need to manually log anything in case Sentry support is not enabled.
|
|
||||||
Raven.capture_exception(error)
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def increment
|
def increment
|
||||||
|
|
|
@ -59,18 +59,6 @@ describe Gitlab::QueryLimiting::Transaction do
|
||||||
expect { transaction.act_upon_results }
|
expect { transaction.act_upon_results }
|
||||||
.to raise_error(described_class::ThresholdExceededError)
|
.to raise_error(described_class::ThresholdExceededError)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'reports the error in Sentry if raising an error is disabled' do
|
|
||||||
expect(transaction)
|
|
||||||
.to receive(:raise_error?)
|
|
||||||
.and_return(false)
|
|
||||||
|
|
||||||
expect(Raven)
|
|
||||||
.to receive(:capture_exception)
|
|
||||||
.with(an_instance_of(described_class::ThresholdExceededError))
|
|
||||||
|
|
||||||
transaction.act_upon_results
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue