From d4d06bded14bf66b341b67a7771f5f2d1d9f400c Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 27 Nov 2017 08:28:59 +0100
Subject: [PATCH] Keep track of storage check timings

---
 .../monitoring/prometheus/gitlab_metrics.md   |  3 ++-
 lib/gitlab/git/storage/checker.rb             | 24 ++++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md
index 11d5e077a36..f495990d9a4 100644
--- a/doc/administration/monitoring/prometheus/gitlab_metrics.md
+++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md
@@ -45,8 +45,9 @@ In this experimental phase, only a few metrics are available:
 | redis_ping_success                | Gauge     | 9.4   | Whether or not the last redis ping succeeded |
 | redis_ping_latency_seconds        | Gauge     | 9.4   | Round trip time of the redis ping |
 | user_session_logins_total         | Counter   | 9.4   | Counter of how many users have logged in |
-| filesystem_circuitbreaker_latency_seconds | Histogram | 9.5 | Latency of the stat check the circuitbreaker uses to probe a shard |
+| filesystem_circuitbreaker_latency_seconds | Gauge | 9.5 | Time spent validating if a storage is accessible |
 | filesystem_circuitbreaker         | Gauge     | 9.5   | Wether or not the circuit for a certain shard is broken or not |
+| circuitbreaker_storage_check_duration_seconds | Histogram | 10.3 | Time a single storage probe took |
 
 ## Metrics shared directory
 
diff --git a/lib/gitlab/git/storage/checker.rb b/lib/gitlab/git/storage/checker.rb
index de63cb4b40c..d3c37f82101 100644
--- a/lib/gitlab/git/storage/checker.rb
+++ b/lib/gitlab/git/storage/checker.rb
@@ -5,6 +5,8 @@ module Gitlab
         include CircuitBreakerSettings
 
         attr_reader :storage_path, :storage, :hostname, :logger
+        METRICS_MUTEX = Mutex.new
+        STORAGE_TIMING_BUCKETS = [0.1, 0.15, 0.25, 0.33, 0.5, 1, 1.5, 2.5, 5, 10, 15].freeze
 
         def self.check_all(logger = Rails.logger)
           threads = Gitlab.config.repositories.storages.keys.map do |storage_name|
@@ -19,6 +21,17 @@ module Gitlab
           end
         end
 
+        def self.check_histogram
+          @check_histogram ||=
+            METRICS_MUTEX.synchronize do
+              @check_histogram || Gitlab::Metrics.histogram(:circuitbreaker_storage_check_duration_seconds,
+                                                            'Storage check time in seconds',
+                                                            {},
+                                                            STORAGE_TIMING_BUCKETS
+                                                           )
+            end
+        end
+
         def initialize(storage, logger = Rails.logger)
           @storage = storage
           config = Gitlab.config.repositories.storages[@storage]
@@ -45,7 +58,7 @@ module Gitlab
         end
 
         def check
-          if Gitlab::Git::Storage::ForkedStorageCheck.storage_available?(storage_path, storage_timeout, access_retries)
+          if perform_access_check
             track_storage_accessible
             true
           else
@@ -57,6 +70,15 @@ module Gitlab
 
         private
 
+        def perform_access_check
+          start_time = Gitlab::Metrics::System.monotonic_time
+
+          Gitlab::Git::Storage::ForkedStorageCheck.storage_available?(storage_path, storage_timeout, access_retries)
+        ensure
+          execution_time = Gitlab::Metrics::System.monotonic_time - start_time
+          self.class.check_histogram.observe({ storage: storage }, execution_time)
+        end
+
         def track_storage_inaccessible
           first_failure = current_failure_info.first_failure || Time.now
           last_failure = Time.now