elastic / elastic/logstash

Logstash::Pipeline.new signature

Open
#6,601 0 comments 0 reactions 0 assignees View on GitHub
code cleanup
Dominant language
Java
Stars
14.9k
Forks
3.5k
Avg merge
1d 4h
Merged PRs (30d)
88

Description

As noted in #6514 the agent code could benefit of refactoring of the pipeline's initialize signature to accept a `PipelineConfig` instance intead of a config string.

Instead of having this signature

`LogStash::Pipeline.new(config_string, settings, metric)` we would like to change it into.
`LogStash::Pipeline.new(pipeline_config, metric)` So all the settings and the config related information is encapsulated in the `pipeline_config` instance.

The problem is the `LogStash::Pipeline` class is also used in the plugins tests making it a bit harder to change without impact all the test suite.

I think the ways to make it work correctly would be one of the following:

1. create a new class named `RubyPipeline` with the new signature, change the current `Pipeline` class to adapt the arguments and use the `RubyPipeline` and proxy the public method to the `RubyPipeline`

2. Do some match of the Pipeline's arguments to make the code work in both cases?

I think 1 is a bit cleaner to do.

cc @jsvd

Contributor guide

Open the contributing guide

Research direction

Locate the LogStash::Pipeline class and the plugin tests that instantiate it. Compare the existing constructor call with the proposed PipelineConfig-based signature, then determine how the compatibility or RubyPipeline approach affects callers. Done means the intended signature is adopted without breaking the test suite.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
backend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.