magnusbaeck / logstash-filter-verifier

Apache License 2.0
192 stars 27 forks source link

Remove the input and output config sections #57

Closed kvalev closed 5 years ago

kvalev commented 6 years ago

This RP automatically removes the input and output config sections when copying over the logstash config file(s), which will allow testing the actual configuration that is being used.

Ref #10

kvalev commented 6 years ago

Let me know if you would be reluctant to make such a change to the functionality.

breml commented 6 years ago

If my logstash config parser will be used here I should maybe give it some love in the future.

matejzero commented 6 years ago

Would this also be useful if you have your config files in conf.d and each section in it's own file? Right now I copy all filters without input/output configuration to a separate folder, run tests and then delete it. Not that it's complicated now that the scripts are written and tested but elimination of one more step is always welcome:)

magnusbaeck commented 6 years ago

@matejzero wrote:

Would this also be useful if you have your config files in conf.d and each section in it's own file?

Yes, unless Logstash objects to reading an empty file (which we then we have to omit when copying the files).

Right now I copy all filters without input/output configuration to a separate folder, run tests and then delete it.

If you name your configuration files distinctly you can give LFV a filename pattern rather than a directory. For example, all my configuration files containing filters are have names matching *-filter.conf.

kvalev commented 6 years ago

Honestly, I wasn't aware that logstash allows loading multiple configuration files. I guess this explains why this change was not implemented yet. I tested the handling of empty configuration files and it seems that logstash does not care much (at least the latest version). If you want I can change it, so that empty files are not copied over. Also thanks for the library @breml, I was pleasantly surprised that there was a golang logstash config parser library.

matejzero commented 6 years ago

@magnusbaeck: thanks for the idea. My files are already named similar to your, so this is an easy fix:)

breml commented 6 years ago

@magnusbaeck the advantage of my logstash parser library is, that it detects syntax errors in the logstash config a lot faster than logstash it self due to the very slow startup time of logstash. my library needs only fractions of a second whereas logstash took up to 30 seconds. the difference is orders of magnitudes. so the addition of this library could make sense even if you have your logstash config already divided into multiple files (separation of inputs, filters and outputs).

kvalev commented 6 years ago

@magnusbaeck A while back I added a commit that hopefully addresses your comments. Is there anything else (besides the documentation), that has to be changed?

breml commented 5 years ago

I just added go.mod file to github.com/breml/logstash-config package and tagged it with v0.1.0 in order to work with modules in Go 1.11.

magnusbaeck commented 5 years ago

Thank you @kvalev! I apologize for the ridiculously slow response. It's very hard to find time for hobby projects with an infant at home.

I'll just fix up the documentation and rewrite the tests for removeInputOutput() to be table-driven. I'll include this in a 1.5.0 release along with a few other changes.

matejzero commented 5 years ago

Thank you both for this addition. It will be very useful.

On 1 Sep 2018, at 20:02, Magnus Bäck notifications@github.com wrote:

Thank you @kvalev https://github.com/kvalev! I apologize for the ridiculously slow response. It's very hard to find time for hobby projects with an infant at home.

I'll just fix up the documentation and rewrite the tests for removeInputOutput() to be table-driven. I'll include this in a 1.5.0 release along with a few other changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/magnusbaeck/logstash-filter-verifier/pull/57#issuecomment-417877099, or mute the thread https://github.com/notifications/unsubscribe-auth/AEeNZEpMWsSiHbD6P50hGJIoE45ABwuXks5uWsu9gaJpZM4UMoPc .