Merge branch 'backward-compatible-request-profiles' into 'master'
Bring backward compatibility for request profiles See merge request gitlab-org/gitlab-ce!30956
This commit is contained in:
commit
9cffa428ad
7 changed files with 130 additions and 32 deletions
|
@ -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'
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
59
spec/lib/gitlab/request_profiler/profile_spec.rb
Normal file
59
spec/lib/gitlab/request_profiler/profile_spec.rb
Normal file
|
@ -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
|
|
@ -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
|
||||
|
||||
described_class.remove_all_profiles
|
||||
expect(Dir.exist?(dir)).to be false
|
||||
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) }
|
||||
|
||||
it 'returns all profiles' do
|
||||
expect(subject.name).to eq(profile_name)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue