diff --git a/CHANGELOG b/CHANGELOG
index e71a154d1d5cd3853e015b77b66c6715d62808e0..74fb52d3aebbf0a400ccf6caf6e5f63a1cb156b5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -78,6 +78,7 @@ v 8.9.0 (unreleased)
- Remove deprecated issues_tracker and issues_tracker_id from project model
- Allow users to create confidential issues in private projects
- Measure CPU time for instrumented methods
+ - Instrument private methods and private instance methods by default instead just public methods
v 8.8.5 (unreleased)
- Ensure branch cleanup regardless of whether the GitHub import process succeeds
diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb
index f6509ee43f1f419c0130baf607a42098393ab735..989404c6a615e8f6c5af5c3345b24cbed9061912 100644
--- a/config/initializers/metrics.rb
+++ b/config/initializers/metrics.rb
@@ -128,11 +128,6 @@ if Gitlab::Metrics.enabled?
config.instrument_instance_methods(API::Helpers)
config.instrument_instance_methods(RepositoryCheck::SingleRepositoryWorker)
- # Iterate over each non-super private instance method to keep up to date if
- # internals change
- RepositoryCheck::SingleRepositoryWorker.private_instance_methods(false).each do |method|
- config.instrument_instance_method(RepositoryCheck::SingleRepositoryWorker, method)
- end
end
GC::Profiler.enable
diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md
index 50d2866ca46df92486315383b455b6516c303e9a..6cd9b274d115a86815e55a7bc43d4c56d9b94b15 100644
--- a/doc/development/instrumentation.md
+++ b/doc/development/instrumentation.md
@@ -15,8 +15,8 @@ instrument code:
* `instrument_instance_method`: instruments a single instance method.
* `instrument_class_hierarchy`: given a Class this method will recursively
instrument all sub-classes (both class and instance methods).
-* `instrument_methods`: instruments all public class methods of a Module.
-* `instrument_instance_methods`: instruments all public instance methods of a
+* `instrument_methods`: instruments all public and private class methods of a Module.
+* `instrument_instance_methods`: instruments all public and private instance methods of a
Module.
To remove the need for typing the full `Gitlab::Metrics::Instrumentation`
diff --git a/lib/gitlab/metrics/instrumentation.rb b/lib/gitlab/metrics/instrumentation.rb
index ad9ce3fa442b7fb0be1ee014005af6fce93863ba..d81d26754fe004934a19cbcdd3b6f8d17d0bd981 100644
--- a/lib/gitlab/metrics/instrumentation.rb
+++ b/lib/gitlab/metrics/instrumentation.rb
@@ -56,7 +56,7 @@ module Gitlab
end
end
- # Instruments all public methods of a module.
+ # Instruments all public and private methods of a module.
#
# This method optionally takes a block that can be used to determine if a
# method should be instrumented or not. The block is passed the receiving
@@ -65,7 +65,8 @@ module Gitlab
#
# mod - The module to instrument.
def self.instrument_methods(mod)
- mod.public_methods(false).each do |name|
+ methods = mod.methods(false) + mod.private_methods(false)
+ methods.each do |name|
method = mod.method(name)
if method.owner == mod.singleton_class
@@ -76,13 +77,14 @@ module Gitlab
end
end
- # Instruments all public instance methods of a module.
+ # Instruments all public and private instance methods of a module.
#
# See `instrument_methods` for more information.
#
# mod - The module to instrument.
def self.instrument_instance_methods(mod)
- mod.public_instance_methods(false).each do |name|
+ methods = mod.instance_methods(false) + mod.private_instance_methods(false)
+ methods.each do |name|
method = mod.instance_method(name)
if method.owner == mod
diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb
index c6e979b69a45ac20b93dcf31c68394cd30cc04a8..cdf641341cbe6ba56b7c2d12c21477962a99b0b7 100644
--- a/spec/lib/gitlab/metrics/instrumentation_spec.rb
+++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb
@@ -9,9 +9,31 @@ describe Gitlab::Metrics::Instrumentation do
text
end
+ class << self
+ def buzz(text = 'buzz')
+ text
+ end
+ private :buzz
+
+ def flaky(text = 'flaky')
+ text
+ end
+ protected :flaky
+ end
+
def bar(text = 'bar')
text
end
+
+ def wadus(text = 'wadus')
+ text
+ end
+ private :wadus
+
+ def chaf(text = 'chaf')
+ text
+ end
+ protected :chaf
end
allow(@dummy).to receive(:name).and_return('Dummy')
@@ -208,6 +230,21 @@ describe Gitlab::Metrics::Instrumentation do
described_class.instrument_methods(@dummy)
expect(described_class.instrumented?(@dummy.singleton_class)).to eq(true)
+ expect(@dummy.method(:foo).source_location.first).to match(/instrumentation\.rb/)
+ end
+
+ it 'instruments all protected class methods' do
+ described_class.instrument_methods(@dummy)
+
+ expect(described_class.instrumented?(@dummy.singleton_class)).to eq(true)
+ expect(@dummy.method(:flaky).source_location.first).to match(/instrumentation\.rb/)
+ end
+
+ it 'instruments all private instance methods' do
+ described_class.instrument_methods(@dummy)
+
+ expect(described_class.instrumented?(@dummy.singleton_class)).to eq(true)
+ expect(@dummy.method(:buzz).source_location.first).to match(/instrumentation\.rb/)
end
it 'only instruments methods directly defined in the module' do
@@ -241,6 +278,21 @@ describe Gitlab::Metrics::Instrumentation do
described_class.instrument_instance_methods(@dummy)
expect(described_class.instrumented?(@dummy)).to eq(true)
+ expect(@dummy.new.method(:bar).source_location.first).to match(/instrumentation\.rb/)
+ end
+
+ it 'instruments all protected instance methods' do
+ described_class.instrument_instance_methods(@dummy)
+
+ expect(described_class.instrumented?(@dummy)).to eq(true)
+ expect(@dummy.new.method(:chaf).source_location.first).to match(/instrumentation\.rb/)
+ end
+
+ it 'instruments all private instance methods' do
+ described_class.instrument_instance_methods(@dummy)
+
+ expect(described_class.instrumented?(@dummy)).to eq(true)
+ expect(@dummy.new.method(:wadus).source_location.first).to match(/instrumentation\.rb/)
end
it 'only instruments methods directly defined in the module' do
@@ -253,7 +305,7 @@ describe Gitlab::Metrics::Instrumentation do
described_class.instrument_instance_methods(@dummy)
- expect(@dummy.method_defined?(:_original_kittens)).to eq(false)
+ expect(@dummy.new.method(:kittens).source_location.first).not_to match(/instrumentation\.rb/)
end
it 'can take a block to determine if a method should be instrumented' do
@@ -261,7 +313,7 @@ describe Gitlab::Metrics::Instrumentation do
false
end
- expect(@dummy.method_defined?(:_original_bar)).to eq(false)
+ expect(@dummy.new.method(:bar).source_location.first).not_to match(/instrumentation\.rb/)
end
end
end