Removal of Ruby bridge classes for Gauge and Counter (#17858)

 - Update `Collector#push` to avoid dynamic dispatch of method invocation, explicitly invoke set on Gauge and increment on Counter.
 - Remove Ruby classes Gauge and Counter replacing by existing Java classes LongCounter and LazyDelegatingGauge which was wrapped by Ruby ones.

Co-authored-by: João Duarte <[email protected]>

Author: andsel

logstash-core/lib/logstash/instrument/collector.rb

@@ -45,9 +45,16 @@ def initialize
     def push(namespaces_path, key, type, *metric_type_params)
       begin
         metric_proxy = get(namespaces_path, key, type)
-        return metric_proxy.execute(*metric_type_params) if metric_proxy.respond_to?(:execute)
-
-        logger.error("Collector: Cannot record metric action #{type}@#{metric_type_params.join('/')} on <#{metric_proxy}> at path #{namespaces_path.join('/')}/#{key}")
+        _, metric_arg = metric_type_params
+        case type
+        when :gauge
+          return metric_proxy.set(metric_arg)
+        when :counter
+          return metric_proxy.increment if metric_arg.nil?
+          return metric_proxy.increment(metric_arg)
+        else
+          logger.error("Collector: Cannot record metric action #{type}@#{metric_type_params.join('/')} on <#{metric_proxy}> at path #{namespaces_path.join('/')}/#{key}")
+        end
       rescue MetricStore::NamespacesExpectedError => e
         logger.error("Collector: Cannot record metric", :exception => e)
       rescue NameError => e

logstash-core/lib/logstash/instrument/metric_type.rb

@@ -15,9 +15,6 @@
 # specific language governing permissions and limitations
 # under the License.
 
-require "logstash/instrument/metric_type/counter"
-require "logstash/instrument/metric_type/gauge"
-
 module LogStash module Instrument
   module MetricType
     # Use the string to generate a concrete class for this metrics
@@ -28,8 +25,8 @@ module MetricType
     # @raise [NameError] If the class is not found
     def self.create(type, namespaces, key)
       case type
-      when :counter then return LogStash::Instrument::MetricType::Counter.new(namespaces, key)
-      when :gauge   then return LogStash::Instrument::MetricType::Gauge.new(namespaces, key)
+      when :counter then return org.logstash.instrument.metrics.counter.LongCounter.new(key.to_s)
+      when :gauge   then return org.logstash.instrument.metrics.gauge.LazyDelegatingGauge.new(key.to_s)
       when :uptime  then return org.logstash.instrument.metrics.UptimeMetric.new(key.to_s)
       when :timer   then return org.logstash.instrument.metrics.timer.TimerMetric::create(key.to_s)
       else fail NameError, "Unknown Metric Type `#{type}`"

logstash-core/lib/logstash/instrument/metric_type/counter.rb

@@ -62,8 +62,8 @@
     let(:key) { :my_key }
 
     {
-      counter: LogStash::Instrument::MetricType::Counter,
-      gauge:   LogStash::Instrument::MetricType::Gauge,
+      counter: org.logstash.instrument.metrics.counter.LongCounter,
+      gauge:   org.logstash.instrument.metrics.gauge.LazyDelegatingGauge,
       uptime:  org.logstash.instrument.metrics.UptimeMetric,
       timer:   org.logstash.instrument.metrics.timer.TimerMetric,
     }.each do |type, type_specific_implementation|

logstash-core/lib/logstash/instrument/metric_type/gauge.rb

@@ -20,7 +20,7 @@
 describe LogStash::Instrument::MetricStore do
   let(:namespaces) { [:root, :pipelines, :pipeline_01] }
   let(:key) { :events_in }
-  let(:counter) { LogStash::Instrument::MetricType::Counter.new(namespaces, key) }
+  let(:counter) { org.logstash.instrument.metrics.counter.LongCounter.new(key.to_s) }
 
   context "when the metric object doesn't exist" do
     it "store the object" do
@@ -33,7 +33,7 @@
   end
 
   context "when the metric object exist in the namespace"  do
-    let(:new_counter) { LogStash::Instrument::MetricType::Counter.new(namespaces, key) }
+    let(:new_counter) { org.logstash.instrument.metrics.counter.LongCounter.new(key.to_s) }
 
     it "return the object" do
       subject.fetch_or_store(namespaces, key, counter)
@@ -53,18 +53,18 @@
   context "retrieving events" do
     let(:metric_events) {
       [
-        [[:node, :sashimi, :pipelines, :pipeline01, :plugins, :"logstash-output-elasticsearch"], :event_in, :increment],
-        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_in, :increment],
-        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_out, :increment],
-        [[:node, :sashimi, :pipelines, :pipeline02], :processed_events_out, :increment],
+        [[:node, :sashimi, :pipelines, :pipeline01, :plugins, :"logstash-output-elasticsearch"], :event_in],
+        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_in],
+        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_out],
+        [[:node, :sashimi, :pipelines, :pipeline02], :processed_events_out],
       ]
     }
 
     before :each do
       # Lets add a few metrics in the store before trying to find them
