search

magnusbaeck / logstash-filter-verifier

Apache License 2.0
192 stars 27 forks source link

Change default codec to `line` #24

Closed breml closed 7 years ago

breml commented 7 years ago

Issue to followup on the discussion in #15.

Initial question by @magnusbaeck:

Is there a reason not to make the line codec the default? Having to adjust the test case files to make --sockets work doesn't feel right.

breml commented 7 years ago

@magnusbaeck do you remember, why you choosed plain as default codec in https://github.com/magnusbaeck/logstash-filter-verifier/blob/master/testcase/testcase.go#L112 in the first place?

Based on https://github.com/logstash-plugins/logstash-input-stdin/blame/master/lib/logstash/inputs/stdin.rb#L15 and https://github.com/logstash-plugins/logstash-input-stdin/blame/master/lib/logstash/inputs/stdin.rb#L38 the logstash-input-stdin plugin used the line codec for a long time. When reading from stdin, one always get the content line by line. So I assume, that there is no difference between using plain or line codec.

So my question is, is it possible to change https://github.com/magnusbaeck/logstash-filter-verifier/blob/master/testcase/testcase.go#L112 to line?

matejzero commented 7 years ago

Quick note, you have a typo in README.md, where you say one should have propery lines in test case, not line, which is correct name for codec.

For the test cases to work properly together with the unix domain socket input, the test case files need to include the property codec set to the value lines (or json_lines, if json formatted input should be processed).

magnusbaeck commented 7 years ago

@magnusbaeck do you remember, why you choosed plain as default codec in https://github.com/magnusbaeck/logstash-filter-verifier/blob/master/testcase/testcase.go#L112 in the first place?

Probably no good reason at all. I think I just assumed that it was the default, possibly after looking at the documentation. It's weird that the code indicates that "line" is the default codec when the docs state that "plain" is the default. As long as "line" works (which I have no reason to believe it doesn't) I'm happy with a codec switch.

breml commented 7 years ago

Tested all my test cases with codec line in both modes (stdin and unix domain sockets) and this worked without problems. Also performed a quick test with json_lines and suggest to change the recommendation for json based input to json_lines.