Added line and columns to plugin name

[andsel]

Note
this PR is superseeded by #11288 for the Java pipeline part, for the Ruby pipeline we have to check if it worthwhile

[Feature Discussion]

We want to give a more useful name/id to the plugin so that could be easily mapped to which point in the pipeline it refers to. I think the best option would be to have a line number referring the position into the pipeline config file, like input-L5. What do you think about it?

solves #11154

[yaauie]

IIRC, the line_and_column isn't quite as helpful as we would expect it to be, because it represents the position of the element after concatenating all source configuration files together, and it is an especially common practice in large pipelines to split up the configuration into many source files.

[andsel]

@yaauie thanks for the hint. We could do this:

• in the management HTTP API for pipelines (localhost:9600/_node/stats/pipelines?pretty) expose the triple source file, line, column as code reference
• in the logs keep the plugin.id
• provide an HTTP API (eg: localhost:9600/_node/pipelines/main/<plugin.id>) that return the plugin information regarding its definition

[yaauie]

IIRC, the source files for a pipeline are concatenated together into a single string, which is fed through the grammar to get an AST, the nodes of which are used to create the plugins and control structure. Because the AST contains line and column metadata from the concatenated source string and not the original files, there is no way presently for a plugin instance to be mapped back to a specific file source.

We would either need to change how the AST is built (e.g., build multiple independent AST's and allow them to be concatenated after parsing to produce a net-same resulting AST with improved source metadata), or somehow map the line_and_column through a utility that is aware of which lines came from which files (e.g., if we know lines 0-72 in file X, 73-97 in file Y, an error on line 77 of the concatenated source is actually line 4 of file Y).

[jsvd]

I agree that providing this information means carrying new metadata from much earlier in the compilation process.

The goal of this suggestion is to have a way for users to correlate logs and metrics from plugins with the actual plugins without requiring the user to id them all. Linking auto generated IDs with file+line+column seems to be most user friendly, but happy to go back to drawing board at any time.

[andsel]

Merging ASTs could be a mess, because there is not any formal rule how the pipelines config files could be sliced, so we could have the filter section splitted in something like :

filter {
    geoip {
   }

and

   grok{}
}

At this point the remapping of line and cols after the global AST in created is the unique viable solution

[andsel]

Added the source file and source line into configuration reference for each metric node (Ruby pipeline)

[yaauie]

I think we are going to have to find a way to get this metadata to plugin instances without routing through their initialize methods (e.g., a map on the pipeline indexed by plugin id?). Changing the method signatures of classes that are extended by plugins in external codebases (including customer-owned private plugins) is going to put us in a position where we cannot be confident that our changes do not break real-world plugins.

[yaauie]

iterating over all of the parts for each lookup, doing all of the work to count lines of each config part each time, seems like it would have performance side-effects.

I think we can do the bulk of the work just once, by caching a mapping of file names, offsets, and sizes, and then using that mapping at lookup time instead of going all the way to the config parts.

Below is untested, and may be subject to off-by-one errors

    def lookup_source_and_line(merged_line_number)
      source_map.each do |part_offset, part_lines, part_id|
        rebased_line_number = merged_line_number - part_offset
        next if rebased_line_number > part_lines

        return [part_id, rebased_line_number]
      end
      
      raise IndexError
    end

    private

    def source_map
      @source_map ||= begin
        offset = 0
        source_map = []
        config_parts.each do |config_part|
          part_lines = config_part.getLinesCount()
          @source_map << [offset, part_lines, config_part.id]
          offset += part_lines
        end
        source_map.freeze
      end
    end

[andsel]

Thanks for all the suggestions. I've also introduced a class to express the concept of segments, and make it more readable

[andsel]

Jenkins test this please

[yaauie]

In general, I think this works as-is, but the complexity of routing everything through serialisation and the changes to the delegators and what they have to break out of the config arguments feels like it will be fragile.

I also do not immediately see a path forward that is less fragile. I'm going to take a stab at reworking this, and will circle back in the next day or so with a more concrete review and/or suggestions for a path forward.

[yaauie]

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

[andsel]

This counter could be cached, calculated only when text field is changed

[andsel]

I've created a PR #11288 which contains the common parts and the Java execution pipeline logic.
The files not ported that interests the Ruby's execution are:

• logstash-core/lib/logstash/config/config_ast.rb
• logstash-core/lib/logstash/pipeline.rb
• logstash-core/spec/logstash/pipeline_spec.rb
• logstash-core/lib/logstash/inputs/base.rb
• logstash-core/spec/logstash/inputs/base_spec.rb

[andsel]

Closed new work #11288 for reason of #11497 (comment)