Commit 9575920a authored by nmilojevic1's avatar nmilojevic1

Addressed MR comments

- Add missing specs for measuring
- Add variables as attr_accessors
- Renamed some attributes
parent 4cbfaa06
...@@ -13,7 +13,7 @@ module Projects ...@@ -13,7 +13,7 @@ module Projects
measurement_enabled = !!options[:measurement_enabled] measurement_enabled = !!options[:measurement_enabled]
measurement_logger = options[:measurement_logger] measurement_logger = options[:measurement_logger]
::Gitlab::Utils::Measuring.execute_with(measurement_enabled, measurement_logger, base_data) do ::Gitlab::Utils::Measuring.execute_with(measurement_enabled, measurement_logger, base_log_data) do
save_all! save_all!
execute_after_export_action(after_export_strategy) execute_after_export_action(after_export_strategy)
end end
...@@ -25,12 +25,12 @@ module Projects ...@@ -25,12 +25,12 @@ module Projects
attr_accessor :shared attr_accessor :shared
def base_data def base_log_data
{ {
class: self.class.name, class: self.class.name,
current_user: current_user.name, current_user: current_user.name,
project_full_path: project.full_path, project_full_path: project.full_path,
file_path: @shared.export_path file_path: shared.export_path
} }
end end
......
...@@ -12,16 +12,18 @@ module Gitlab ...@@ -12,16 +12,18 @@ module Gitlab
@namespace = Namespace.find_by_full_path(opts.fetch(:namespace_path)) @namespace = Namespace.find_by_full_path(opts.fetch(:namespace_path))
@current_user = User.find_by_username(opts.fetch(:username)) @current_user = User.find_by_username(opts.fetch(:username))
@measurement_enabled = opts.fetch(:measurement_enabled) @measurement_enabled = opts.fetch(:measurement_enabled)
@measurement = Gitlab::Utils::Measuring.new(logger: logger) if @measurement_enabled
@logger = logger @logger = logger
end end
private private
attr_reader :measurement, :project, :namespace, :current_user, :file_path, :project_path, :logger attr_reader :project, :namespace, :current_user, :file_path, :project_path, :logger, :measurement_enabled
def measurement_enabled? def measurement_options
@measurement_enabled {
measurement_enabled: measurement_enabled,
measurement_logger: logger
}
end end
def success(message) def success(message)
...@@ -30,13 +32,6 @@ module Gitlab ...@@ -30,13 +32,6 @@ module Gitlab
true true
end end
def measurement_options
{
measurement_enabled: measurement_enabled?,
measurement_logger: logger
}
end
def error(message) def error(message)
logger.error(message) logger.error(message)
......
...@@ -23,10 +23,11 @@ module Gitlab ...@@ -23,10 +23,11 @@ module Gitlab
end end
def with_measuring def with_measuring
result = with_gc_stats do result = nil
with_gc_stats do
with_count_queries do with_count_queries do
with_measure_time do with_measure_time do
yield result = yield
end end
end end
end end
...@@ -56,31 +57,27 @@ module Gitlab ...@@ -56,31 +57,27 @@ module Gitlab
ActiveSupport::Notifications.subscribed(counter_f, "sql.active_record", &block) ActiveSupport::Notifications.subscribed(counter_f, "sql.active_record", &block)
end end
def log_info(details)
details = base_log_data.merge(details)
details = details.to_yaml if ActiveSupport::Logger.logger_outputs_to?(logger, STDOUT)
logger.info(details)
end
def with_gc_stats def with_gc_stats
GC.start # perform a full mark-and-sweep GC.start # perform a full mark-and-sweep
stats_before = GC.stat stats_before = GC.stat
result = yield yield
stats_after = GC.stat stats_after = GC.stat
@gc_stats = stats_after.map do |key, after_value| @gc_stats = stats_after.map do |key, after_value|
before_value = stats_before[key] before_value = stats_before[key]
[key, before: before_value, after: after_value, diff: after_value - before_value] [key, before: before_value, after: after_value, diff: after_value - before_value]
end.to_h end.to_h
result
end end
def with_measure_time def with_measure_time
result = nil
@time_to_finish = Benchmark.realtime do @time_to_finish = Benchmark.realtime do
result = yield yield
end
end end
result def log_info(details)
details = base_log_data.merge(details)
details = details.to_yaml if ActiveSupport::Logger.logger_outputs_to?(logger, STDOUT)
logger.info(details)
end end
def duration_in_numbers(duration_in_seconds) def duration_in_numbers(duration_in_seconds)
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'fast_spec_helper' require 'fast_spec_helper'
describe Gitlab::Utils::Measuring do describe Gitlab::Utils::Measuring do
describe '#execute_with' do describe '.execute_with' do
let(:measurement_logger) { double(:logger) } let(:measurement_logger) { double(:logger) }
let(:base_log_data) do let(:base_log_data) do
{ {
......
...@@ -179,7 +179,7 @@ describe Projects::ImportExport::ExportService do ...@@ -179,7 +179,7 @@ describe Projects::ImportExport::ExportService do
end end
context 'when measurable params are provided' do context 'when measurable params are provided' do
let(:base_data) do let(:base_log_data) do
{ {
class: described_class.name, class: described_class.name,
current_user: user.name, current_user: user.name,
...@@ -204,7 +204,7 @@ describe Projects::ImportExport::ExportService do ...@@ -204,7 +204,7 @@ describe Projects::ImportExport::ExportService do
end end
it 'measure service execution with Gitlab::Utils::Measuring' do it 'measure service execution with Gitlab::Utils::Measuring' do
expect(Gitlab::Utils::Measuring).to receive(:execute_with).with(true, logger, base_data).and_call_original expect(Gitlab::Utils::Measuring).to receive(:execute_with).with(true, logger, base_log_data).and_call_original
expect_next_instance_of(Gitlab::Utils::Measuring) do |measuring| expect_next_instance_of(Gitlab::Utils::Measuring) do |measuring|
expect(measuring).to receive(:with_measuring).and_call_original expect(measuring).to receive(:with_measuring).and_call_original
end end
...@@ -221,7 +221,7 @@ describe Projects::ImportExport::ExportService do ...@@ -221,7 +221,7 @@ describe Projects::ImportExport::ExportService do
end end
it 'does not measure service execution' do it 'does not measure service execution' do
expect(Gitlab::Utils::Measuring).to receive(:execute_with).with(false, nil, base_data).and_call_original expect(Gitlab::Utils::Measuring).to receive(:execute_with).with(false, nil, base_log_data).and_call_original
expect(Gitlab::Utils::Measuring).not_to receive(:new) expect(Gitlab::Utils::Measuring).not_to receive(:new)
service.execute(after_export_strategy, measurable_options) service.execute(after_export_strategy, measurable_options)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment