Merge branch '59-git-tracing' into 'master'

 Enable GIT_TRACE_PERFORMANCE through a config variable. The value of the variable must an absolute path needs to exist so we’re able to check if we can write in that file. Because in the case we cannot write we’ll throw a warning to the output of the users. ```sh ~/dev/gitlab/local/pacoguzman/gitlab-ce (master=)$ git push origin master warning: could not open '/wadus' for tracing: Permission denied Everything up-to-date ``` Closes #59 See merge request !91

Author: rymai

CHANGELOG

@@ -1,3 +1,6 @@
+v3.6.2
+ - Enable GIT_TRACE/GIT_TRACE_PACKET/GIT_TRACE_PERFORMANCE by providing the git_trace_log_file config key
+
 v3.6.1
  - Set a low IO priority for storage moves to lower performance impact
 

config.yml.example

@@ -68,3 +68,10 @@ audit_usernames: false
 # For Debian and Ubuntu systems this can be done with: sudo apt-get install git-annex
 # For CentOS: sudo yum install epel-release && sudo yum install git-annex
 git_annex_enabled: false
+
+# Git trace log file.
+# If set, git commands receive GIT_TRACE* environment variables
+# See https://git-scm.com/book/es/v2/Git-Internals-Environment-Variables#Debugging for documentation
+# An absolute path starting with / – the trace output will be appended to that file.
+# It needs to exist so we can check permissions and avoid to throwing warnings to the users.
+git_trace_log_file:

lib/gitlab_config.rb

@@ -50,4 +50,8 @@ def audit_usernames
  def git_annex_enabled?
  @config['git_annex_enabled'] ||= false
  end
+
+ def git_trace_log_file
+ @config['git_trace_log_file']
+ end
 end

lib/gitlab_shell.rb

@@ -1,4 +1,5 @@
 require 'shellwords'
+require 'pathname'
 
 require_relative 'gitlab_net'
 
@@ -150,6 +151,14 @@ def exec_cmd(*args)
  env.merge!({ 'GIT_ANNEX_SHELL_LIMITED' => '1' })
  end
 
+ if git_trace_available?
+ env.merge!({
+ 'GIT_TRACE' => @config.git_trace_log_file,
+ 'GIT_TRACE_PACKET' => @config.git_trace_log_file,
+ 'GIT_TRACE_PERFORMANCE' => @config.git_trace_log_file,
+ })
+ end
+
  Kernel::exec(env, *args, unsetenv_others: true)
  end
 
@@ -232,6 +241,23 @@ def api_2fa_recovery_codes
  end
  end
 
+ def git_trace_available?
+ return false unless @config.git_trace_log_file
+
+ if Pathname(@config.git_trace_log_file).relative?
+ $logger.warn "gitlab-shell: is configured to trace git commands with #{@config.git_trace_log_file.inspect} but an absolute path needs to be provided"
+ return false
+ end
+
+ begin
+ File.open(@config.git_trace_log_file, 'a') { nil }
+ return true
+ rescue => ex
+ $logger.warn "gitlab-shell: is configured to trace git commands with #{@config.git_trace_log_file.inspect} but it's not possible to write in that path #{ex.message}"
+ return false
+ end
+ end
+
  def repo_path=(repo_path)
  raise ArgumentError, "Repository path not provided. Please make sure you're using GitLab v8.10 or later." unless repo_path
  raise InvalidRepositoryPathError if File.absolute_path(repo_path) != repo_path

spec/gitlab_shell_spec.rb

@@ -361,7 +361,7 @@
 
  describe :exec_cmd do
  let(:shell) { GitlabShell.new(key_id) }
- before { Kernel.stub!(:exec) }
+ before { Kernel.stub(:exec) }
 
  it "uses Kernel::exec method" do
  Kernel.should_receive(:exec).with(kind_of(Hash), 1, 2, unsetenv_others: true).once
@@ -376,6 +376,72 @@
  Kernel.should_receive(:exec).with(kind_of(Hash), [1, 2], unsetenv_others: true).once
  shell.send :exec_cmd, [1, 2]
  end
+
+ context "when specifying a git_tracing log file" do
+ let(:git_trace_log_file) { '/tmp/git_trace_performance.log' }
+
+ before do
+ GitlabConfig.any_instance.stub(git_trace_log_file: git_trace_log_file)
+ shell
+ end
+
+ it "uses GIT_TRACE_PERFORMANCE" do
+ expected_hash = hash_including(
+ 'GIT_TRACE' => git_trace_log_file,
+ 'GIT_TRACE_PACKET' => git_trace_log_file,
+ 'GIT_TRACE_PERFORMANCE' => git_trace_log_file
+ )
+ Kernel.should_receive(:exec).with(expected_hash, [1, 2], unsetenv_others: true).once
+
+ shell.send :exec_cmd, [1, 2]
+ end
+
+ context "when provides a relative path" do
+ let(:git_trace_log_file) { 'git_trace_performance.log' }
+
+ it "does not uses GIT_TRACE*" do
+ # If we try to use it we'll show a warning to the users
+ expected_hash = hash_excluding(
+ 'GIT_TRACE', 'GIT_TRACE_PACKET', 'GIT_TRACE_PERFORMANCE'
+ )
+ Kernel.should_receive(:exec).with(expected_hash, [1, 2], unsetenv_others: true).once
+
+ shell.send :exec_cmd, [1, 2]
+ end
+
+ it "writes an entry on the log" do
+ expect($logger).to receive(:warn).
+ with("gitlab-shell: is configured to trace git commands with #{git_trace_log_file.inspect} but an absolute path needs to be provided")
+
+ Kernel.should_receive(:exec).with(kind_of(Hash), [1, 2], unsetenv_others: true).once
+ shell.send :exec_cmd, [1, 2]
+ end
+ end
+
+ context "when provides a file not writable" do
+ before do
+ expect(File).to receive(:open).with(git_trace_log_file, 'a').and_raise(Errno::EACCES)
+ end
+
+ it "does not uses GIT_TRACE*" do
+ # If we try to use it we'll show a warning to the users
+ expected_hash = hash_excluding(
+ 'GIT_TRACE', 'GIT_TRACE_PACKET', 'GIT_TRACE_PERFORMANCE'
+ )
+ Kernel.should_receive(:exec).with(expected_hash, [1, 2], unsetenv_others: true).once
+
+ shell.send :exec_cmd, [1, 2]
+ end
+
+ it "writes an entry on the log" do
+ expect($logger).to receive(:warn).
+ with("gitlab-shell: is configured to trace git commands with #{git_trace_log_file.inspect} but it's not possible to write in that path Permission denied")
+
+ Kernel.should_receive(:exec).with(kind_of(Hash), [1, 2], unsetenv_others: true).once
+ shell.send :exec_cmd, [1, 2]
+ end
+ end
+ end
  end
 
  describe :api do