Skip to content

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

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Jan 26, 2024

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.

Comment on lines 107 to +112
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,
Copy link
Contributor Author

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.

Copy link
Contributor Author

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

Comment on lines 118 to 128
// 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

Copy link
Contributor Author

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.

@mrodm
Copy link
Contributor Author

mrodm commented Jan 29, 2024

/test

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm merged commit 808efee into elastic:main Jan 30, 2024
@mrodm mrodm deleted the remove_reroute_processor_coverage_pipeline branch January 30, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants