allow the disabling of metric collection

if `metric.collect` is set to false then no metrics are collected
before this setting was necessarily set to true.

Fixes #6097

logstash-core/lib/logstash/agent.rb

@@ -162,7 +162,7 @@ class LogStash::Agent
                 @logger.debug("Agent: Configuring metric collection")
                 LogStash::Instrument::Metric.new(@collector)
               else
-                LogStash::Instrument::NullMetric.new
+                LogStash::Instrument::NullMetric.new(@collector)
               end
 
 

logstash-core/lib/logstash/environment.rb

@@ -23,7 +23,7 @@ module LogStash
            Setting::Boolean.new("config.test_and_exit", false),
            Setting::Boolean.new("config.reload.automatic", false),
            Setting::Numeric.new("config.reload.interval", 3), # in seconds
-           Setting::Boolean.new("metric.collect", true) {|v| v == true }, # metric collection cannot be disabled
+           Setting::Boolean.new("metric.collect", true),
             Setting::String.new("pipeline.id", "main"),
    Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum),
    Setting::PositiveInteger.new("pipeline.output.workers", 1),

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

@@ -0,0 +1,54 @@
+# encoding: utf-8
+require "logstash/instrument/null_metric"
+
+module LogStash module Instrument
+  # This class acts a a proxy between the metric library and the user calls.
+  #
+  # This is the class that plugins authors will use to interact with the `MetricStore`
+  # It has the same public interface as `Metric` class but doesnt require to send
+  # the namespace on every call.
+  #
+  # @see Logstash::Instrument::Metric
+  class NamespacedNullMetric
+    attr_reader :namespace_name
+    # Create metric with a specific namespace
+    #
+    # @param metric [LogStash::Instrument::Metric] The metric instance to proxy
+    # @param namespace [Array] The namespace to use
+    def initialize(metric = nil, namespace_name = :null)
+      @metric = metric
+      @namespace_name = Array(namespace_name)
+    end
+
+    def increment(key, value = 1)
+    end
+
+    def decrement(key, value = 1)
+    end
+
+    def gauge(key, value)
+    end
+
+    def report_time(key, duration)
+    end
+
+    def time(key, &block)
+      if block_given?
+        yield
+      else
+        ::LogStash::Instrument::NullMetric::NullTimedExecution
+      end
+    end
+
+    def collector
+      @metric.collector
+    end
+
+    def namespace(name)
+      NamespacedNullMetric.new(metric, namespace_name + Array(name))
+    end
+
+    private
+    attr_reader :metric
+  end
+end; end

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

@@ -2,50 +2,59 @@
 require "logstash/instrument/metric"
 
 module LogStash module Instrument
- # This class is used in the context when we disable the metric collection
+  # This class is used in the context when we disable the metric collection
- # for specific plugin to replace the `NamespacedMetric` class with this one
+  # for specific plugin to replace the `NamespacedMetric` class with this one
- # which doesn't produce any metric to the collector.
+  # which doesn't produce any metric to the collector.
- class NullMetric
+  class NullMetric
-   attr_reader :namespace_name, :collector
+    attr_reader :namespace_name, :collector
 
-   def increment(key, value = 1)
+    def initialize(collector = nil)
-     Metric.validate_key!(key)
+      @collector = collector
-   end
+    end
 
-   def decrement(key, value = 1)
+    def increment(namespace, key, value = 1)
-     Metric.validate_key!(key)
+      Metric.validate_key!(key)
-   end
+    end
 
-   def gauge(key, value)
+    def decrement(namespace, key, value = 1)
-     Metric.validate_key!(key)
+      Metric.validate_key!(key)
-   end
+    end
 
-   def report_time(key, duration)
+    def gauge(namespace, key, value)
-     Metric.validate_key!(key)
+      Metric.validate_key!(key)
-   end
+    end
 
