diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index bde04232725ba1393be05c35eb25550dbbf2d804..556e968137ebf8a40a93c402e9d91cf9cfff3569 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -7,7 +7,6 @@ if Gitlab::Metrics.enabled? # ActiveSupport. require 'gitlab/metrics/subscribers/action_view' require 'gitlab/metrics/subscribers/active_record' - require 'gitlab/metrics/subscribers/method_call' Gitlab::Application.configure do |config| config.middleware.use(Gitlab::Metrics::RackMiddleware) diff --git a/lib/gitlab/metrics/instrumentation.rb b/lib/gitlab/metrics/instrumentation.rb index 982e35adfc98f48c5edfaf8551d3048cb8c0ca8f..5b56f2e851316268279db241f2a5205e33ce759f 100644 --- a/lib/gitlab/metrics/instrumentation.rb +++ b/lib/gitlab/metrics/instrumentation.rb @@ -9,6 +9,8 @@ module Gitlab # # Gitlab::Metrics::Instrumentation.instrument_method(User, :by_login) module Instrumentation + SERIES = 'method_calls' + # Instruments a class method. # # mod - The module to instrument as a Module/Class. @@ -31,19 +33,42 @@ module Gitlab alias_name = "_original_#{name}" target = type == :instance ? mod : mod.singleton_class + if type == :instance + target = mod + label = "#{mod.name}##{name}" + else + target = mod.singleton_class + label = "#{mod.name}.#{name}" + end + target.class_eval <<-EOF, __FILE__, __LINE__ + 1 alias_method :#{alias_name}, :#{name} def #{name}(*args, &block) - ActiveSupport::Notifications - .instrument("#{type}_method.method_call", - module: #{mod.name.inspect}, - name: #{name.inspect}) do - #{alias_name}(*args, &block) - end + trans = Gitlab::Metrics::Instrumentation.transaction + + if trans + start = Time.now + retval = #{alias_name}(*args, &block) + duration = (Time.now - start) * 1000.0 + + trans.add_metric(Gitlab::Metrics::Instrumentation::SERIES, + { duration: duration }, + method: #{label.inspect}) + + retval + else + #{alias_name}(*args, &block) + end end EOF end + + # Small layer of indirection to make it easier to stub out the current + # transaction. + def self.transaction + Transaction.current + end end end end diff --git a/lib/gitlab/metrics/subscribers/method_call.rb b/lib/gitlab/metrics/subscribers/method_call.rb deleted file mode 100644 index 0094ed0dc6a62da440457e972572766779695f8b..0000000000000000000000000000000000000000 --- a/lib/gitlab/metrics/subscribers/method_call.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Gitlab - module Metrics - module Subscribers - # Class for tracking method call timings. - class MethodCall < ActiveSupport::Subscriber - attach_to :method_call - - SERIES = 'method_calls' - - def instance_method(event) - return unless current_transaction - - label = "#{event.payload[:module]}##{event.payload[:name]}" - - add_metric(label, event.duration) - end - - def class_method(event) - return unless current_transaction - - label = "#{event.payload[:module]}.#{event.payload[:name]}" - - add_metric(label, event.duration) - end - - private - - def add_metric(label, duration) - file, line = Metrics.last_relative_application_frame - - values = { duration: duration, file: file, line: line } - - current_transaction.add_metric(SERIES, values, method: label) - end - - def current_transaction - Transaction.current - end - end - end - end -end diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index 3e7e9869e30327c26323a386a4f8ec25b881d45d..9308fb157d8081dd9ee25f19590bbfb3e63b24c3 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Metrics::Instrumentation do + let(:transaction) { Gitlab::Metrics::Transaction.new } + before do @dummy = Class.new do def self.foo(text = 'foo') @@ -31,9 +33,13 @@ describe Gitlab::Metrics::Instrumentation do expect(@dummy.foo).to eq('foo') end - it 'fires an ActiveSupport notification upon calling the method' do - expect(ActiveSupport::Notifications).to receive(:instrument). - with('class_method.method_call', module: 'Dummy', name: :foo) + it 'tracks the call duration upon calling the method' do + allow(Gitlab::Metrics::Instrumentation).to receive(:transaction). + and_return(transaction) + + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, an_instance_of(Hash), + method: 'Dummy.foo') @dummy.foo end @@ -69,9 +75,13 @@ describe Gitlab::Metrics::Instrumentation do expect(@dummy.new.bar).to eq('bar') end - it 'fires an ActiveSupport notification upon calling the method' do - expect(ActiveSupport::Notifications).to receive(:instrument). - with('instance_method.method_call', module: 'Dummy', name: :bar) + it 'tracks the call duration upon calling the method' do + allow(Gitlab::Metrics::Instrumentation).to receive(:transaction). + and_return(transaction) + + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, an_instance_of(Hash), + method: 'Dummy#bar') @dummy.new.bar end diff --git a/spec/lib/gitlab/metrics/subscribers/method_call_spec.rb b/spec/lib/gitlab/metrics/subscribers/method_call_spec.rb deleted file mode 100644 index 5c76eb3ba50420c14c1f8b25e39adc9793be416d..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/metrics/subscribers/method_call_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Metrics::Subscribers::MethodCall do - let(:transaction) { Gitlab::Metrics::Transaction.new } - - let(:subscriber) { described_class.new } - - let(:event) do - double(:event, duration: 0.2, payload: { module: 'Foo', name: :foo }) - end - - before do - allow(subscriber).to receive(:current_transaction).and_return(transaction) - - allow(Gitlab::Metrics).to receive(:last_relative_application_frame). - and_return(['app/models/foo.rb', 4]) - end - - describe '#instance_method' do - it 'tracks the execution of an instance method' do - values = { duration: 0.2, file: 'app/models/foo.rb', line: 4 } - - expect(transaction).to receive(:add_metric). - with(described_class::SERIES, values, method: 'Foo#foo') - - subscriber.instance_method(event) - end - end - - describe '#class_method' do - it 'tracks the execution of a class method' do - values = { duration: 0.2, file: 'app/models/foo.rb', line: 4 } - - expect(transaction).to receive(:add_metric). - with(described_class::SERIES, values, method: 'Foo.foo') - - subscriber.class_method(event) - end - end -end