summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-09-27 14:29:51 +0000
committerRémy Coutable <remy@rymai.me>2016-09-27 14:29:51 +0000
commit0b4fd0af16555b2bdc28f6b18781d72226c5d56c (patch)
treec3a252d71cab51e47d4f1a03c40abe0a1851f6f2
parentb71ca5da9fde4fa8457af146bd090ec7caa28d60 (diff)
parent192e2bd367494bf66746c8971896a2d9cb84fc92 (diff)
downloadgitlab-shell-0b4fd0af16555b2bdc28f6b18781d72226c5d56c.tar.gz
Merge branch '59-git-tracing' into 'master' v3.6.2
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
-rw-r--r--CHANGELOG3
-rw-r--r--config.yml.example7
-rw-r--r--lib/gitlab_config.rb4
-rw-r--r--lib/gitlab_shell.rb26
-rw-r--r--spec/gitlab_shell_spec.rb68
5 files changed, 107 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 57b074a..54ccc82 100644
--- a/CHANGELOG
+++ b/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
diff --git a/config.yml.example b/config.yml.example
index cd66022..0164830 100644
--- a/config.yml.example
+++ b/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:
diff --git a/lib/gitlab_config.rb b/lib/gitlab_config.rb
index beaf173..781c706 100644
--- a/lib/gitlab_config.rb
+++ b/lib/gitlab_config.rb
@@ -50,4 +50,8 @@ class GitlabConfig
def git_annex_enabled?
@config['git_annex_enabled'] ||= false
end
+
+ def git_trace_log_file
+ @config['git_trace_log_file']
+ end
end
diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb
index 971b22f..f72ce74 100644
--- a/lib/gitlab_shell.rb
+++ b/lib/gitlab_shell.rb
@@ -1,4 +1,5 @@
require 'shellwords'
+require 'pathname'
require_relative 'gitlab_net'
@@ -150,6 +151,14 @@ class GitlabShell
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 @@ class GitlabShell
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
diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb
index 96cae40..b9b8659 100644
--- a/spec/gitlab_shell_spec.rb
+++ b/spec/gitlab_shell_spec.rb
@@ -361,7 +361,7 @@ describe GitlabShell do
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 @@ describe GitlabShell do
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