search

severb / graypy

Python logging handler for Graylog that sends messages in GELF (Graylog Extended Log Format).
https://www.graylog.org/
BSD 3-Clause "New" or "Revised" License
260 stars 91 forks source link

Skip messages with more than 128 chunks #84

Closed akhoronko closed 5 years ago

akhoronko commented 6 years ago

According to GELF specification a message must not consists of more than 128 chunks (http://docs.graylog.org/en/latest/pages/gelf.html#chunking).

If you try to send a big message in more than 255 chunks GELFHandler.send will raise struct.error exception since sequence count is 1 byte, see https://gist.github.com/akhoronko/98665bf5229daf92f621d677239fbb83

severb commented 6 years ago

Is this patch dropping the entire message when it exceeds 128 chunks? If so, is it desirable to truncate to the maximum possible size instead?

akhoronko commented 6 years ago

Yes, the entire message will be dropped. We can't just truncate the message since it's serialized to the json already. Of course we can deserialize this json to the python dict, try to truncate the message, serialize to the json, try to send it again and so on, but it will waste resources. And who knows will the truncated message bring some value or it will be useless. May be some warning will be enough to show that application trying to send too big messages

Btw, GELF doesn't allow more than 255 chunks (1 byte) and such messages will be dropped with exception and mess the sender application's log with errors.

severb commented 6 years ago

Yes, the entire message will be dropped. We can't just truncate the message since it's serialized to the json already. Of course we can deserialize this json to the python dict, try to truncate the message, serialize to the json, try to send it again and so on, but it will waste resources. And who knows will the truncated message bring some value or it will be useless. May be some warning will be enough to show that application trying to send too big messages

I agree. Do you think it's a good idea to insert a static placeholder message to inform that the original one has been dropped because it exceeded the maximum length?

nklapste commented 5 years ago

@akhoronko with #118 are we good to close this PR? I assume this use-case is now covered with graypy 2.0.0.

akhoronko commented 5 years ago

@nklapste yes, I will close it