search

t0xa / gelfj

Graylog Extended Log Format (GELF) implementation in Java and log4j appender without any dependencies.
https://github.com/t0xa/gelfj/wiki
Other
186 stars 116 forks source link

Chunking implmentation seems to be broken #22

Closed narkisr closed 12 years ago

narkisr commented 12 years ago

While developing gelfino (a tiny gelf server) I used your gelf client as a bench to test message send, the sequence and count fields are wrong (I get 52 53 as values) I think its related to the way int's are converted to bytes (gelf4r seems to do this right)

Iv used both gelf4r and gelf4j and they work fine,

Thanks Ronen

rocketraman commented 12 years ago

I am getting this error on the graylog2 server, which appears to be caused by the issue reported here.

2012-02-03 01:14:25,018 ERROR: org.graylog2.messagehandlers.gelf.GELFClientHandlerThread - Invalid GELF header in message: org.graylog2.messagehandlers.gelf.InvalidGELFChunkException: Invalid GELF chunk: Sequence number must be lower than sequence count.
org.graylog2.messagehandlers.gelf.InvalidGELFHeaderException: org.graylog2.messagehandlers.gelf.InvalidGELFChunkException: Invalid GELF chunk: Sequence number must be lower than sequence count.
        at org.graylog2.messagehandlers.gelf.ChunkedGELFClientHandler.<init>(ChunkedGELFClientHandler.java:74)
        at org.graylog2.messagehandlers.gelf.GELFClientHandlerThread.run(GELFClientHandlerThread.java:58)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
        at java.lang.Thread.run(Thread.java:636)
Caused by: org.graylog2.messagehandlers.gelf.InvalidGELFChunkException: Invalid GELF chunk: Sequence number must be lower than sequence count.
        at org.graylog2.messagehandlers.gelf.GELFClientChunk.checkStructure(GELFClientChunk.java:159)
        at org.graylog2.messagehandlers.gelf.ChunkedGELFMessage.insertChunk(ChunkedGELFMessage.java:52)
        at org.graylog2.messagehandlers.gelf.ChunkedGELFClientManager.insertChunk(ChunkedGELFClientManager.java:70)
        at org.graylog2.messagehandlers.gelf.ChunkedGELFClientHandler.<init>(ChunkedGELFClientHandler.java:72)
        ... 4 more

It seems to happen pretty consistently given a specific log message input.

t0xa commented 12 years ago

Hi,

Thanks for finding this!

Anton

rocketraman commented 12 years ago

I submitted a https://github.com/t0xa/gelfj/pull/24 to fix this -- the current code didn't seem to be following the correct GELF header spec as per https://github.com/Graylog2/graylog2-docs/wiki/GELF.

t0xa commented 12 years ago

Thanks. However this code will break compatibility with graylog2-server < 0.9.6.

rocketraman commented 12 years ago

I haven't tried it with < 0.9.6, but I don't think so... if you look at the GELF wiki history it seems like it has always been this way, except for during one day on Jul 15th when someone changed it, and than lennart reverted the change.

Besides, if graylog has changed it in 0.9.6 you are going to have to release the changes anyway, with a note that it only works with 0.9.6 and above.

rocketraman commented 12 years ago

Also, take a look at:

https://github.com/Graylog2/graylog2-server/commits/develop/src/main/java/org/graylog2/messagehandlers/gelf/GELFHeader.java

t0xa commented 12 years ago

That's quite different from: https://github.com/Graylog2/graylog2-server/blob/0.9.5/src/main/java/org/graylog2/messagehandlers/gelf/GELFHeader.java

But you're right, at some point I should migrate to 0.9.6.

rocketraman commented 12 years ago

Hmm, one would think this would have been an important change to highlight in the graylog 0.9.6 release notes!

t0xa commented 12 years ago

I'll note that on Wiki.