Refactor by removing some redundant variables

target is already defined in the if clause below. There's no need to
cast name to symbol. All methods that use name variable accept string or
symbol

changelogs/unreleased/respect_visiblity_instrument_methods.yml

+---
+title: Respect original visibility for instrumented methods
+merge_request: 39951
+author:
+type: fixed

lib/gitlab/metrics/instrumentation.rb

@@ -120,9 +120,6 @@ module Gitlab
       def self.instrument(type, mod, name)
         return unless ::Gitlab::Metrics.enabled?
 
-        name = name.to_sym
-        target = type == :instance ? mod : mod.singleton_class
-
         if type == :instance
           target = mod
           method_name = "##{name}"
@@ -154,6 +151,8 @@ module Gitlab
             '*args'
           end
 
+        method_visibility = method_visibility_for(target, name)
+
         proxy_module.class_eval <<-EOF, __FILE__, __LINE__ + 1
           def #{name}(#{args_signature})
             if trans = Gitlab::Metrics::Instrumentation.transaction
@@ -163,11 +162,23 @@ module Gitlab
               super
             end
           end
+          #{method_visibility} :#{name}
         EOF
 
         target.prepend(proxy_module)
       end
 
+      def self.method_visibility_for(mod, name)
+        if mod.private_method_defined?(name)
+          :private
+        elsif mod.protected_method_defined?(name)
+          :protected
+        else
+          :public
+        end
+      end
+      private_class_method :method_visibility_for
+
       # Small layer of indirection to make it easier to stub out the current
       # transaction.
       def self.transaction

spec/lib/gitlab/metrics/instrumentation_spec.rb

@@ -12,6 +12,11 @@ RSpec.describe Gitlab::Metrics::Instrumentation do
         text
       end
 
+      def self.wat(text = 'wat')
+        text
+      end
+      private_class_method :wat
+
       class << self
         def buzz(text = 'buzz')
           text
@@ -242,6 +247,7 @@ RSpec.describe Gitlab::Metrics::Instrumentation do
 
       expect(described_class.instrumented?(@dummy.singleton_class)).to eq(true)
       expect(@dummy.method(:foo).source_location.first).to match(/instrumentation\.rb/)
+      expect(@dummy.public_methods).to include(:foo)
     end
 
     it 'instruments all protected class methods' do
@@ -249,13 +255,16 @@ RSpec.describe Gitlab::Metrics::Instrumentation do
 
       expect(described_class.instrumented?(@dummy.singleton_class)).to eq(true)
       expect(@dummy.method(:flaky).source_location.first).to match(/instrumentation\.rb/)
+      expect(@dummy.protected_methods).to include(:flaky)
     end
 
-    it 'instruments all private instance methods' do
+    it 'instruments all private class 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/)
+      expect(@dummy.private_methods).to include(:buzz)
+      expect(@dummy.private_methods).to include(:wat)
     end
 
     it 'only instruments methods directly defined in the module' do
@@ -290,6 +299,7 @@ RSpec.describe Gitlab::Metrics::Instrumentation do
 
       expect(described_class.instrumented?(@dummy)).to eq(true)
       expect(@dummy.new.method(:bar).source_location.first).to match(/instrumentation\.rb/)
+      expect(@dummy.public_instance_methods).to include(:bar)
     end
 
     it 'instruments all protected instance methods' do
@@ -297,6 +307,7 @@ RSpec.describe Gitlab::Metrics::Instrumentation do
 
       expect(described_class.instrumented?(@dummy)).to eq(true)
       expect(@dummy.new.method(:chaf).source_location.first).to match(/instrumentation\.rb/)
+      expect(@dummy.protected_instance_methods).to include(:chaf)
     end
 
     it 'instruments all private instance methods' do
@@ -304,6 +315,7 @@ RSpec.describe Gitlab::Metrics::Instrumentation do
 
       expect(described_class.instrumented?(@dummy)).to eq(true)
       expect(@dummy.new.method(:wadus).source_location.first).to match(/instrumentation\.rb/)
+      expect(@dummy.private_instance_methods).to include(:wadus)
     end
 
     it 'only instruments methods directly defined in the module' do