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

[Filebeat] allow ingest pipelines in YAML format #11209

Merged
merged 4 commits into from
Mar 13, 2019

Conversation

adriansr
Copy link
Contributor

This updates the ingest pipeline loading to look at the pipeline definition file extension and use a JSON or YAML decoder depending on the type.

This improves readability a lot, as now it's possible to add comments and multiline strings.

This updates the ingest pipeline loading to look at the
pipeline definition file extension and use a JSON or YAML
decoder depending on the type.
@adriansr adriansr requested a review from a team as a code owner March 12, 2019 14:44
@adriansr
Copy link
Contributor Author

adriansr commented Mar 12, 2019

Didn't add a test, but #11171 includes a YAML pipeline that will be used during tests

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

This definitely improves readability.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM, great idea. Looking forward to our first .yml pipeline.

This could even be worth an entry in the dev changelog.

@kvch
Copy link
Contributor

kvch commented Mar 13, 2019

The Filebeat modules generator documentation also needs a follow-up. When generating a new Filebeat fileset, it would be nice to provide a YAML pipeline instead of a JSON. Or maybe making it configurable, if someone prefers JSON over YAML.
@adriansr Could you please create a new issue for the follow-up in Filebeat module generators? Or if you feel like it, you can do it in this PR. :D

@ruflin
Copy link
Member

ruflin commented Mar 13, 2019

I would keep the default as JSON as otherwise it might be confusing the users. My guess is that most users are not aware that ES API's support YAML.

@kvch
Copy link
Contributor

kvch commented Mar 13, 2019

OK. Still the documentation needs to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants