Send gitaly-ruby exceptions to Sentry

Closes #988

9 files changed, 178 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e2c01cc6..dd85f82f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Send gitaly-ruby exceptions to Sentry
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/598
 - Detect License type for repositories
   https://gitlab.com/gitlab-org/gitaly/merge_requests/601
 
diff --git a/config.toml.example b/config.toml.example
index 10f8cce49..113a22232 100644
--- a/config.toml.example
+++ b/config.toml.example
@@ -31,7 +31,7 @@ path = "/home/git/repositories"
 # # You can optionally configure Gitaly to output JSON-formatted log messages to stdout
 # [logging]
 # format = "json"
-# # Additionally exceptions can be reported to Sentry
+# # Additionally exceptions from the Go server and gitaly-ruby can be reported to Sentry
 # sentry_dsn = "https://<key>:<secret>@sentry.io/<project>"
 
 # # You can optionally configure Gitaly to record histogram latencies on GRPC method calls
diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go
index 2f191048e..35dcdcca0 100644
--- a/internal/rubyserver/rubyserver.go
+++ b/internal/rubyserver/rubyserver.go
@@ -15,6 +15,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver/balancer"
 	"gitlab.com/gitlab-org/gitaly/internal/supervisor"
+	"gitlab.com/gitlab-org/gitaly/internal/version"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -104,7 +105,12 @@ func Start() (*Server, error) {
 		fmt.Sprintf("GITALY_RUBY_WRITE_BUFFER_SIZE=%d", streamio.WriteBufferSize),
 		"GITALY_RUBY_GITLAB_SHELL_PATH="+cfg.GitlabShell.Dir,
 		"GITALY_RUBY_GITALY_BIN_DIR="+cfg.BinDir,
+		"GITALY_VERSION="+version.GetVersion(),
 	)
+	if dsn := cfg.Logging.SentryDSN; dsn != "" {
+		env = append(env, "SENTRY_DSN="+dsn)
+	}
+
 	gitalyRuby := path.Join(cfg.Ruby.Dir, "bin/gitaly-ruby")
 
 	s := &Server{}
diff --git a/ruby/Gemfile b/ruby/Gemfile
index c57375866..74c464336 100644
--- a/ruby/Gemfile
+++ b/ruby/Gemfile
@@ -8,6 +8,7 @@ gem 'rdoc', '~> 4.2'
 gem 'gollum-lib', '~> 4.2', require: false
 gem 'gollum-rugged_adapter', '~> 0.4.4', require: false
 gem 'grpc', '~> 1.8.0'
+gem 'sentry-raven', '~> 2.7.2', require: false
 
 # Detects the open source license the repository includes
 # This version needs to be in sync with GitLab CE/EE
diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock
index 4751e186b..8ad696dc6 100644
--- a/ruby/Gemfile.lock
+++ b/ruby/Gemfile.lock
@@ -123,6 +123,8 @@ GEM
     rugged (0.26.0)
     sanitize (2.1.0)
       nokogiri (>= 1.4.4)
+    sentry-raven (2.7.2)
+      faraday (>= 0.7.6, < 1.0)
     signet (0.8.1)
       addressable (~> 2.3)
       faraday (~> 0.9)
@@ -150,6 +152,7 @@ DEPENDENCIES
   licensee (~> 8.7.0)
   rdoc (~> 4.2)
   rspec
+  sentry-raven (~> 2.7.2)
 
 BUNDLED WITH
    1.16.1
diff --git a/ruby/bin/gitaly-ruby b/ruby/bin/gitaly-ruby
index 6f69dabf5..c8d8f909b 100755
--- a/ruby/bin/gitaly-ruby
+++ b/ruby/bin/gitaly-ruby
@@ -5,6 +5,7 @@ require 'fileutils'
 require 'grpc'
 
 require_relative '../lib/gitaly_server.rb'
+require_relative '../lib/gitaly_server/sentry_interceptor.rb'
 
 SHUTDOWN_TIMEOUT = 600
 
@@ -22,7 +23,10 @@ def main
   FileUtils.mkdir_p(socket_dir)
   File.chmod(0700, socket_dir)
 
-  s = GRPC::RpcServer.new(poll_period: SHUTDOWN_TIMEOUT)
+  s = GRPC::RpcServer.new(
+    poll_period: SHUTDOWN_TIMEOUT,
+    interceptors: [GitalyServer::SentryInterceptor.new]
+  )
   port = 'unix:' + socket_path
   s.add_http2_port(port, :this_port_is_insecure)
   GRPC.logger.info("... running insecurely on #{port}")
