From e3bd674e81d565ef3bd399a70b3039316b518693 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 13 Feb 2018 16:40:23 +0100
Subject: [PATCH] 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).
---
 doc/development/query_count_limits.md              |  5 ++---
 lib/gitlab/query_limiting/transaction.rb           |  8 +-------
 spec/lib/gitlab/query_limiting/transaction_spec.rb | 12 ------------
 3 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/doc/development/query_count_limits.md b/doc/development/query_count_limits.md
index ebb6e0c2dac..310e3faf61b 100644
--- a/doc/development/query_count_limits.md
+++ b/doc/development/query_count_limits.md
@@ -1,8 +1,7 @@
 # Query Count Limits
 
-Each controller or API endpoint is allowed to execute up to 100 SQL queries. In
-a production environment we'll only log an error in case this threshold is
-exceeded, but in a test environment we'll raise an error instead.
+Each controller or API endpoint is allowed to execute up to 100 SQL queries and
+in test environments we'll raise an error when this threshold is exceeded.
 
 ## Solving Failing Tests
 
diff --git a/lib/gitlab/query_limiting/transaction.rb b/lib/gitlab/query_limiting/transaction.rb
index 3cbafadb0d0..66d7d9275cf 100644
--- a/lib/gitlab/query_limiting/transaction.rb
+++ b/lib/gitlab/query_limiting/transaction.rb
@@ -51,13 +51,7 @@ module Gitlab
 
         error = ThresholdExceededError.new(error_message)
 
-        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
+        raise(error) if raise_error?
       end
 
       def increment
diff --git a/spec/lib/gitlab/query_limiting/transaction_spec.rb b/spec/lib/gitlab/query_limiting/transaction_spec.rb
index b4231fcd0fa..b72b8574174 100644
--- a/spec/lib/gitlab/query_limiting/transaction_spec.rb
+++ b/spec/lib/gitlab/query_limiting/transaction_spec.rb
@@ -59,18 +59,6 @@ describe Gitlab::QueryLimiting::Transaction do
         expect { transaction.act_upon_results }
           .to raise_error(described_class::ThresholdExceededError)
       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