search

Graylog2 / gelfclient

GELF client library for Java based on Netty 4
https://www.graylog.org/
Apache License 2.0
38 stars 19 forks source link

Revert support for boolean values #31

Closed bernd closed 6 years ago

bernd commented 7 years ago

The GELF spec does not support boolean values for custom message fields and the Graylog GELF code will currently drop all message fields with boolean values. (see https://github.com/Graylog2/graylog2-server/pull/3958)

Since we previously said that boolean values are not supported due to the spec, I think https://github.com/Graylog2/gelfclient/pull/30 should be reverted. Otherwise people might lose message field data when sending GELF messages to Graylog.

bernd commented 7 years ago

And I apologize for totally missing that during my review on #30. :confused:

bernd commented 7 years ago

We should extend the GELF spec to be explicit about boolean values instead.

I agree. But until then, this library is broken when using boolean values.

They're not forbidden and there's no list of valid data types in GELF messages.

Our documentation for GELF explicitly states: "_[additional field] string (UTF-8) or number"

See: http://docs.graylog.org/en/latest/pages/gelf.html#gelf-payload-specification

There's no reason not to support them and we're already quite liberal in what we accept in Graylog and there are already libraries out there (unknowingly) supporting boolean values, such as gelfj.

As said, I agree. I am perfectly fine adding support for boolean values after 2.3. I just don't want to put this in at the very last moment and break something.

joschi commented 6 years ago

@bernd You're completely correct. I've reverted #30 and we'll have to improve the GELF payload format in the next version of the specification. 👍