Skip to content

Commit

Permalink
Add a Plugin#id that return an ID for a plugin
Browse files Browse the repository at this point in the history
This method return an ID for the plugins and be configured by the users
in the configuration like this:

```
elasticsearch {
  id => "ABC"
  ...
}
```

This information will be used when collecting metrics for a specific
plugin. Allowing the user to change it allow to stick between restart.

Fixes #3892

Fixes #4525
  • Loading branch information
ph committed Feb 1, 2016
1 parent 2766d5b commit 1a780ab
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 23 deletions.
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/filter_delegator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def initialize(logger, klass, metric, *args)
@filter = klass.new(options)

# Scope the metrics to the plugin
@metric = metric.namespace(@filter.identifier_name)
@metric = metric.namespace(@filter.id.to_sym)
@filter.metric = @metric

define_flush_method if @filter.respond_to?(:flush)
Expand Down
6 changes: 3 additions & 3 deletions logstash-core/lib/logstash/output_delegator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ def initialize(logger, klass, default_worker_count, metric, *args)
@threadsafe = klass.threadsafe?
@config = args.reduce({}, :merge)
@klass = klass

# Create an instance of the input so we can fetch the identifier
output = @klass.new(*args)

# Scope the metrics to the plugin
@metric = metric.namespace(output.identifier_name)
@metric = metric.namespace(output.id.to_sym)

# We define this as an array regardless of threadsafety
# to make reporting simpler, even though a threadsafe plugin will just have
Expand Down
39 changes: 23 additions & 16 deletions logstash-core/lib/logstash/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
require "logstash/instrument/null_metric"
require "cabin"
require "concurrent"
require "digest/md5"
require "securerandom"

class LogStash::Plugin

Expand All @@ -21,13 +21,19 @@ class LogStash::Plugin
# for a specific plugin.
config :enable_metric, :validate => :boolean, :default => true

# Under which name you want to collect metric for this plugin?
# This will allow you to compare the performance of the configuration change, this
# name need to be unique per plugin configuration.
# Add a unique `ID` to the plugin instance, this `ID` is used for tracking
# information for a specific configuration of the plugin.
#
# ```
# output {
# stdout {
# id => "ABC"
# }
# }
# ```
#
# If you don't explicitely set this variable Logstash will generate a unique name.
# This name will be valid until the configuration change.
config :metric_identifier, :validate => :string, :default => ""
config :id, :validate => :string, :default => ""

public
def hash
Expand All @@ -46,6 +52,16 @@ def initialize(params=nil)
@logger = Cabin::Channel.get(LogStash)
end

# Return a uniq ID for this plugin configuration, by default
# we will generate a UUID
#
# If the user defines a `id => 'ABC'` in the configuration we will return
#
# @return [String] A plugin ID
def id
(@params["id"].nil? || @params["id"].empty?) ? SecureRandom.uuid : @params["id"]
end

# close is called during shutdown, after the plugin worker
# main task terminates
public
Expand Down Expand Up @@ -83,21 +99,13 @@ def debug_info
end

def metric=(new_metric)
@metric = new_metric.namespace(identifier_name)
@metric = new_metric.namespace(@id)
end

def metric
@metric_plugin ||= enable_metric ? @metric : LogStash::Instrument::NullMetric.new
end

def identifier_name
@identifier_name ||= (@metric_identifier.nil? || @metric_identifier.empty?) ? "#{self.class.config_name}-#{params_hash_code}".to_sym : @identifier.to_sym
end

def params_hash_code
Digest::MD5.hexdigest(params.to_s)
end

# Look up a plugin by type and name.
public
def self.lookup(type, name)
Expand All @@ -120,7 +128,6 @@ def self.lookup(type, name)
end

private

# lookup a plugin by type and name in the existing LogStash module namespace
# ex.: namespace_lookup("filter", "grok") looks for LogStash::Filters::Grok
# @param type [String] plugin type, "input", "ouput", "filter"
Expand Down
6 changes: 3 additions & 3 deletions logstash-core/spec/logstash/output_delegator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
allow(out_inst).to receive(:register)
allow(out_inst).to receive(:multi_receive)
allow(out_inst).to receive(:metric=).with(any_args)
allow(out_inst).to receive(:identifier_name).and_return("a-simple-plugin")
allow(out_inst).to receive(:id).and_return("a-simple-plugin")
allow(logger).to receive(:debug).with(any_args)
end

Expand Down Expand Up @@ -60,7 +60,7 @@
allow(out_klass).to receive(:threadsafe?).and_return(false)
allow(out_klass).to receive(:workers_not_supported?).and_return(false)
allow(out_inst).to receive(:metric=).with(any_args)
allow(out_inst).to receive(:identifier_name).and_return("a-simple-plugin")
allow(out_inst).to receive(:id).and_return("a-simple-plugin")
end

it "should instantiate multiple workers" do
Expand All @@ -77,7 +77,7 @@
before do
allow(out_klass).to receive(:threadsafe?).and_return(true)
allow(out_inst).to receive(:metric=).with(any_args)
allow(out_inst).to receive(:identifier_name).and_return("a-simple-plugin")
allow(out_inst).to receive(:id).and_return("a-simple-plugin")
allow(out_klass).to receive(:workers_not_supported?).and_return(false)
end

Expand Down
56 changes: 56 additions & 0 deletions logstash-core/spec/logstash/plugin_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# encoding: utf-8
require "spec_helper"
require "logstash/plugin"
require "logstash/outputs/base"
require "logstash/codecs/base"
require "logstash/inputs/base"
require "logstash/filters/base"

describe LogStash::Plugin do
it "should fail lookup on inexisting type" do
Expand Down Expand Up @@ -149,4 +153,56 @@ class LogStash::Filters::Stromae < LogStash::Filters::Base
end
end
end

describe "#id" do
plugin_types = [
LogStash::Filters::Base,
LogStash::Codecs::Base,
LogStash::Outputs::Base,
LogStash::Inputs::Base
]

plugin_types.each do |plugin_type|
let(:plugin) do
Class.new(plugin_type) do
config_name "simple_plugin"

config :host, :validate => :string
config :export, :validte => :boolean

def register; end
end
end

let(:config) do
{
"host" => "127.0.0.1",
"export" => true
}
end

subject { plugin.new(config) }

context "plugin type is #{plugin_type}" do
context "when there is not ID configured for the output" do
it "it uses a UUID to identify this plugins" do
expect(subject.id).not_to eq(nil)
end

it "will be different between instance of plugins" do
expect(subject.id).not_to eq(plugin.new(config).id)
end
end

context "When a user provide an ID for the plugin" do
let(:id) { "ABC" }
let(:config) { super.merge("id" => id) }

it "uses the user provided ID" do
expect(subject.id).to eq(id)
end
end
end
end
end
end

0 comments on commit 1a780ab

Please sign in to comment.