-
Notifications
You must be signed in to change notification settings - Fork 126
Remove reroute processors from coverage reports #1647
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
Remove reroute processors from coverage reports #1647
Conversation
pipelines = append(pipelines, Pipeline{ | ||
Path: path, | ||
Name: getPipelineNameWithNonce(name[:strings.Index(name, ".")], nonce), | ||
Format: filepath.Ext(path)[1:], | ||
Content: c, | ||
Path: path, | ||
Name: getPipelineNameWithNonce(name[:strings.Index(name, ".")], nonce), | ||
Format: filepath.Ext(path)[1:], | ||
Content: cWithRerouteProcessors, | ||
ContentOriginal: c, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a new field ContentOriginal
in this struct to be able to keep the original contents, the ones read from the ingest pipeline file defined in the package.
It is being kept Content
to hold the same contents (containing reroute processors) to not change the current behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another consequence of adding those reroute processors, is that the YAML written afterwards is not the same as in the package:
- comments are not kept
- fields in the YAML do not keep the same order
// remove reroute processors if any so the pipeline has the same processors as in the file | ||
// reroute processors are added if there are any routing_rules file defined | ||
var processors []ingest.ProcessorStats | ||
for _, proc := range pstats.Processors { | ||
if proc.Type == "reroute" { | ||
continue | ||
} | ||
processors = append(processors, proc) | ||
} | ||
pstats.Processors = processors | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the stats returned by Elasticsearch contain the reroute
processors added by elastic-package in a previous step, here it is needed to remove those processors so it contains the same number of processors as in the file.
/test |
💚 Build Succeeded
History
cc @mrodm |
Relates #1595
Reroute processors are added to the ingest pipeline automatically if there is any
routing_rules.yml
defined in the data stream (added here #1372).If these reroute processors are added to the ingest pipeline, the stats obtained from the pipelines contain also those processors that are not defined in the original pipeline (ingest pipeline defined in the package).
This PR adds the logic to remove the reroute processors from the stats to generate the coverage reports. And at the same time, to be able to use the right lines in the coverage reports, it is needed to keep the original contents (the ones read from the file) of the pipeline.