Add latest changes from gitlab-org/gitlab@master

lib/gitlab/exception_log_formatter.rb

@@ -21,6 +21,10 @@ module Gitlab
           payload['exception.cause_class'] = exception.cause.class.name
         end
 
+        if gitaly_metadata = find_gitaly_metadata(exception)
+          payload['exception.gitaly'] = gitaly_metadata.to_s
+        end
+
         if sql = find_sql(exception)
           payload['exception.sql'] = sql
         end
@@ -35,6 +39,16 @@ module Gitlab
         end
       end
 
+      def find_gitaly_metadata(exception)
+        if exception.is_a?(::Gitlab::Git::BaseError)
+          exception.metadata
+        elsif exception.is_a?(::GRPC::BadStatus)
+          exception.metadata[::Gitlab::Git::BaseError::METADATA_KEY]
+        elsif exception.cause.present?
+          find_gitaly_metadata(exception.cause)
+        end
+      end
+
       private
 
       def normalize_query(sql)

lib/gitlab/git/base_error.rb

@@ -4,6 +4,7 @@ require 'grpc'
 module Gitlab
   module Git
     class BaseError < StandardError
+      METADATA_KEY = :gitaly_error_metadata
       DEBUG_ERROR_STRING_REGEX = /(.*?) debug_error_string:.*$/m.freeze
       GRPC_CODES = {
         '0' => 'ok',
@@ -25,12 +26,15 @@ module Gitlab
         '16' => 'unauthenticated'
       }.freeze
 
-      attr_reader :status, :code, :service
+      attr_reader :status, :code, :service, :metadata
 
       def initialize(msg = nil)
         super && return if msg.nil?
 
-        set_grpc_error_code(msg) if msg.is_a?(::GRPC::BadStatus)
+        if msg.is_a?(::GRPC::BadStatus)
+          set_grpc_error_code(msg)
+          set_grpc_error_metadata(msg)
+        end
 
         super(build_raw_message(msg))
       end
@@ -46,6 +50,10 @@ module Gitlab
         @code = GRPC_CODES[@status.to_s]
         @service = 'git'
       end
+
+      def set_grpc_error_metadata(grpc_error)
+        @metadata = grpc_error.metadata.fetch(METADATA_KEY, {}).clone
+      end
     end
   end
 end

lib/gitlab/gitaly_client/call.rb

@@ -32,6 +32,8 @@ module Gitlab
         end
       rescue StandardError => err
         store_timings
+        set_gitaly_error_metadata(err) if err.is_a?(::GRPC::BadStatus)
+
         raise err
       end
 
@@ -44,6 +46,9 @@ module Gitlab
 
             yielder.yield(value)
           end
+        rescue ::GRPC::BadStatus => err
+          set_gitaly_error_metadata(err)
+          raise err
         ensure
           store_timings
         end
@@ -73,6 +78,15 @@ module Gitlab
           backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller)
         )
       end
+
+      def set_gitaly_error_metadata(err)
+        err.metadata[::Gitlab::Git::BaseError::METADATA_KEY] = {
+          storage: @storage,
+          address: ::Gitlab::GitalyClient.address(@storage),
+          service: @service,
+          rpc: @rpc
+        }
+      end
     end
   end
 end

spec/lib/gitlab/exception_log_formatter_spec.rb

@@ -67,5 +67,53 @@ RSpec.describe Gitlab::ExceptionLogFormatter do
         expect(payload['exception.sql']).to eq('SELECT SELECT FROM SELECT')
       end
     end
+
+    context 'when exception is a gRPC bad status' do
+      let(:unavailable_error) do
+        ::GRPC::Unavailable.new(
+          "unavailable",
+          gitaly_error_metadata: {
+            storage: 'default',
+            address: 'unix://gitaly.socket',
+            service: :ref_service,
+            rpc: :find_local_branches
+          }
+        )
+      end
+
+      context 'when the gRPC error is wrapped by ::Gitlab::Git::BaseError' do
+        let(:exception) { ::Gitlab::Git::CommandError.new(unavailable_error) }
+
+        it 'adds gitaly metadata to payload' do
+          described_class.format!(exception, payload)
+
+          expect(payload['exception.gitaly']).to eq('{:storage=>"default", :address=>"unix://gitaly.socket", :service=>:ref_service, :rpc=>:find_local_branches}')
+        end
+      end
+
+      context 'when the gRPC error is wrapped by another error' do
+        before do
+          allow(exception).to receive(:cause).and_return(unavailable_error)
+        end
+
+        it 'adds gitaly metadata to payload' do
+          described_class.format!(exception, payload)
+
+          expect(payload['exception.cause_class']).to eq('GRPC::Unavailable')
+          expect(payload['exception.gitaly']).to eq('{:storage=>"default", :address=>"unix://gitaly.socket", :service=>:ref_service, :rpc=>:find_local_branches}')
+        end
+      end
+
+      context 'when the gRPC error is not wrapped' do
+        let(:exception) { unavailable_error }
+
+        it 'adds gitaly metadata to payload' do
+          described_class.format!(exception, payload)
+
+          expect(payload['exception.cause_class']).to be_nil
+          expect(payload['exception.gitaly']).to eq('{:storage=>"default", :address=>"unix://gitaly.socket", :service=>:ref_service, :rpc=>:find_local_branches}')
+        end
+      end
+    end
   end
 end

