Merge branch 'jc-client-gitaly-session-id' into 'master'

Add gitaly session id & catfile-cache feature flag

See merge request gitlab-org/gitlab-ce!27472

changelogs/unreleased/jc-client-gitaly-session-id.yml

@@ -0,0 +1,5 @@
+---
+title: Add gitaly session id & catfile-cache feature flag
+merge_request: 27472
+author:
+type: performance

lib/gitlab/gitaly_client.rb

@@ -31,6 +31,9 @@ def initialize(call_site, invocation_count, max_call_stack, most_invoked_stack)
     MAXIMUM_GITALY_CALLS = 30
     CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
 
+    SERVER_FEATURE_CATFILE_CACHE = 'catfile-cache'.freeze
+    SERVER_FEATURE_FLAGS = [SERVER_FEATURE_CATFILE_CACHE].freeze
+
     MUTEX = Mutex.new
 
     define_histogram :gitaly_controller_action_duration_seconds do
@@ -219,6 +222,7 @@ def self.request_kwargs(storage, timeout, remote_storage: nil)
       metadata['call_site'] = feature.to_s if feature
       metadata['gitaly-servers'] = address_metadata(remote_storage) if remote_storage
       metadata['x-gitlab-correlation-id'] = Labkit::Correlation::CorrelationId.current_id if Labkit::Correlation::CorrelationId.current_id
+      metadata['gitaly-session-id'] = session_id if feature_enabled?(SERVER_FEATURE_CATFILE_CACHE)
 
       metadata.merge!(server_feature_flags)
 
@@ -235,7 +239,9 @@ def self.request_kwargs(storage, timeout, remote_storage: nil)
       result
     end
 
-    SERVER_FEATURE_FLAGS = %w[].freeze
+    def self.session_id
+      Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid
+    end
 
     def self.server_feature_flags
       SERVER_FEATURE_FLAGS.map do |f|

spec/lib/gitlab/gitaly_client_spec.rb

@@ -142,6 +142,48 @@ def stub_repos_storages(address)
     end
   end
 
+  describe '.request_kwargs' do
+    context 'when catfile-cache feature is enabled' do
+      before do
+        stub_feature_flags('gitaly_catfile-cache': true)
+      end
+
+      it 'sets the gitaly-session-id in the metadata' do
+        results = described_class.request_kwargs('default', nil)
+        expect(results[:metadata]).to include('gitaly-session-id')
+      end
+
+      context 'when RequestStore is not enabled' do
+        it 'sets a different gitaly-session-id per request' do
+          gitaly_session_id = described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id']
+
+          expect(described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id']).not_to eq(gitaly_session_id)
+        end
+      end
+
+      context 'when RequestStore is enabled', :request_store do
+        it 'sets the same gitaly-session-id on every outgoing request metadata' do
+          gitaly_session_id = described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id']
+
+          3.times do
+            expect(described_class.request_kwargs('default', nil)[:metadata]['gitaly-session-id']).to eq(gitaly_session_id)
+          end
+        end
+      end
+    end
+
+    context 'when catfile-cache feature is disabled' do
+      before do
+        stub_feature_flags({ 'gitaly_catfile-cache': false })
+      end
+
+      it 'does not set the gitaly-session-id in the metadata' do
+        results = described_class.request_kwargs('default', nil)
+        expect(results[:metadata]).not_to include('gitaly-session-id')
+      end
+    end
+  end
+
   describe 'enforce_gitaly_request_limits?' do
     def call_gitaly(count = 1)
       (1..count).each do