Skip to content

Commit 1b077d2

Browse files
author
Yorick Peterse
committed
Use custom code for instrumenting method calls
The use of ActiveSupport would slow down instrumented method calls by about 180x due to: 1. ActiveSupport itself not being the fastest thing on the planet 2. caller_locations() having quite some overhead The use of caller_locations() has been removed because it's not _that_ useful since we already know the full namespace of receivers and the names of the called methods. The use of ActiveSupport has been replaced with some custom code that's generated using eval() (which can be quite a bit faster than using define_method). This new setup results in instrumented methods only being about 35-40x slower (compared to non instrumented methods).
1 parent b66a16c commit 1b077d2

File tree

5 files changed

+47
-95
lines changed

5 files changed

+47
-95
lines changed

config/initializers/metrics.rb

-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
# ActiveSupport.
88
require 'gitlab/metrics/subscribers/action_view'
99
require 'gitlab/metrics/subscribers/active_record'
10-
require 'gitlab/metrics/subscribers/method_call'
1110

1211
Gitlab::Application.configure do |config|
1312
config.middleware.use(Gitlab::Metrics::RackMiddleware)

lib/gitlab/metrics/instrumentation.rb

+31-6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ module Metrics
99
#
1010
# Gitlab::Metrics::Instrumentation.instrument_method(User, :by_login)
1111
module Instrumentation
12+
SERIES = 'method_calls'
13+
1214
# Instruments a class method.
1315
#
1416
# mod - The module to instrument as a Module/Class.
@@ -31,19 +33,42 @@ def self.instrument(type, mod, name)
3133
alias_name = "_original_#{name}"
3234
target = type == :instance ? mod : mod.singleton_class
3335

36+
if type == :instance
37+
target = mod
38+
label = "#{mod.name}##{name}"
39+
else
40+
target = mod.singleton_class
41+
label = "#{mod.name}.#{name}"
42+
end
43+
3444
target.class_eval <<-EOF, __FILE__, __LINE__ + 1
3545
alias_method :#{alias_name}, :#{name}
3646
3747
def #{name}(*args, &block)
38-
ActiveSupport::Notifications
39-
.instrument("#{type}_method.method_call",
40-
module: #{mod.name.inspect},
41-
name: #{name.inspect}) do
42-
#{alias_name}(*args, &block)
43-
end
48+
trans = Gitlab::Metrics::Instrumentation.transaction
49+
50+
if trans
51+
start = Time.now
52+
retval = #{alias_name}(*args, &block)
53+
duration = (Time.now - start) * 1000.0
54+
55+
trans.add_metric(Gitlab::Metrics::Instrumentation::SERIES,
56+
{ duration: duration },
57+
method: #{label.inspect})
58+
59+
retval
60+
else
61+
#{alias_name}(*args, &block)
62+
end
4463
end
4564
EOF
4665
end
66+
67+
# Small layer of indirection to make it easier to stub out the current
68+
# transaction.
69+
def self.transaction
70+
Transaction.current
71+
end
4772
end
4873
end
4974
end

lib/gitlab/metrics/subscribers/method_call.rb

-42
This file was deleted.

spec/lib/gitlab/metrics/instrumentation_spec.rb

+16-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
require 'spec_helper'
22

33
describe Gitlab::Metrics::Instrumentation do
4+
let(:transaction) { Gitlab::Metrics::Transaction.new }
5+
46
before do
57
@dummy = Class.new do
68
def self.foo(text = 'foo')
@@ -31,9 +33,13 @@ def bar(text = 'bar')
3133
expect(@dummy.foo).to eq('foo')
3234
end
3335

34-
it 'fires an ActiveSupport notification upon calling the method' do
35-
expect(ActiveSupport::Notifications).to receive(:instrument).
36-
with('class_method.method_call', module: 'Dummy', name: :foo)
36+
it 'tracks the call duration upon calling the method' do
37+
allow(Gitlab::Metrics::Instrumentation).to receive(:transaction).
38+
and_return(transaction)
39+
40+
expect(transaction).to receive(:add_metric).
41+
with(described_class::SERIES, an_instance_of(Hash),
42+
method: 'Dummy.foo')
3743

3844
@dummy.foo
3945
end
@@ -69,9 +75,13 @@ def bar(text = 'bar')
6975
expect(@dummy.new.bar).to eq('bar')
7076
end
7177

72-
it 'fires an ActiveSupport notification upon calling the method' do
73-
expect(ActiveSupport::Notifications).to receive(:instrument).
74-
with('instance_method.method_call', module: 'Dummy', name: :bar)
78+
it 'tracks the call duration upon calling the method' do
79+
allow(Gitlab::Metrics::Instrumentation).to receive(:transaction).
80+
and_return(transaction)
81+
82+
expect(transaction).to receive(:add_metric).
83+
with(described_class::SERIES, an_instance_of(Hash),
84+
method: 'Dummy#bar')
7585

7686
@dummy.new.bar
7787
end

spec/lib/gitlab/metrics/subscribers/method_call_spec.rb

-40
This file was deleted.

0 commit comments

Comments
 (0)