Add TCP support for GELF

[zsprackett]

Not final. I'm just submitting this for review. Still needs tests but it works and I want to make sure you're onboard.

[zsprackett]

@jordansissel can you have a look at this and let me know what you'd need to change for logstash?

Thanks

[lennartkoopmann]

Also looking at the changes now.

[lennartkoopmann]

Didn't test it yet but it generally looks good from my side. :+1:

[zsprackett]

Sweet. I'm concerned about what the best strategy for dealing with exceptions is. I don't think we can hide them completely from the caller as silently dropping things basically defeats the purpose of using TCP in the first place. Maybe @jordansissel will have an opinion on what he'd like to see as a caller.

[lennartkoopmann]

The UDP sender is by default sending the caused exception as GELF message. :)

[jordansissel]

Haven't tested either, but +1 on the proposal to support additional transports. Code reviewed and looks good to me.

In terms of exceptions, I think wrapping up exceptions in some kind of GELF::TransportException would make it easier for consumers of this library (like logstash) to react.

In logstash's case, if there's a transport error, logstash is likely to simply retry the call anyway, probably forever (with the goal of never dropping logs by default).

[jordansissel]

Alternately, from logstash's side, we could just use a "gelf" codec and then any input/output would be able to consume or produce gelf messages and any transport concerns go away.

[jordansissel]

For context, in case I missed any, codecs in logstash are used to separate transportation (input, output) concerns from serialization (gelf, json, gzip, protobuf, etc) concerns.

[lennartkoopmann]

A GELF codec for logstash sounds good. However if would at least need a special UDP output because of the byte header that needs to be prepended when the message is chunked.

Also keep in mind that with TCP the GELF payload must not be GZIP/ZLIB compressed. (because of the \0 framing that easily breaks when compressed)

[jordansissel]

Oh yeah, I forgot about stateful udp transmission for chunking. Let's pretend I didn't say anything about codecs, then! ;)

[lennartkoopmann]

:)

I'll keep an eye on this PR in case any questions appear.

[jordansissel]

After this merges/releases, we can update the gelf output for logstash to let you select a transport:

output { gelf { transport => "tcp" } }

or something?

I see this also adds support to multiple endpoints which are selected using round-robin. We can add support for this configuration in logstash as well.

[lennartkoopmann]

@zsprackett please ping here when you consider this PR ready for merging. Thanks!

[zsprackett]

I think this is about as good as I'll get it at this point.

[lennartkoopmann]

I have not forgotten this and will merge soon. :) Thanks!

[zsprackett]

i assumed it was still waiting on me to add tests. :) no worries...

[kke]

+1

[JeremyEinfeld]

+1

[dstockman]

TCP & GELF would be the bees knees. Any possibility of this getting merged soon?

[dougrad]

+1

[bgregersen]

+1

[abecciu]

+1

[rcord02]

+1

[kiranos]

@lennartkoopmann any new status of what needs to get done to get this merged? would be great to give logstash the ability to ship logs to graylog2 over tcp.

[rajatvig]

+1

[ofross]

Hey @lennartkoopmann - looks like this PR is merge-able - Are there any bits that we can do out here to help you out? I'm definitely in the banging-my-head-to-get-gelf+tcp camp over here, as I know @rajatvig is too! Thanks so much!

[joschi]

@zsprackett Could you please sign the TORCH Contributor Agreement as outlined in CONTRIBUTING.md and send it to hello@torch.sh? Other than that we're good to go.

[asm0dey]

:+1:

[zsprackett]

CA sent in today.

[lennartkoopmann]

Hey @zsprackett,

we unfortunately have a timing problem here. The IP holding entity has changed to Graylog, Inc and we need to adapt the CA. This might take us a few days and you'll have to re-issue it. Sorry about that.

Thanks, Lennart

[craigw]

Hello all,

Thanks for your work on this - I'd really like to use it. Are there any updates you can share on the progress of getting this merged in?

Cheers, Craig

[dennusb]

Is there any update?

[happysundar]

Ping!

[00WCEnte]

+1

[lazureykis]

+1

[rlanore]

+1

[andral]

+1

[daniilyar]

+1

[edefaria]

In order to use a patched logstash to send logs to a graylog2 with TCP/TLS support. I base on this patch and I add the TLS support by forking TCP classes. I have encountered some issue with dead socket and half-dead socket (socket timeout because we use an HA proxy behind the graylog2 server). So, I add some more reliable check (read the socket after writing to catch more exception). That is my first code in ruby, so it may probably better, but it does the job.

[evanccnyc]

Any update on when this might be merged in?

[vladimirsavenkov]

+1

[asm0dey]

:+1:

[Mark2009]

+1

On Tue, Jul 21, 2015 at 8:58 AM, Paul notifications@github.com wrote:

[image: :+1:]

— Reply to this email directly or view it on GitHub https://github.com/Graylog2/gelf-rb/pull/21#issuecomment-123296856.

[zsprackett]

What do i need to sign to get this merged?

[dstockman]

@zsprackett Guessing it's this: https://www.graylog.org/contributing-to-graylog/

[zsprackett]

Resubmitted CLA

[dstockman]

Thanks! @zsprackett for President 2016!

[siawyoung]

I needed this quite urgently so I forked the current master and merged @zsprackett's 3 commits in (TCP works quite smoothly btw, great job!)

https://github.com/siawyoung/gelf-rb

[edefaria]

You manage the TCP/UDP option by adding a default option protocol. So the gelf, add a protocol field by default with this patch (with 0 or 1 as value). This information is futile and I think it should not be added as a gelf field by default.

I also think that would be cleaner to move "module Protocol" for TCP definition from file "gelf.rb" to the begin of the file "ruby_sender.rb".

[adrianlyjak]

+1