Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chunking implmentation seems to be broken #22

Closed
narkisr opened this issue Feb 1, 2012 · 9 comments
Closed

Chunking implmentation seems to be broken #22

narkisr opened this issue Feb 1, 2012 · 9 comments
Assignees

Comments

@narkisr
Copy link

narkisr commented Feb 1, 2012

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
Copy link
Contributor

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
Copy link
Owner

t0xa commented Feb 6, 2012

Hi,

Thanks for finding this!

Anton

@ghost ghost assigned t0xa Feb 6, 2012
@rocketraman rocketraman mentioned this issue Feb 6, 2012
@rocketraman
Copy link
Contributor

I submitted a #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
Copy link
Owner

t0xa commented Feb 6, 2012

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

@rocketraman
Copy link
Contributor

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
Copy link
Contributor

@t0xa
Copy link
Owner

t0xa commented Feb 7, 2012

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
Copy link
Contributor

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

@t0xa
Copy link
Owner

t0xa commented Feb 9, 2012

I'll note that on Wiki.

@t0xa t0xa closed this as completed Feb 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants