diff --git a/logstash-core/lib/logstash/agent.rb b/logstash-core/lib/logstash/agent.rb index 86f605763..e6114cf1d 100644 --- a/logstash-core/lib/logstash/agent.rb +++ b/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 diff --git a/logstash-core/lib/logstash/environment.rb b/logstash-core/lib/logstash/environment.rb index ddef42918..a918a4041 100644 --- a/logstash-core/lib/logstash/environment.rb +++ b/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), diff --git a/logstash-core/lib/logstash/instrument/namespaced_null_metric.rb b/logstash-core/lib/logstash/instrument/namespaced_null_metric.rb new file mode 100644 index 000000000..c4e8e762c --- /dev/null +++ b/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 diff --git a/logstash-core/lib/logstash/instrument/null_metric.rb b/logstash-core/lib/logstash/instrument/null_metric.rb index aec7d8116..afdb345ef 100644 --- a/logstash-core/lib/logstash/instrument/null_metric.rb +++ b/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 - # for specific plugin to replace the `NamespacedMetric` class with this one - # which doesn't produce any metric to the collector. - class NullMetric - attr_reader :namespace_name, :collector + # This class is used in the context when we disable the metric collection + # for specific plugin to replace the `NamespacedMetric` class with this one + # which doesn't produce any metric to the collector. + class NullMetric + attr_reader :namespace_name, :collector - def increment(key, value = 1) - Metric.validate_key!(key) - end + def initialize(collector = nil) + @collector = collector + end - def decrement(key, value = 1) - Metric.validate_key!(key) - end + def increment(namespace, key, value = 1) + Metric.validate_key!(key) + end - def gauge(key, value) - Metric.validate_key!(key) - end + def decrement(namespace, key, value = 1) + Metric.validate_key!(key) + end - def report_time(key, duration) - Metric.validate_key!(key) - end + def gauge(namespace, key, value) + Metric.validate_key!(key) + end - # We have to manually redefine this method since it can return an - # object this object also has to be implemented as a NullObject - def time(key) - Metric.validate_key!(key) - if block_given? - yield - else - NullTimedExecution - end - end + def report_time(namespace, key, duration) + Metric.validate_key!(key) + end - def namespace(key) - self.class.new - end + # We have to manually redefine this method since it can return an + # object this object also has to be implemented as a NullObject + def time(namespace, key) + Metric.validate_key!(key) + if block_given? + yield + else + NullTimedExecution + end + end - private - # Null implementation of the internal timer class - # - # @see LogStash::Instrument::TimedExecution` - class NullTimedExecution - def self.stop - end - end - end + def namespace(name) + raise MetricNoNamespaceProvided if name.nil? || name.empty? + NamespacedNullMetric.new(self, name) + end + + def self.validate_key!(key) + raise MetricNoKeyProvided if key.nil? || key.empty? + end + + private + # Null implementation of the internal timer class + # + # @see LogStash::Instrument::TimedExecution` + class NullTimedExecution + def self.stop + end + end + end end; end diff --git a/logstash-core/lib/logstash/pipeline.rb b/logstash-core/lib/logstash/pipeline.rb index ad1297cba..6b4c3b966 100644 --- a/logstash-core/lib/logstash/pipeline.rb +++ b/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" diff --git a/logstash-core/lib/logstash/plugin.rb b/logstash-core/lib/logstash/plugin.rb index 80f0bd04e..c3ebd1c9a 100644 --- a/logstash-core/lib/logstash/plugin.rb +++ b/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 diff --git a/logstash-core/spec/logstash/filter_delegator_spec.rb b/logstash-core/spec/logstash/filter_delegator_spec.rb index 98586a173..951c72f69 100644 --- a/logstash-core/spec/logstash/filter_delegator_spec.rb +++ b/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 diff --git a/logstash-core/spec/logstash/instrument/namespaced_null_metric_spec.rb b/logstash-core/spec/logstash/instrument/namespaced_null_metric_spec.rb new file mode 100644 index 000000000..fdd831dfb --- /dev/null +++ b/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 diff --git a/logstash-core/spec/logstash/instrument/null_metric_spec.rb b/logstash-core/spec/logstash/instrument/null_metric_spec.rb index 8f4aa4b1f..27a861eae 100644 --- a/logstash-core/spec/logstash/instrument/null_metric_spec.rb +++ b/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 - expect(subject.namespace(key)).to be_kind_of LogStash::Instrument::NullMetric + it "return a NamespacedNullMetric" do + expect(subject.namespace(key)).to be_kind_of LogStash::Instrument::NamespacedNullMetric end end end diff --git a/logstash-core/spec/logstash/output_delegator_spec.rb b/logstash-core/spec/logstash/output_delegator_spec.rb index 9c3808a02..56dbd5168 100644 --- a/logstash-core/spec/logstash/output_delegator_spec.rb +++ b/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) } diff --git a/logstash-core/spec/logstash/plugin_spec.rb b/logstash-core/spec/logstash/plugin_spec.rb index 9747d4638..6ef3398b8 100644 --- a/logstash-core/spec/logstash/plugin_spec.rb +++ b/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 diff --git a/logstash-core/spec/logstash/util/wrapped_synchronous_queue_spec.rb b/logstash-core/spec/logstash/util/wrapped_synchronous_queue_spec.rb index 4290a6228..d7b403ed0 100644 --- a/logstash-core/spec/logstash/util/wrapped_synchronous_queue_spec.rb +++ b/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_pipeline_metric(LogStash::Instrument::NullMetric.new) + read_client.set_events_metric(LogStash::Instrument::NamespacedNullMetric.new([], :null)) + read_client.set_pipeline_metric(LogStash::Instrument::NamespacedNullMetric.new([], :null)) end it "appends batches to the queue" do