Skip to content

Commit

Permalink
Replace def #{type}_func with define_singleton_method
Browse files Browse the repository at this point in the history
When you run multiples pipeline and the config code get evaluated, on
every evaluation the class cached is clear, everytime you were calling a
`func` method you had the latest evaluated code. The `filter_func` and
the `output_func` need to be unique for every instance of the pipeline,
this PR replace the `def` with a `define_single_method` call ensuring
the uniqueness of the code on every instance.

This PR is based on #4254

Fixes #4298
  • Loading branch information
ph committed Jan 6, 2016
1 parent 5e76d97 commit 9d876fa
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
7 changes: 5 additions & 2 deletions logstash-core/lib/logstash/config/config_ast.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ def compile
["filter", "output"].each do |type|
# defines @filter_func and @output_func

definitions << "def #{type}_func(event)"
# This need to be defined as a singleton method
# so each instance of the pipeline has his own implementation
# of the output/filter function
definitions << "define_singleton_method :#{type}_func do |event|"
definitions << " targeted_outputs = []" if type == "output"
definitions << " events = [event]" if type == "filter"
definitions << " @logger.debug? && @logger.debug(\"#{type} received\", :event => event.to_hash)"
Expand Down Expand Up @@ -544,4 +547,4 @@ def _inspect(indent="")
""
)
end
end
end
31 changes: 30 additions & 1 deletion logstash-core/spec/logstash/pipeline_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def initialize(params={})

def register
end

def receive(event)
@events << event
end
Expand Down Expand Up @@ -382,4 +382,33 @@ class TestPipeline < LogStash::Pipeline
pipeline.shutdown
end
end

context "Multiple pipelines" do
before do
allow(LogStash::Plugin).to receive(:lookup).with("input", "generator").and_return(LogStash::Inputs::Generator)
allow(LogStash::Plugin).to receive(:lookup).with("codec", "plain").and_return(DummyCodec)
allow(LogStash::Plugin).to receive(:lookup).with("filter", "dummyfilter").and_return(DummyFilter)
allow(LogStash::Plugin).to receive(:lookup).with("output", "dummyoutput").and_return(DummyOutput)
end

let(:pipeline1) { LogStash::Pipeline.new("input { generator {} } filter { dummyfilter {} } output { dummyoutput {}}") }
let(:pipeline2) { LogStash::Pipeline.new("input { generator {} } filter { dummyfilter {} } output { dummyoutput {}}") }

it "should handle evaluating different config" do
# When the functions are compiled from the AST it will generate instance
# variables that are unique to the actual config, the intance are pointing
# to conditionals/plugins.
#
# Before the `defined_singleton_method`, the definition of the method was
# not unique per class, but the `instance variables` were unique per class.
#
# So the methods were trying to access instance variables that did not exist
# in the current instance and was returning an array containing nil values for
# the match.
expect(pipeline1.output_func(LogStash::Event.new)).not_to include(nil)
expect(pipeline1.filter_func(LogStash::Event.new)).not_to include(nil)
expect(pipeline2.output_func(LogStash::Event.new)).not_to include(nil)
expect(pipeline1.filter_func(LogStash::Event.new)).not_to include(nil)
end
end
end

0 comments on commit 9d876fa

Please sign in to comment.