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

out_gelf: Fluent Bit is refusing to send what Graylog accepts as a GELF message #2256

Closed
alitoufighi opened this issue Jun 13, 2020 · 3 comments

Comments

@alitoufighi
Copy link
Contributor

alitoufighi commented Jun 13, 2020

Bug Report

My story with log levels never ends.

Describe the bug
When the log level is an integer, we check its value according to the GELF payload specification that chooses the value to be a number between 0 and 7.

if (val_len == 1 && val[0] >= '0' && val[0] <= '7') {
v = &vtmp;
v->type = MSGPACK_OBJECT_POSITIVE_INTEGER;
v->via.u64 = (uint64_t)(val[0] - '0');
}

But the fact is that this field is optional and Graylog doesn't do anything special with it (Apparently, at least since Graylog 2.x).

I mean, here I successfully sent a GELF message with level 50 (Without Fluent Bit GELF output plugin):
image

It is usual for logging libraries to not follow syslog severity levels, so that lots of applications may have difficulties to convert these levels to what GELF (specifically, in Flutent Bit plugin) asks.

But I believe Fluent Bit mustn't be a "pain in the ass" for the person who is responsible to facilitate the management of logs. So I think allowing this field to be any arbitrary integer, and forward it anyway, and let the destination server decide if it accepts it or not, can be a better idea.

To Reproduce
Create a log with level key that is not between 0 and 7:

{"log":"foobar", "level":10}

and GELF output plugin will complain:

[flb_msgpack_to_gelf] level is 10, but should be in 0..7 or a syslog keyword

Expected behavior
I expect this log to be sent to the server, and if the server rejects it, it's not the problem with Fluent Bit. This field is optional and I think it being optional has made the specifications of it optional as well.

Your Environment

  • Version used: master
@alitoufighi
Copy link
Contributor Author

alitoufighi commented Jun 13, 2020

@manuelluis and @edsiper I'd be happy to hear your feedbacks.

@alitoufighi
Copy link
Contributor Author

alitoufighi commented Jun 14, 2020

Using this configuration:

[INPUT]
        Name   dummy
        Tag    kube.dummy
        Dummy {"level": 30, "time": "2020-06-13T15:02:12.637309", "host": "inja", "short_message":"{\"key1\":\"val1\"}"}

[OUTPUT]
        Name                    gelf
        Match                   *
        Host                    my-graylog-instance
        Port                    12201
        Mode                    tcp
        Gelf_Short_Message_Key  short_message

Simply if we remove the return NULL from this scope (when level is int):

if (v->type == MSGPACK_OBJECT_POSITIVE_INTEGER) {
if ( v->via.u64 > 7 ) {
flb_error("[flb_msgpack_to_gelf] level is %" PRIu64 ", "
"but should be in 0..7 or a syslog keyword", v->via.u64);
return NULL;
}
}

This works:
image

And if we use a string log level by changing the dummy message above to this:

{"level": "ERROR", "time": "2020-06-13T15:02:12.637309", "host": "inja", "short_message":"{\"key1\":\"val1\"}"}

And remove the return NULL instruction from here:

if (allowed_levels[n] == NULL) {
flb_error("[flb_msgpack_to_gelf] level is '%.*s', "
"but should be in 0..7 or a syslog keyword", val_len, val);
return NULL;
}

The only complain is from the Elasticsearch side where it requires level to be of type long:

error=<{"type":"mapper_parsing_exception","reason":"failed to parse field [level] of type [long] in document with id 'cb767b26-ae14-11ea-936f-00505680ecd6'","caused_by":{"type":"illegal_argument_exception","reason":"For input string: \"ERROR\""}}>

In this case also I believe if someone likes to send his log levels in string format, she can simply change data type of that field to string in ES and this shouldn't be a concern for Fluent Bit.

@alitoufighi
Copy link
Contributor Author

Closing in favor of merging #2257

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

1 participant