-   # We have to manually redefine this method since it can return an
+    def report_time(namespace, key, duration)
-   # object this object also has to be implemented as a NullObject
+      Metric.validate_key!(key)
-   def time(key)
+    end
-     Metric.validate_key!(key)
-     if block_given?
-       yield
-     else
-       NullTimedExecution
-     end
-   end
 
-   def namespace(key)
+    # We have to manually redefine this method since it can return an
-     self.class.new
+    # object this object also has to be implemented as a NullObject
-   end
+    def time(namespace, key)
+      Metric.validate_key!(key)
+      if block_given?
+        yield
+      else
+        NullTimedExecution
+      end
+    end
 
-   private
+    def namespace(name)
-   # Null implementation of the internal timer class
+      raise MetricNoNamespaceProvided if name.nil? || name.empty?
-   #
+      NamespacedNullMetric.new(self, name)
-   # @see LogStash::Instrument::TimedExecution`
+    end
-   class NullTimedExecution
+
-     def self.stop
+    def self.validate_key!(key)
-     end
+      raise MetricNoKeyProvided if key.nil? || key.empty?
-   end
+    end
- end
+
+    private
+    # Null implementation of the internal timer class
+    #
+    # @see LogStash::Instrument::TimedExecution`
+    class NullTimedExecution
+      def self.stop
+      end
+    end
+  end
 end; end

logstash-core/lib/logstash/pipeline.rb

@@ -15,6 +15,7 @@ require "logstash/pipeline_reporter"
 require "logstash/instrument/metric"
 require "logstash/instrument/namespaced_metric"
 require "logstash/instrument/null_metric"
+require "logstash/instrument/namespaced_null_metric"
 require "logstash/instrument/collector"
 require "logstash/output_delegator"
 require "logstash/filter_delegator"

logstash-core/lib/logstash/plugin.rb

@@ -103,9 +103,9 @@ class LogStash::Plugin
     # we will use the NullMetric in this case.
     @metric_plugin ||= if @enable_metric
                          # Fallback when testing plugin and no metric collector are correctly configured.
-                         @metric.nil? ? LogStash::Instrument::NullMetric.new : @metric
+                         @metric.nil? ? LogStash::Instrument::NamespacedNullMetric.new : @metric
                        else
-                         LogStash::Instrument::NullMetric.new
+                         LogStash::Instrument::NamespacedNullMetric.new(@metric, :null)
                        end
   end
   # return the configured name of this plugin

logstash-core/spec/logstash/filter_delegator_spec.rb

@@ -10,7 +10,8 @@ describe LogStash::FilterDelegator do
   let(:config) do
     { "host" => "127.0.0.1", "id" => filter_id }
   end
-  let(:metric) { LogStash::Instrument::NullMetric.new }
+  let(:collector) { [] }
+  let(:metric) { LogStash::Instrument::NamespacedNullMetric.new(collector, :null) }
   let(:events) { [LogStash::Event.new, LogStash::Event.new] }
 
   before :each do

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

@@ -0,0 +1,33 @@
+# encoding: utf-8
+require "logstash/instrument/namespaced_null_metric"
+require "logstash/instrument/null_metric"
+require_relative "../../support/matchers"
+require "spec_helper"
+
+describe LogStash::Instrument::NamespacedNullMetric do
+  let(:namespace) { :root }
+  let(:collector) { [] }
+  let(:metric) { LogStash::Instrument::NullMetric.new(collector) }
+
+  subject { described_class.new(metric, namespace) }
+
+  it "defines the same interface as `Metric`" do
+    expect(described_class).to implement_interface_of(LogStash::Instrument::NamespacedMetric)
+  end
+
+  it "returns a TimedException when we call without a block" do
+    expect(subject.time(:duration_ms)).to be(LogStash::Instrument::NullMetric::NullTimedExecution)
+  end
+
+  it "returns the value of the block" do
+    expect(subject.time(:duration_ms) { "hello" }).to eq("hello")
+  end
+
+  it "its doesnt change the original `namespace` when creating a subnamespace" do
+    new_namespace = subject.namespace(:wally)
+
+    expect(subject.namespace_name).to eq([namespace])
+    expect(new_namespace.namespace_name).to eq([:root, :wally])
+  end
+
+end

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

