summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzegorz@gitlab.com>2019-07-23 09:30:01 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2019-07-23 09:30:01 +0000
commit9cffa428ad1b572437c38d7a3a41a26c35cc3576 (patch)
treed6631643d70462ee38936708107bbedefbb5f45f
parent5e102f17f0ef16d0fd1eff98b9229fea2bc1fec9 (diff)
parent3a4cb6d6759abbfd16e048413e8545d1d94e7d9e (diff)
downloadgitlab-ce-9cffa428ad1b572437c38d7a3a41a26c35cc3576.tar.gz
Merge branch 'backward-compatible-request-profiles' into 'master'
Bring backward compatibility for request profiles See merge request gitlab-org/gitlab-ce!30956
-rw-r--r--app/controllers/admin/requests_profiles_controller.rb4
-rw-r--r--lib/gitlab/request_profiler.rb15
-rw-r--r--lib/gitlab/request_profiler/profile.rb29
-rw-r--r--spec/controllers/admin/requests_profiles_controller_spec.rb2
-rw-r--r--spec/features/admin/admin_requests_profiles_spec.rb12
-rw-r--r--spec/lib/gitlab/request_profiler/profile_spec.rb59
-rw-r--r--spec/lib/gitlab/request_profiler_spec.rb41
7 files changed, 130 insertions, 32 deletions
diff --git a/app/controllers/admin/requests_profiles_controller.rb b/app/controllers/admin/requests_profiles_controller.rb
index 1d14812186a..24383455064 100644
--- a/app/controllers/admin/requests_profiles_controller.rb
+++ b/app/controllers/admin/requests_profiles_controller.rb
@@ -3,12 +3,12 @@
class Admin::RequestsProfilesController < Admin::ApplicationController
def index
@profile_token = Gitlab::RequestProfiler.profile_token
- @profiles = Gitlab::RequestProfiler::Profile.all.group_by(&:request_path)
+ @profiles = Gitlab::RequestProfiler.all.group_by(&:request_path)
end
def show
clean_name = Rack::Utils.clean_path_info(params[:name])
- profile = Gitlab::RequestProfiler::Profile.find(clean_name)
+ profile = Gitlab::RequestProfiler.find(clean_name)
unless profile && profile.content_type
return redirect_to admin_requests_profiles_path, alert: 'Profile not found'
diff --git a/lib/gitlab/request_profiler.rb b/lib/gitlab/request_profiler.rb
index 64593153686..033e451dbee 100644
--- a/lib/gitlab/request_profiler.rb
+++ b/lib/gitlab/request_profiler.rb
@@ -6,6 +6,21 @@ module Gitlab
module RequestProfiler
PROFILES_DIR = "#{Gitlab.config.shared.path}/tmp/requests_profiles".freeze
+ def all
+ Dir["#{PROFILES_DIR}/*.{html,txt}"].map do |path|
+ Profile.new(File.basename(path))
+ end.select(&:valid?)
+ end
+ module_function :all
+
+ def find(name)
+ file_path = File.join(PROFILES_DIR, name)
+ return unless File.exist?(file_path)
+
+ Profile.new(name)
+ end
+ module_function :find
+
def profile_token
Rails.cache.fetch('profile-token') do
Devise.friendly_token
diff --git a/lib/gitlab/request_profiler/profile.rb b/lib/gitlab/request_profiler/profile.rb
index 74f2ec1d083..76c675658b1 100644
--- a/lib/gitlab/request_profiler/profile.rb
+++ b/lib/gitlab/request_profiler/profile.rb
@@ -7,19 +7,6 @@ module Gitlab
alias_method :to_param, :name
- def self.all
- Dir["#{PROFILES_DIR}/*.{html,txt}"].map do |path|
- new(File.basename(path))
- end
- end
-
- def self.find(name)
- file_path = File.join(PROFILES_DIR, name)
- return unless File.exist?(file_path)
-
- new(name)
- end
-
def initialize(name)
@name = name
@file_path = File.join(PROFILES_DIR, name)
@@ -27,8 +14,8 @@ module Gitlab
set_attributes
end
- def content
- File.read("#{PROFILES_DIR}/#{name}")
+ def valid?
+ @request_path.present?
end
def content_type
@@ -43,11 +30,13 @@ module Gitlab
private
def set_attributes
- _, path, timestamp, profile_mode, type = name.split(/(.*)_(\d+)_(.*)\.(html|txt)$/)
- @request_path = path.tr('|', '/')
- @time = Time.at(timestamp.to_i).utc
- @profile_mode = profile_mode
- @type = type
+ matches = name.match(/^(?<path>.*)_(?<timestamp>\d+)(_(?<profile_mode>\w+))?\.(?<type>html|txt)$/)
+ return unless matches
+
+ @request_path = matches[:path].tr('|', '/')
+ @time = Time.at(matches[:timestamp].to_i).utc
+ @profile_mode = matches[:profile_mode] || 'unknown'
+ @type = matches[:type]
end
end
end
diff --git a/spec/controllers/admin/requests_profiles_controller_spec.rb b/spec/controllers/admin/requests_profiles_controller_spec.rb
index 289bb58c5a8..345f7720c25 100644
--- a/spec/controllers/admin/requests_profiles_controller_spec.rb
+++ b/spec/controllers/admin/requests_profiles_controller_spec.rb
@@ -23,7 +23,7 @@ describe Admin::RequestsProfilesController do
end
after do
- File.unlink(test_file)
+ FileUtils.rm_rf(tmpdir)
end
context 'when loading HTML profile' do
diff --git a/spec/features/admin/admin_requests_profiles_spec.rb b/spec/features/admin/admin_requests_profiles_spec.rb
index a962af4952b..e8764d0a79c 100644
--- a/spec/features/admin/admin_requests_profiles_spec.rb
+++ b/spec/features/admin/admin_requests_profiles_spec.rb
@@ -1,13 +1,15 @@
require 'spec_helper'
describe 'Admin::RequestsProfilesController' do
+ let(:tmpdir) { Dir.mktmpdir('profiler-test') }
+
before do
- FileUtils.mkdir_p(Gitlab::RequestProfiler::PROFILES_DIR)
+ stub_const('Gitlab::RequestProfiler::PROFILES_DIR', tmpdir)
sign_in(create(:admin))
end
after do
- Gitlab::RequestProfiler.remove_all_profiles
+ FileUtils.rm_rf(tmpdir)
end
describe 'GET /admin/requests_profiles' do
@@ -60,6 +62,12 @@ describe 'Admin::RequestsProfilesController' do
name: "|gitlab-org|infrastructure_#{time2.to_i}_memory.html",
created: time2,
profile_mode: 'Memory'
+ },
+ {
+ request_path: '/gitlab-org/infrastructure',
+ name: "|gitlab-org|infrastructure_#{time2.to_i}.html",
+ created: time2,
+ profile_mode: 'Unknown'
}
]
end
diff --git a/spec/lib/gitlab/request_profiler/profile_spec.rb b/spec/lib/gitlab/request_profiler/profile_spec.rb
new file mode 100644
index 00000000000..b37ee558e1a
--- /dev/null
+++ b/spec/lib/gitlab/request_profiler/profile_spec.rb
@@ -0,0 +1,59 @@
+require 'fast_spec_helper'
+
+describe Gitlab::RequestProfiler::Profile do
+ let(:profile) { described_class.new(filename) }
+
+ describe '.new' do
+ context 'using old filename' do
+ let(:filename) { '|api|v4|version.txt_1562854738.html' }
+
+ it 'returns valid data' do
+ expect(profile).to be_valid
+ expect(profile.request_path).to eq('/api/v4/version.txt')
+ expect(profile.time).to eq(Time.at(1562854738).utc)
+ expect(profile.type).to eq('html')
+ end
+ end
+
+ context 'using new filename' do
+ let(:filename) { '|api|v4|version.txt_1563547949_execution.html' }
+
+ it 'returns valid data' do
+ expect(profile).to be_valid
+ expect(profile.request_path).to eq('/api/v4/version.txt')
+ expect(profile.profile_mode).to eq('execution')
+ expect(profile.time).to eq(Time.at(1563547949).utc)
+ expect(profile.type).to eq('html')
+ end
+ end
+ end
+
+ describe '#content_type' do
+ context 'when using html file' do
+ let(:filename) { '|api|v4|version.txt_1562854738_memory.html' }
+
+ it 'returns valid data' do
+ expect(profile).to be_valid
+ expect(profile.content_type).to eq('text/html')
+ end
+ end
+
+ context 'when using text file' do
+ let(:filename) { '|api|v4|version.txt_1562854738_memory.txt' }
+
+ it 'returns valid data' do
+ expect(profile).to be_valid
+ expect(profile.content_type).to eq('text/plain')
+ end
+ end
+
+ context 'when file is unknown' do
+ let(:filename) { '|api|v4|version.txt_1562854738_memory.xxx' }
+
+ it 'returns valid data' do
+ expect(profile).not_to be_valid
+ expect(profile.content_type).to be_nil
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/request_profiler_spec.rb b/spec/lib/gitlab/request_profiler_spec.rb
index fd8cbf39bce..498c045b6cd 100644
--- a/spec/lib/gitlab/request_profiler_spec.rb
+++ b/spec/lib/gitlab/request_profiler_spec.rb
@@ -13,15 +13,42 @@ describe Gitlab::RequestProfiler do
end
end
- describe '.remove_all_profiles' do
- it 'removes Gitlab::RequestProfiler::PROFILES_DIR directory' do
- dir = described_class::PROFILES_DIR
- FileUtils.mkdir_p(dir)
+ context 'with temporary PROFILES_DIR' do
+ let(:tmpdir) { Dir.mktmpdir('profiler-test') }
+ let(:profile_name) { '|api|v4|version.txt_1562854738_memory.html' }
+ let(:profile_path) { File.join(tmpdir, profile_name) }
- expect(Dir.exist?(dir)).to be true
+ before do
+ stub_const('Gitlab::RequestProfiler::PROFILES_DIR', tmpdir)
+ FileUtils.touch(profile_path)
+ end
+
+ after do
+ FileUtils.rm_rf(tmpdir)
+ end
+
+ describe '.remove_all_profiles' do
+ it 'removes Gitlab::RequestProfiler::PROFILES_DIR directory' do
+ described_class.remove_all_profiles
+
+ expect(Dir.exist?(tmpdir)).to be false
+ end
+ end
+
+ describe '.all' do
+ subject { described_class.all }
+
+ it 'returns all profiles' do
+ expect(subject.map(&:name)).to contain_exactly(profile_name)
+ end
+ end
+
+ describe '.find' do
+ subject { described_class.find(profile_name) }
- described_class.remove_all_profiles
- expect(Dir.exist?(dir)).to be false
+ it 'returns all profiles' do
+ expect(subject.name).to eq(profile_name)
+ end
end
end
end