Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added line and columns to plugin name #11155

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0896771
Added line and columns to plugin name
andsel Sep 20, 2019
8cd3fcc
Introduced new Plugin's node attribute to separate name from name wit…
andsel Sep 30, 2019
3299560
Exposed code reference for plugins in HTTP monitoring API (through me…
andsel Oct 3, 2019
1df10f6
Removed the code reference from plugin (keeping in metrics)
andsel Oct 8, 2019
3d60817
Fixed bad param passing into filters
andsel Oct 8, 2019
6502920
Remapping global config line to source file and source line
andsel Oct 10, 2019
aeae43a
Remapping global config line to source file and source line (Java pip…
andsel Oct 11, 2019
ed59016
Minor, removed debug string
andsel Oct 11, 2019
9fae7da
Minor, removed unusefull method
andsel Oct 11, 2019
6c3a90c
Minor, code style fixes
andsel Oct 12, 2019
edf4360
Moved parameters that broken API to be assigned with another method
andsel Oct 15, 2019
1815412
Fixed failing tests
andsel Oct 15, 2019
89fc967
fixed code-ref for Ruby code generation and fixed all failing tests
andsel Oct 16, 2019
dafe861
Avoid double line computation and used caching of source segments to …
andsel Oct 16, 2019
a9ded3d
[WIP] fixes for codecs with nested parameters
andsel Oct 17, 2019
6b7453d
Removed test that verified behaviour never used
andsel Oct 21, 2019
90342f9
Added config-ref in metrics also for codec in Java pipeline
andsel Oct 21, 2019
6d8a286
Added integration test to validate config references in HTTP monitori…
andsel Oct 21, 2019
2e8cba9
Minor, renamed property
andsel Oct 21, 2019
1d815b3
Fixed badly changed metric name
andsel Oct 22, 2019
0ba9e94
Method parent_config_reference= is present only in Codec's Delegator
andsel Oct 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion logstash-core/lib/logstash/codecs/delegator.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
module LogStash::Codecs
class Delegator < SimpleDelegator
def initialize(obj)
def initialize(obj, parent_plugin_name)
super(obj)
@parent_plugin_name = parent_plugin_name
@parent_config_reference = nil
@encode_metric = LogStash::Instrument::NamespacedNullMetric.new
@decode_metric = LogStash::Instrument::NamespacedNullMetric.new
end
Expand All @@ -14,6 +16,8 @@ def metric=(metric)
__getobj__.metric = metric

__getobj__.metric.gauge(:name, __getobj__.class.config_name)
__getobj__.metric.gauge(:"parent-config-ref", @parent_config_reference)
__getobj__.metric.gauge(:"parent-name", @parent_plugin_name)

@encode_metric = __getobj__.metric.namespace(:encode)
@encode_metric.counter(:writes_in)
Expand All @@ -25,6 +29,10 @@ def metric=(metric)
@decode_metric.report_time(:duration_in_millis, 0)
end

def parent_config_reference=(parent_config_reference)
@parent_config_reference = parent_config_reference
end

def encode(event)
@encode_metric.increment(:writes_in)
@encode_metric.time(:duration_in_millis) do
Expand Down
23 changes: 18 additions & 5 deletions logstash-core/lib/logstash/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
module LogStash; class Compiler
include ::LogStash::Util::Loggable

def self.compile_sources(sources_with_metadata, support_escapes)
def self.compile_sources(pipeline_config, support_escapes)
sources_with_metadata = [org.logstash.common.SourceWithMetadata.new("str", "pipeline", 0, 0, pipeline_config.config_string)]
graph_sections = sources_with_metadata.map do |swm|
self.compile_graph(swm, support_escapes)
self.compile_graph(pipeline_config, swm, support_escapes)
end

input_graph = Graph.combine(*graph_sections.map {|s| s[:input] }).graph
Expand All @@ -29,7 +30,7 @@ def self.compile_sources(sources_with_metadata, support_escapes)
PipelineIR.new(input_graph, filter_graph, output_graph, original_source)
end

def self.compile_imperative(source_with_metadata, support_escapes)
def self.compile_imperative(pipeline_config, source_with_metadata, support_escapes)
if !source_with_metadata.is_a?(org.logstash.common.SourceWithMetadata)
raise ArgumentError, "Expected 'org.logstash.common.SourceWithMetadata', got #{source_with_metadata.class}"
end
Expand All @@ -41,11 +42,23 @@ def self.compile_imperative(source_with_metadata, support_escapes)
raise ConfigurationError, grammar.failure_reason
end

plugins = config.recursive_select(LogStashCompilerLSCLGrammar::LogStash::Compiler::LSCL::AST::Plugin)
added_source_line_column(pipeline_config, plugins)

config.process_escape_sequences = support_escapes
config.compile(source_with_metadata)
end

def self.compile_graph(source_with_metadata, support_escapes)
Hash[compile_imperative(source_with_metadata, support_escapes).map {|section,icompiled| [section, icompiled.toGraph]}]
def self.compile_graph(pipeline_config, source_with_metadata, support_escapes)
Hash[compile_imperative(pipeline_config, source_with_metadata, support_escapes).map {|section,icompiled| [section, icompiled.toGraph]}]
end

def self.added_source_line_column(pipeline_config, plugins)
plugins.each do |plugin|
remapped_line = pipeline_config.lookup_source_and_line(plugin.line_and_column[0])
plugin.source_file = remapped_line[0]
plugin.source_line = remapped_line[1]
end
end

end; end
16 changes: 14 additions & 2 deletions logstash-core/lib/logstash/compiler/lscl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,11 @@ def expr

class Plugins < Node; end
class Plugin < Node

attr_accessor :source_file, :source_line

def expr
jdsl.iPlugin(source_meta, plugin_type_enum, self.plugin_name, self.expr_attributes)
jdsl.iPlugin(source_meta, plugin_type_enum, self.plugin_name, source_file, source_line, self.expr_attributes)
end

def plugin_type_enum
Expand All @@ -95,7 +98,11 @@ def plugin_name

def expr_attributes
# Turn attributes into a hash map
self.attributes.recursive_select(Attribute).map(&:expr).map {|k,v|
self.attributes.recursive_select(Attribute).map {|attribute|
attribute.source_file = source_file
attribute.source_line = source_line
attribute
}.map(&:expr).map {|k,v|
if v.java_kind_of?(Java::OrgLogstashConfigIrExpression::ValueExpression)
[k, v.get]
else
Expand Down Expand Up @@ -130,7 +137,12 @@ def expr
end

class Attribute < Node
attr_accessor :source_file, :source_line
def expr
if value.is_a?(Plugin)
value.source_file = source_file
value.source_line = source_line
end
[name.text_value, value.expr]
end
end
Expand Down
21 changes: 16 additions & 5 deletions logstash-core/lib/logstash/config/config_ast.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ def generate_variables

class Plugins < Node; end
class Plugin < Node
attr_accessor :source_file, :source_line

def plugin_type
if recursive_select_parent(Plugin).any?
return "codec"
Expand All @@ -231,14 +233,18 @@ def variable_name

def compile_initializer
# If any parent is a Plugin, this must be a codec.

if attributes.elements.nil?
return "plugin(#{plugin_type.inspect}, #{plugin_name.inspect}, #{source_meta.line}, #{source_meta.column})" << (plugin_type == "codec" ? "" : "\n")
return "plugin(#{plugin_type.inspect}, #{plugin_name.inspect}, #{source_meta.line}, #{source_meta.column}, #{source_file.inspect}, #{source_line.inspect})" << (plugin_type == "codec" ? "" : "\n")
else
settings = attributes.recursive_select(Attribute).collect(&:compile).reject(&:empty?)
attrs = attributes.recursive_select(Attribute)
attrs.each do |attribute|
attribute.source_file = source_file
attribute.source_line = source_line
end
settings = attrs.collect(&:compile).reject(&:empty?)

attributes_code = "LogStash::Util.hash_merge_many(#{settings.map { |c| "{ #{c} }" }.join(", ")})"
return "plugin(#{plugin_type.inspect}, #{plugin_name.inspect}, #{source_meta.line}, #{source_meta.column}, #{attributes_code})" << (plugin_type == "codec" ? "" : "\n")
return "plugin(#{plugin_type.inspect}, #{plugin_name.inspect}, #{source_meta.line}, #{source_meta.column}, #{source_file.inspect}, #{source_line.inspect}, #{attributes_code})" << (plugin_type == "codec" ? "" : "\n")
end
end

Expand All @@ -255,7 +261,7 @@ def compile
when "codec"
settings = attributes.recursive_select(Attribute).collect(&:compile).reject(&:empty?)
attributes_code = "LogStash::Util.hash_merge_many(#{settings.map { |c| "{ #{c} }" }.join(", ")})"
return "plugin(#{plugin_type.inspect}, #{plugin_name.inspect}, #{source_meta.line}, #{source_meta.column}, #{attributes_code})"
return "plugin(#{plugin_type.inspect}, #{plugin_name.inspect}, #{source_meta.line}, #{source_meta.column}, #{source_file.inspect}, #{source_line.inspect}, #{attributes_code})"
end
end

Expand Down Expand Up @@ -309,7 +315,12 @@ def compile
end
end
class Attribute < Node
attr_accessor :source_file, :source_line
def compile
if value.is_a?(Plugin)
value.source_file = source_file
value.source_line = source_line
end
return %Q(#{name.compile} => #{value.compile})
end
end
Expand Down
4 changes: 3 additions & 1 deletion logstash-core/lib/logstash/config/mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,9 @@ def validate_value(value, validator)
case validator
when :codec
if value.first.is_a?(String)
value = LogStash::Codecs::Delegator.new LogStash::Plugin.lookup("codec", value.first).new
parent_plugin_name = self.config_name
codec = LogStash::Plugin.lookup("codec", value.first).new
value = LogStash::Codecs::Delegator.new(codec, parent_plugin_name)
return true, value
else
value = value.first
Expand Down
25 changes: 25 additions & 0 deletions logstash-core/lib/logstash/config/pipeline_config.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# encoding: utf-8
require "digest"

java_import org.logstash.config.ir.ConfigSourceSegment

module LogStash module Config

class PipelineConfig
include LogStash::Util::Loggable

Expand Down Expand Up @@ -44,5 +47,27 @@ def display_debug_information
logger.debug("Merged config")
logger.debug("\n\n#{config_string}")
end

def lookup_source_and_line(merged_line_number)
res = source_map.find { |source_segment| source_segment.contains(merged_line_number) }
if res == nil
raise IndexError
end
[res.getSource(), res.rebase(merged_line_number)]
end

private
def source_map
@source_map ||= begin
offset = 0
source_map = []
config_parts.each do |config_part|
source_segment = ConfigSourceSegment.new config_part.id, offset, config_part.getLinesCount()
source_map << source_segment
offset += source_segment.getLength()
end
source_map.freeze
end
end
end
end end
9 changes: 9 additions & 0 deletions logstash-core/lib/logstash/inputs/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ def clone
cloned
end

#used only in tests
def codec_attribute
@codec
end

def codec
params["codec"]
end

def metric=(metric)
super
# Hack to create a new metric namespace using 'plugins' as the root
Expand Down
14 changes: 12 additions & 2 deletions logstash-core/lib/logstash/pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ def initialize(pipeline_config, namespaced_metric = nil, agent = nil)
parsed_config = grammar.parse(config_str)
raise(ConfigurationError, grammar.failure_reason) if parsed_config.nil?

plugins = parsed_config.recursive_select(LogStash::Config::AST::Plugin)
added_source_line_column(pipeline_config, plugins)

parsed_config.process_escape_sequences = settings.get_value("config.support_escapes")
config_code = parsed_config.compile

Expand Down Expand Up @@ -68,9 +71,16 @@ def non_reloadable_plugins

private

def added_source_line_column(pipeline_config, plugins)
plugins.each do |plugin|
remapped_line = pipeline_config.lookup_source_and_line(plugin.line_and_column[0])
plugin.source_file = remapped_line[0]
plugin.source_line = remapped_line[1]
end
end

def plugin(plugin_type, name, line, column, *args)
@plugin_factory.plugin(plugin_type, name, line, column, *args)
def plugin(plugin_type, name, line, column, source_file, source_line, *args)
@plugin_factory.plugin(plugin_type, name, line, column, source_file, source_line, *args)
end

def default_logging_keys(other_keys = {})
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/spec/logstash/codecs/delegator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def decode(e)
let(:codec) { LogStash::Codecs::MockCodec.new }

subject do
delegator = described_class.new(codec)
delegator = described_class.new(codec, "MockCodec")
delegator.metric = metric.namespace([:stats, :pipelines, :main, :plugins, :codecs, :my_id])
delegator
end
Expand Down
21 changes: 5 additions & 16 deletions logstash-core/spec/logstash/compiler/compiler_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require "spec_helper"
require "logstash/compiler"
require "support/helpers"
require "logstash/config/pipeline_config"
java_import Java::OrgLogstashConfigIr::DSL

describe LogStash::Compiler do
Expand Down Expand Up @@ -50,8 +51,9 @@ def rand_meta
org.logstash.common.SourceWithMetadata.new("#{source_protocol}_#{idx}", "#{source_id}_#{idx}", 0, 0, source)
end
end
let(:pipeline_config) {LogStash::Config::PipelineConfig.new LogStash::Config::Source::Local, "test_pipeline", sources_with_metadata, LogStash::SETTINGS}

subject(:pipeline) { described_class.compile_sources(sources_with_metadata, false) }
subject(:pipeline) { described_class.compile_sources(pipeline_config, false) }

it "should generate a hash" do
expect(pipeline.unique_hash).to be_a(String)
Expand All @@ -64,20 +66,6 @@ def rand_meta
it "should provide the original source" do
expect(pipeline.original_source).to eq(sources.join("\n"))
end

describe "applying protocol and id metadata" do
it "should apply the correct source metadata to all components" do
# TODO: seems to be a jruby regression we cannot currently call each on a stream
pipeline.get_plugin_vertices.each do |pv|
name_idx = pv.plugin_definition.name.split("_").last
source_protocol_idx = pv.source_with_metadata.protocol.split("_").last
source_id_idx = pv.source_with_metadata.id.split("_").last

expect(name_idx).to eq(source_protocol_idx)
expect(name_idx).to eq(source_id_idx)
end
end
end
end

describe "complex configs" do
Expand All @@ -104,7 +92,8 @@ def rand_meta
describe "compiling imperative" do
let(:source_id) { "fake_sourcefile" }
let(:source_with_metadata) { org.logstash.common.SourceWithMetadata.new(source_protocol, source_id, 0, 0, source) }
subject(:compiled) { described_class.compile_imperative(source_with_metadata, settings.get_value("config.support_escapes")) }
let(:pipeline_config) {LogStash::Config::PipelineConfig.new LogStash::Config::Source::Local, "test_pipeline", [source_with_metadata], LogStash::SETTINGS}
subject(:compiled) { described_class.compile_imperative(pipeline_config, source_with_metadata, settings.get_value("config.support_escapes")) }

context "when config.support_escapes" do
let(:parser) { LogStashCompilerLSCLGrammarParser.new }
Expand Down
54 changes: 54 additions & 0 deletions logstash-core/spec/logstash/config/pipeline_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,58 @@
end
end
end

describe "source and line remapping" do
context "when pipeline is constructed from single file single line" do
let (:pipeline_conf_string) { 'input { generator1 }' }
subject { described_class.new(source, pipeline_id, [org.logstash.common.SourceWithMetadata.new("file", "/tmp/1", 0, 0, pipeline_conf_string)], settings) }
it "return the same line of the queried" do
expect(subject.lookup_source_and_line(1)[1]).to eq(1)
end
end

context "when pipeline is constructed from single file" do
let (:pipeline_conf_string) { 'input {
generator1
}' }
subject { described_class.new(source, pipeline_id, [org.logstash.common.SourceWithMetadata.new("file", "/tmp/1", 0, 0, pipeline_conf_string)], settings) }

it "return the same line of the queried" do
expect(subject.lookup_source_and_line(1)[1]).to eq(1)
expect(subject.lookup_source_and_line(2)[1]).to eq(2)
end

it "throw exception if line is out of bound" do
expect { subject.lookup_source_and_line(100) }.to raise_exception(IndexError)
end
end

context "when pipeline is constructed from multiple files" do
let (:pipeline_conf_string_part1) { 'input {
generator1
}' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: test case where first file has trailing newline to ensure it doesn't offset the line numbers from subsequent files

let (:pipeline_conf_string_part2) { 'output {
stdout
}' }
let(:merged_config_parts) do
[
org.logstash.common.SourceWithMetadata.new("file", "/tmp/input", 0, 0, pipeline_conf_string_part1),
org.logstash.common.SourceWithMetadata.new("file", "/tmp/output", 0, 0, pipeline_conf_string_part2)
]
end
subject { described_class.new(source, pipeline_id, merged_config_parts, settings) }

it "return the line of first segment" do
expect(subject.lookup_source_and_line(2)).to eq(["/tmp/input", 2])
end

it "return the line of second segment" do
expect(subject.lookup_source_and_line(4)).to eq(["/tmp/output", 1])
end

it "throw exception if line is out of bound" do
expect { subject.lookup_source_and_line(100) }.to raise_exception(IndexError)
end
end
end
end
2 changes: 1 addition & 1 deletion logstash-core/spec/logstash/filter_delegator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

let(:filter_id) { "my-filter" }
let(:config) do
{ "host" => "127.0.0.1", "id" => filter_id }
{ "host" => "127.0.0.1", "id" => filter_id, "config-ref" => "S: <filter_delegator_spec.rb> L: 14" }
end
let(:collector) {LogStash::Instrument::Collector.new}
let(:metric) { LogStash::Instrument::Metric.new(collector).namespace(:null) }
Expand Down
Loading