-      metric_events.each do |namespaces, metric_key, action|
-        metric = subject.fetch_or_store(namespaces, metric_key, LogStash::Instrument::MetricType::Counter.new(namespaces, metric_key))
-        metric.execute(action)
+      metric_events.each do |namespaces, metric_key|
+        metric = subject.fetch_or_store(namespaces, metric_key, org.logstash.instrument.metrics.counter.LongCounter.new(metric_key.to_s))
+        metric.increment
       end
     end
 
@@ -96,7 +96,7 @@
 
         it "allow to retrieve a specific metrics" do
           metrics = subject.get(:node, :sashimi, :pipelines, :pipeline01, :plugins, :"logstash-output-elasticsearch", :event_in)
-          expect(metrics).to match(a_hash_including(:node => a_hash_including(:sashimi => a_hash_including(:pipelines => a_hash_including(:pipeline01 => a_hash_including(:plugins => a_hash_including(:"logstash-output-elasticsearch" => a_hash_including(:event_in => be_kind_of(LogStash::Instrument::MetricType::Counter)))))))))
+          expect(metrics).to match(a_hash_including(:node => a_hash_including(:sashimi => a_hash_including(:pipelines => a_hash_including(:pipeline01 => a_hash_including(:plugins => a_hash_including(:"logstash-output-elasticsearch" => a_hash_including(:event_in => be_kind_of(org.logstash.instrument.metrics.counter.LongCounter)))))))))
         end
 
         context "with filtered keys" do
@@ -142,7 +142,7 @@
 
           it "allow to retrieve a specific metrics" do
             metrics = subject.get_with_path("node/sashimi/pipelines/pipeline01/plugins/logstash-output-elasticsearch/event_in")
-            expect(metrics).to match(a_hash_including(:node => a_hash_including(:sashimi => a_hash_including(:pipelines => a_hash_including(:pipeline01 => a_hash_including(:plugins => a_hash_including(:"logstash-output-elasticsearch" => a_hash_including(:event_in => be_kind_of(LogStash::Instrument::MetricType::Counter)))))))))
+            expect(metrics).to match(a_hash_including(:node => a_hash_including(:sashimi => a_hash_including(:pipelines => a_hash_including(:pipeline01 => a_hash_including(:plugins => a_hash_including(:"logstash-output-elasticsearch" => a_hash_including(:event_in => be_kind_of(org.logstash.instrument.metrics.counter.LongCounter)))))))))
           end
 
           context "with filtered keys" do
@@ -260,18 +260,18 @@
   describe "#prune" do
     let(:metric_events) {
       [
-        [[:node, :sashimi, :pipelines, :pipeline01, :plugins, :"logstash-output-elasticsearch"], :event_in, :increment],
-        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_in, :increment],
-        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_out, :increment],
-        [[:node, :sashimi, :pipelines, :pipeline02], :processed_events_out, :increment],
+        [[:node, :sashimi, :pipelines, :pipeline01, :plugins, :"logstash-output-elasticsearch"], :event_in],
+        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_in],
+        [[:node, :sashimi, :pipelines, :pipeline01], :processed_events_out],
+        [[:node, :sashimi, :pipelines, :pipeline02], :processed_events_out],
       ]
     }
 
     before :each do
       # Lets add a few metrics in the store before trying to find them
-      metric_events.each do |namespaces, metric_key, action|
-        metric = subject.fetch_or_store(namespaces, metric_key, LogStash::Instrument::MetricType::Counter.new(namespaces, metric_key))
-        metric.execute(action)
+      metric_events.each do |namespaces, metric_key|
+        metric = subject.fetch_or_store(namespaces, metric_key, org.logstash.instrument.metrics.counter.LongCounter.new(metric_key.to_s))
+        metric.increment
       end
     end
 

logstash-core/spec/logstash/instrument/collector_spec.rb

@@ -15,14 +15,13 @@
 # specific language governing permissions and limitations
 # under the License.
 
-require "logstash/instrument/metric_type/counter"
+require "logstash/json"
 require "spec_helper"
 
-describe LogStash::Instrument::MetricType::Counter do
-  let(:namespaces) { [:root, :pipelines, :pipeline_01] }
+describe org.logstash.instrument.metrics.counter.LongCounter do
   let(:key) { :mykey }
 
-  subject { LogStash::Instrument::MetricType::Counter.new(namespaces, key) }
+  subject { described_class.new(key.to_s) }
 
   describe "#increment" do
     it "increment the counter" do

logstash-core/spec/logstash/instrument/metric_store_spec.rb

@@ -15,19 +15,17 @@
 # specific language governing permissions and limitations
 # under the License.
 
-require "logstash/instrument/metric_type/gauge"
 require "logstash/json"
 require "spec_helper"
 
-describe LogStash::Instrument::MetricType::Gauge do
-  let(:namespaces) { [:root, :pipelines, :pipeline_01] }
+describe org.logstash.instrument.metrics.gauge.LazyDelegatingGauge do
   let(:key) { :mykey }
   let(:value) { "hello" }
 
-  subject { described_class.new(namespaces, key) }
+  subject { described_class.new(key.to_s) }
 
   before :each do
-    subject.execute(:set, value)
+    subject.set(value)
   end
 
   describe "#execute" do