@@ -2,18 +2,22 @@
 require "logstash/instrument/null_metric"
 require "logstash/instrument/namespaced_metric"
 require_relative "../../support/shared_examples"
+require_relative "../../support/matchers"
+require "spec_helper"
 
 describe LogStash::Instrument::NullMetric do
-  # This is defined in the `namespaced_metric_spec`
+
-  include_examples "metrics commons operations"
+  let(:key) { "test" }
+  let(:collector) { [] }
+  subject { LogStash::Instrument::NullMetric.new(collector) }
 
   it "defines the same interface as `Metric`" do
-    expect(described_class).to implement_interface_of(LogStash::Instrument::NamespacedMetric)
+    expect(described_class).to implement_interface_of(LogStash::Instrument::Metric)
   end
 
   describe "#namespace" do
-    it "return a NullMetric" do
+    it "return a NamespacedNullMetric" do
-      expect(subject.namespace(key)).to be_kind_of LogStash::Instrument::NullMetric
+      expect(subject.namespace(key)).to be_kind_of LogStash::Instrument::NamespacedNullMetric
     end
   end
 end

logstash-core/spec/logstash/output_delegator_spec.rb

@@ -6,7 +6,8 @@ describe LogStash::OutputDelegator do
   let(:logger) { double("logger") }
   let(:events) { 7.times.map { LogStash::Event.new }}
   let(:plugin_args) { {"id" => "foo", "arg1" => "val1"} }
-  let(:metric) { LogStash::Instrument::NullMetric.new }
+  let(:collector) { [] }
+  let(:metric) { LogStash::Instrument::NamespacedNullMetric.new(collector, :null) }
 
   subject { described_class.new(logger, out_klass, metric, ::LogStash::OutputDelegatorStrategyRegistry.instance, plugin_args) }
 

logstash-core/spec/logstash/plugin_spec.rb

@@ -296,7 +296,7 @@ describe LogStash::Plugin do
           end
 
           it "should use a `NullMetric`" do
-            expect(subject.metric).to be_kind_of(LogStash::Instrument::NullMetric)
+            expect(subject.metric).to be_kind_of(LogStash::Instrument::NamespacedNullMetric)
           end
         end
 
@@ -308,7 +308,7 @@ describe LogStash::Plugin do
           end
 
           it "should use a `NullMetric`" do
-            expect(subject.metric).to be_kind_of(LogStash::Instrument::NullMetric)
+            expect(subject.metric).to be_kind_of(LogStash::Instrument::NamespacedNullMetric)
           end
         end
       end
@@ -338,7 +338,7 @@ describe LogStash::Plugin do
           end
 
           it "should use a `NullMetric`" do
-            expect(subject.metric).to be_kind_of(LogStash::Instrument::NullMetric)
+            expect(subject.metric).to be_kind_of(LogStash::Instrument::NamespacedNullMetric)
           end
         end
       end

logstash-core/spec/logstash/util/wrapped_synchronous_queue_spec.rb

@@ -88,8 +88,8 @@ describe LogStash::Util::WrappedSynchronousQueue do
 
       context "when writing to the queue" do
         before :each do
-          read_client.set_events_metric(LogStash::Instrument::NullMetric.new)
+          read_client.set_events_metric(LogStash::Instrument::NamespacedNullMetric.new([], :null))
-          read_client.set_pipeline_metric(LogStash::Instrument::NullMetric.new)
+          read_client.set_pipeline_metric(LogStash::Instrument::NamespacedNullMetric.new([], :null))
         end
 
         it "appends batches to the queue" do