Make performance bar enabled checks consistent

Previously, we called the `peek_enabled?` method like so:

    prepend_before_action :set_peek_request_id, if: :peek_enabled?

Now we don't have a `set_peek_request_id` method, so we don't need that
line. However, the `peek_enabled?` part had a side-effect: it would also
populate the request store cache for whether the performance bar was
enabled for the current request or not.

This commit makes that side-effect explicit, and replaces all uses of
`peek_enabled?` with the more explicit
`Gitlab::PerformanceBar.enabled_for_request?`. There is one spec that
still sets `SafeRequestStore[:peek_enabled]` directly, because it is
contrasting behaviour with and without a request store enabled.

The upshot is:

1. We still set the value in one place. We make it more explicit that
   that's what we're doing.
2. Reading that value uses a consistent method so it's easier to find in
   future.

6 files changed, 12 insertions, 20 deletions

diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index 201db9fec26..0aba0ff2bb2 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -157,7 +157,7 @@ module Gitlab
 
       # Keep track, separately, for the performance bar
       self.query_time += duration
-      if peek_enabled?
+      if Gitlab::PerformanceBar.enabled_for_request?
         add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc,
                          backtrace: Gitlab::Profiler.clean_backtrace(caller))
       end
@@ -335,17 +335,13 @@ module Gitlab
       Gitlab::SafeRequestStore["gitaly_call_permitted"] = 0
     end
 
-    def self.peek_enabled?
-      Gitlab::SafeRequestStore[:peek_enabled]
-    end
-
     def self.add_call_details(details)
       Gitlab::SafeRequestStore['gitaly_call_details'] ||= []
       Gitlab::SafeRequestStore['gitaly_call_details'] << details
     end
 
     def self.list_call_details
-      return [] unless peek_enabled?
+      return [] unless Gitlab::PerformanceBar.enabled_for_request?
 
       Gitlab::SafeRequestStore['gitaly_call_details'] || []
     end
diff --git a/lib/gitlab/performance_bar.rb b/lib/gitlab/performance_bar.rb
index 07439d8e011..68af290d069 100644
--- a/lib/gitlab/performance_bar.rb
+++ b/lib/gitlab/performance_bar.rb
@@ -6,7 +6,11 @@ module Gitlab
     EXPIRY_TIME_L1_CACHE = 1.minute
     EXPIRY_TIME_L2_CACHE = 5.minutes
 
-    def self.enabled?(user = nil)
+    def self.enabled_for_request?
+      Gitlab::SafeRequestStore[:peek_enabled]
+    end
+
+    def self.enabled_for_user?(user = nil)
       return true if Rails.env.development?
       return true if user&.admin?
       return false unless user && allowed_group_id
diff --git a/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb b/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb
index 9595ced0177..cddd4f18cc3 100644
--- a/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb
+++ b/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb
@@ -5,7 +5,7 @@ module Gitlab
   module PerformanceBar
     module RedisAdapterWhenPeekEnabled
       def save(request_id)
-        super if ::Peek.enabled? && request_id.present?
+        super if ::Gitlab::PerformanceBar.enabled_for_request? && request_id.present?
       end
     end
   end
diff --git a/lib/gitlab/rugged_instrumentation.rb b/lib/gitlab/rugged_instrumentation.rb
index 8bb8c547ae1..c2b55431547 100644
--- a/lib/gitlab/rugged_instrumentation.rb
+++ b/lib/gitlab/rugged_instrumentation.rb
@@ -27,19 +27,15 @@ module Gitlab
       SafeRequestStore.active?
     end
 
-    def self.peek_enabled?
-      SafeRequestStore[:peek_enabled]
-    end
-
     def self.add_call_details(details)
-      return unless peek_enabled?
+      return unless Gitlab::PerformanceBar.enabled_for_request?
 
       Gitlab::SafeRequestStore[:rugged_call_details] ||= []
       Gitlab::SafeRequestStore[:rugged_call_details] << details
     end
 
     def self.list_call_details
-      return [] unless peek_enabled?
+      return [] unless Gitlab::PerformanceBar.enabled_for_request?
 
       Gitlab::SafeRequestStore[:rugged_call_details] || []
     end
diff --git a/lib/peek/views/active_record.rb b/lib/peek/views/active_record.rb
index 2d78818630d..250c7d6aa8f 100644
--- a/lib/peek/views/active_record.rb
+++ b/lib/peek/views/active_record.rb
@@ -9,7 +9,7 @@ module Peek
         super
 
         subscribe('sql.active_record') do |_, start, finish, _, data|
-          if Gitlab::SafeRequestStore.store[:peek_enabled]
+          if Gitlab::PerformanceBar.enabled_for_request?
             unless data[:cached]
               detail_store << {
                 duration: finish - start,
diff --git a/lib/peek/views/redis_detailed.rb b/lib/peek/views/redis_detailed.rb
index f36f581d5e9..84041b6be73 100644
--- a/lib/peek/views/redis_detailed.rb
+++ b/lib/peek/views/redis_detailed.rb
@@ -16,7 +16,7 @@ module Gitlab
       private
 
       def add_call_details(duration, args)
-        return unless peek_enabled?
+        return unless Gitlab::PerformanceBar.enabled_for_request?
         # redis-rb passes an array (e.g. [:get, key])
         return unless args.length == 1
 
@@ -27,10 +27,6 @@ module Gitlab
         }
       end
 
-      def peek_enabled?
-        Gitlab::SafeRequestStore.store[:peek_enabled]
-      end
-
       def detail_store
         ::Gitlab::SafeRequestStore['redis_call_details'] ||= []
       end