Add support for proxy protocol

[nneul]

Adds proxy protocol support to log4j plugin

Related to:

logstash-plugins/logstash-input-tcp#57
elastic/logstash#4418
logstash-plugins/logstash-input-syslog#30

[nneul]

Not sure what's up with the build failure - it doesn't seem to reference anything I touched other than the input itself.

[nneul]

@jordansissel I have absolutely no idea how to write the spec test case for this one as there is no similar example in the current tests. I made the switch to boolean in the settings though.

This plugin also fails existing tests without my changes.

[nneul]

@ph Can you take a look at this PR? Suggestions for the testing are welcome as well.

[jordansissel]

I didn't have time to review this last week, btw. If you would rather @ph
review this, that is ok.

On Sat, Nov 19, 2016 at 5:16 PM Nathan Neulinger notifications@github.com
wrote:

@ph https://github.com/ph Can you take a look at this PR? Suggestions
for the testing are welcome as well.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#34 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIC6oF1BlAeWtNsHPwZK6OphmicM2FDks5q_59RgaJpZM4KkaMl
.

[nneul]

No preference... wasn't sure who was primary...

[jordansissel]

This is still on my todo list.

[jordansissel]

Sorry again for the delays. I'm working on making it so this plugin can work at all on Logstash 5.x -- Tracking here: elastic/logstash#6309

[jordansissel]

I reviewed the code and it looks good to me.

I have ideas on how to write a test for this:

0. Run nc -l 4000 > log4j.capture to listen on port 4000 and capture the data sent
1. Configure a log4j1 app to use SocketAppender to localhost:4000
2. Send 1 log message, maybe something simple like `logger.info("Hello world")
3. Use this captured payload as the payload to send in the test suite

The above might be simpler to maintain than setting up a full log4j logger with SocketAppender, but I'd accept a log4j SOcketAppender test also.

[nneul]

I'll take at what I can come up with for 0-3 above. Can definitely get 0-2, just need to review the code for how to do #3.

[nneul]

Can you trigger a CI test for this package unmodified? I'd really like to start from a known working state, and right now, pretty sure the tests are failing unrelated to my changes.

[ph]

@nneul I've rekicked the job

[nneul]

I was meaning on the mainline - not on my branch.

[ph]

@nneul I've kicked both the master branch and yours, you were right the master branch has also the same problem. See https://travis-ci.org/logstash-plugins/logstash-input-log4j/builds/151618256

[nneul]

Thanks. I'll ignore - trying to figure out enough of this testing mechanism to write the requested test case.

[nneul]

This doesn't work, but I'm not fluent in the any of this test infrastructure (or ruby for that matter). Can you take a look at the wip test code and see if anything looks obviously the wrong way to go about it - it's just hanging for me on the 'collect' line.

[nneul]

Note to be clear - that test/wip isn't touching the proxy protocol stuff yet - figured it needed to test the base/default case first. That's what I'm stuck on.

[nneul]

Can you merge this - since the underlying proxy protocol code works (I've been running it on my environment for a while now live), and can follow up with working test code once there is time to figure out how to make it work?

[jordansissel]

Since this is a plugin covered by our support team , we really need to have tests before merging code in. I will work on tests as I have time and merge it once I finish. Probably some time in January.

…

On Fri, Dec 30, 2016 at 7:09 AM Nathan Neulinger ***@***.***> wrote: Can you merge this - since the underlying proxy protocol code works (I've been running it on my environment for a while now live), and can follow up with working test code once there is time to figure out how to make it work? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIC6vNHyD5hzz9CkhvuZ33WWWcatooxks5rNR6dgaJpZM4KkaMl> .

[nneul]

Ok. Whenever you have a chance to look at it - if you have any ideas on why what I was trying isn't working, let me know. If we can get the base case working, adding test for proxy proto in the same style should be trivial.

[jordansissel]

@nneul did you intend to have this not set proxy_host or proxy_port fields? The TCP input does this :

• https://github.com/logstash-plugins/logstash-input-tcp/blob/master/lib/logstash/inputs/tcp.rb#L183-L188

[jordansissel]

I have forked this PR to #38.