spec/lib/gitlab/git/base_error_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe Gitlab::Git::BaseError do
     it { is_expected.to eq(result) }
   end
 
-  describe "When initialized with GRPC errors" do
+  describe "when initialized with GRPC errors without metadata" do
     let(:grpc_error) { GRPC::DeadlineExceeded.new }
     let(:git_error) { described_class.new grpc_error }
 
@@ -29,6 +29,33 @@ RSpec.describe Gitlab::Git::BaseError do
       expect(git_error.service).to eq('git')
       expect(git_error.status).to eq(4)
       expect(git_error.code).to eq('deadline_exceeded')
+      expect(git_error.metadata).to eq({})
+    end
+  end
+
+  describe "when initialized with GRPC errors with metadata" do
+    let(:grpc_error) do
+      GRPC::DeadlineExceeded.new(
+        "deadline exceeded",
+        gitaly_error_metadata: {
+          storage: "default",
+          address: "unix://gitaly.socket",
+          service: :ref_service, rpc: :find_local_branches
+        }
+      )
+    end
+
+    let(:git_error) { described_class.new grpc_error }
+
+    it "has status, code, and metadata fields" do
+      expect(git_error.service).to eq('git')
+      expect(git_error.status).to eq(4)
+      expect(git_error.code).to eq('deadline_exceeded')
+      expect(git_error.metadata).to eq(
+        storage: "default",
+        address: "unix://gitaly.socket",
+        service: :ref_service, rpc: :find_local_branches
+      )
     end
   end
 end

spec/lib/gitlab/gitaly_client/call_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::GitalyClient::Call do
+RSpec.describe Gitlab::GitalyClient::Call, feature_category: :gitaly do
   describe '#call', :request_store do
     let(:client) { Gitlab::GitalyClient }
     let(:storage) { 'default' }
@@ -50,7 +50,7 @@ RSpec.describe Gitlab::GitalyClient::Call do
         expect_call_details_to_match
       end
 
-      context 'when err' do
+      context 'when the call raises an standard error' do
         before do
           allow(client).to receive(:execute).and_raise(StandardError)
         end
@@ -62,6 +62,25 @@ RSpec.describe Gitlab::GitalyClient::Call do
           expect_call_details_to_match
         end
       end
+
+      context 'when the call raises a BadStatus error' do
+        before do
+          allow(client).to receive(:execute).and_raise(GRPC::Unavailable)
+        end
+
+        it 'attaches gitaly metadata' do
+          expect { subject }.to raise_error do |err|
+            expect(err.metadata).to eql(
+              gitaly_error_metadata: {
+                storage: storage,
+                address: client.address(storage),
+                service: service,
+                rpc: rpc
+              }
+            )
+          end
+        end
+      end
     end
 
     context 'when the response is an enumerator' do
@@ -103,7 +122,7 @@ RSpec.describe Gitlab::GitalyClient::Call do
           expect_call_details_to_match(duration_higher_than: 0.1)
         end
 
-        context 'when err' do
+        context 'when the call raises an standard error' do
           let(:response) do
             Enumerator.new do |yielder|
               sleep 0.2
@@ -119,6 +138,28 @@ RSpec.describe Gitlab::GitalyClient::Call do
             expect_call_details_to_match(duration_higher_than: 0.2)
           end
         end
+
+        context 'when the call raises a BadStatus error' do
+          let(:response) do
+            Enumerator.new do |yielder|
+              yielder << 1
+              raise GRPC::Unavailable
+            end
+          end
+
+          it 'attaches gitaly metadata' do
+            expect { subject.to_a }.to raise_error do |err|
+              expect(err.metadata).to eql(
+                gitaly_error_metadata: {
+                  storage: storage,
+                  address: client.address(storage),
+                  service: service,
+                  rpc: rpc
+                }
+              )
+            end
+          end
+        end
       end
     end
   end