diff options
-rw-r--r-- | CHANGELOG.md | 2 | ||||
-rw-r--r-- | config.toml.example | 2 | ||||
-rw-r--r-- | internal/rubyserver/rubyserver.go | 6 | ||||
-rw-r--r-- | ruby/Gemfile | 1 | ||||
-rw-r--r-- | ruby/Gemfile.lock | 3 | ||||
-rwxr-xr-x | ruby/bin/gitaly-ruby | 6 | ||||
-rw-r--r-- | ruby/lib/gitaly_server/sentry.rb | 14 | ||||
-rw-r--r-- | ruby/lib/gitaly_server/sentry_interceptor.rb | 64 | ||||
-rw-r--r-- | ruby/spec/lib/gitaly_server/sentry_interceptor_spec.rb | 82 |
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 |