in_syslog: Provide appending source address parameter #7651
Conversation
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
requested review from
edsiper,
leonardo-albertovich,
fujimotos and
koleini
as code owners
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
There was a problem hiding this comment.
We need to make a few changes so this doesn't cause trouble in the future :
Unify the implementations of append_raw_message_to_record_data and append_source_address_to_record_data in a single generic function that receives the type, pointer and length of the new value. As a starting point just add support for MSGPACK_OBJECT_BIN and MSGPACK_OBJECT_STR and for any other types return an error.
Let's make the most of this opportunity to improve code in this newly created function by making these changes :
- Because flb_msgpack_expand_map already allocates a new buffer we can just avoid repackaging the data and return that buffer directly.
- Modify the code of the newly created function so it has 3 possible result values :
SUCCESS,MAP NOT MODIFIEDandMAP EXPANSION ERROR
Then you need to modify pack_line to properly process the result code and only print an error message if MAP_EXPANSION_ERROR is returned.
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
plugins/in_syslog/syslog_prot.c
Outdated
| { | ||
| int i; | ||
| int result; |
There was a problem hiding this comment.
Initialize result to FLB_MAP_NOT_MODIFIED, othwewise if message_key_name is NULL the check in line 87 is accessing garbage which considering that the value of FLB_MAP_EXPAND_SUCCESS is 0 could cause this function to return garbage.
plugins/in_syslog/syslog_prot.c
Outdated
|
|
||
| *result_buffer = mp_sbuf.data; | ||
| *result_size = mp_sbuf.size; | ||
| if (result != FLB_MAP_EXPAND_SUCCESS) { |
There was a problem hiding this comment.
As per the comment in line 42 you need to invert the clause in this conditional block and remove the result override.
Then you need to add an else block to the clause in line 82 so for any other values returned by flb_msgpack_expand_map result is set to FLB_MAP_EXPANSION_ERROR.
plugins/in_syslog/syslog_prot.c
Outdated
| message_entry.val.via.str.ptr = message_buffer; | ||
| } | ||
| else { | ||
| result = FLB_MAP_NOT_MODIFIED; |
There was a problem hiding this comment.
Add a new constant named FLB_MAP_EXPANSION_INVALID_VALUE_TYPE and use it in this line, trying to use a type that's not acceptable is an error, we don't want to miss it.
plugins/in_syslog/syslog_prot.c
Outdated
|
|
||
| msgpack_pack_map(&mp_pck, unpacked_buffer.data.via.map.size + 1); | ||
| return result; |
There was a problem hiding this comment.
Remove this return and wrap the code in lines 77-84 in a conditional that only executes if result equals FLB_MAP_NOT_MODIFIED
Signed-off-by: Hiroshi Hatake <[email protected]>
|
@leonardo-albertovich pls review latest changes |
* in_syslog: Append source_address into records if needed
* in_syslog: Append source_address into records if needed
Closes #7581
Currently, in_syslog plugin does not offer adding source address feature.
This PR provides this feature.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
fluent/fluent-bit-docs#1151
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.