diff --git a/ruby/lib/gitaly_server/sentry.rb b/ruby/lib/gitaly_server/sentry.rb
new file mode 100644
index 000000000..178950eba
--- /dev/null
+++ b/ruby/lib/gitaly_server/sentry.rb
@@ -0,0 +1,14 @@
+require 'raven/base'
+
+module GitalyServer
+  module Sentry
+    def self.enabled?
+      ENV['SENTRY_DSN'].present?
+    end
+  end
+end
+
+Raven.configure do |config|
+  config.release = ENV['GITALY_VERSION'].presence
+  config.sanitize_fields = %w[gitaly-servers authorization]
+end
diff --git a/ruby/lib/gitaly_server/sentry_interceptor.rb b/ruby/lib/gitaly_server/sentry_interceptor.rb
new file mode 100644
index 000000000..69272334b
--- /dev/null
+++ b/ruby/lib/gitaly_server/sentry_interceptor.rb
@@ -0,0 +1,64 @@
+require 'grpc'
+require 'raven/base'
+
+require_relative 'sentry.rb'
+
+# rubocop:disable Lint/RescueWithoutErrorClass
+module GitalyServer
+  class SentryInterceptor < GRPC::ServerInterceptor
+    # Intercept a unary request response call
+    def request_response(request: nil, call: nil, method: nil)
+      start = Time.now
+      yield
+    rescue => e
+      time_ms = Time.now - start
+      handle_exception(e, call, method, time_ms)
+    end
+
+    # Intercept a server streaming call
+    def server_streamer(request: nil, call: nil, method: nil)
+      start = Time.now
+      yield
+    rescue => e
+      time_ms = Time.now - start
+      handle_exception(e, call, method, time_ms)
+    end
+
+    # Intercept a client streaming call
+    def client_streamer(call: nil, method: nil)
+      start = Time.now
+      yield
+    rescue => e
+      time_ms = Time.now - start
+      handle_exception(e, call, method, time_ms)
+    end
+
+    # Intercept a BiDi streaming call
+    def bidi_streamer(requests: nil, call: nil, method: nil)
+      start = Time.now
+      yield
+    rescue => e
+      time_ms = Time.now - start
+      handle_exception(e, call, method, time_ms)
+    end
+
+    private
+
+    def handle_exception(exc, call, method, time_ms)
+      raise exc unless GitalyServer::Sentry.enabled?
+
+      grpc_method = "#{method.owner.name}##{method.name}"
+      tags = {
+        'system' => 'gitaly-ruby',
+        'gitaly-ruby.method' => grpc_method,
+        'gitaly-ruby.time_ms' => format("%.0f", (time_ms * 1000))
+      }
+      tags.merge!(call.metadata)
+
+      Raven.tags_context(tags)
+      Raven.capture_exception(exc, fingerprint: ['gitaly-ruby', grpc_method, exc.message])
+
+      raise exc
+    end
+  end
+end
diff --git a/ruby/spec/lib/gitaly_server/sentry_interceptor_spec.rb b/ruby/spec/lib/gitaly_server/sentry_interceptor_spec.rb
new file mode 100644
index 000000000..b9bdd26aa
--- /dev/null
+++ b/ruby/spec/lib/gitaly_server/sentry_interceptor_spec.rb
@@ -0,0 +1,82 @@
+require 'spec_helper'
+
+require_relative '../../../lib/gitaly_server/sentry_interceptor.rb'
+
+# rubocop:disable Lint/RescueWithoutErrorClass
+describe GitalyServer::SentryInterceptor do
+  describe 'handling exceptions' do
+    let(:meth) { GitalyServer::RefService.instance_method(:create_branch) }
+    let(:ex) { ArgumentError.new("unknown encoding") }
+    let(:call) { nil }
+
+    subject do
+      described_class.new.server_streamer(call: call, method: meth) { raise ex }
+    end
+
+    context 'Sentry is disabled' do
+      it 're-raises the exception' do
+        expect { subject }.to raise_error(ex)
+      end
+
+      it 'sends nothing to Sentry' do
+        expect(Raven).not_to receive(:capture_exception)
+
+        begin
+          subject
+        rescue
+          nil
+        end
+      end
+    end
+
+    context 'Sentry is enabled' do
+      before do
+        allow(GitalyServer::Sentry).to receive(:enabled?).and_return(true)
+      end
+
+      let(:call_metadata) do
+        {
+          'user-agent' => 'grpc-go/1.9.1',
+          'gitaly-storage-path' => '/home/git/repositories',
+          'gitaly-repo-path' => '/home/git/repositories/gitlab-org/gitaly.git',
+          'gitaly-gl-repository' => 'project-52',
+          'gitaly-repo-alt-dirs' => ''
+        }
+      end
+      let(:call) { double(metadata: call_metadata) }
+      let(:expected_tags) do
+        call_metadata.merge(
+          'system' => 'gitaly-ruby',
+          'gitaly-ruby.method' => 'GitalyServer::RefService#create_branch'
+        )
+      end
+
+      it 're-raises the exception' do
+        expect { subject }.to raise_error(ex)
+      end
+
+      it 'sets Sentry tags' do
+        expect(Raven).to receive(:tags_context).with(hash_including(expected_tags))
+
+        begin
+          subject
+        rescue
+          nil
+        end
+      end
+
+      it 'sends the exception to Sentry' do
+        expect(Raven).to receive(:capture_exception).with(
+          ex,
+          { fingerprint: ['gitaly-ruby', 'GitalyServer::RefService#create_branch', 'unknown encoding'] }
+        )
+
+        begin
+          subject
+        rescue
+          nil
+        end
+      end
+    end
+  end
+end