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

filter_lua: support nil_str to replace null value by nil_str string #7799

Closed
wants to merge 3 commits into from

Conversation

nokute78
Copy link
Collaborator

@nokute78 nokute78 commented Aug 6, 2023

#7708
In Lua, nil value is a special value to delete key/value pair from associative array.
https://www.lua.org/pil/2.5.html

you can assign nil to a table field to delete it.

It means the key/value will be removed from record if its value is null.
e.g. If record is {"aaa":null, "bbb":"cccc"} ,"aaa":null will be removed in Lua.

This patch is to add nil_str property to prevent it.
Default value is NULL to keep backward compatibility.

Value Description Default
nil_str If set, nil value will be replaced by this string in Lua. It is to prevent remove nil value from record since nil value is to delete key/value from associative array in Lua. NULL
  1. null value is replaced by nil_str if set when record is converted from msgpack to Lua value.
  2. The string is replaced by null value if the string is nil_str when record is converted from Lua value to msgpack.

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:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • [N/A] Backport to latest stable release.

Configuration

[INPUT]
    Name dummy
    Dummy {"aaa":null, "bbb":"cccc"}

[FILTER]
    Name    Lua
    Match   *
    call    append_tag
    code    function append_tag(tag, timestamp, record) new_record = record new_record["extra"] = {} return 1, timestamp, new_record end
    Nil_Str nil

[OUTPUT]
    Name            stdout
    Match           *
    format          json_lines

Debug/Valgrind output

"aaa":null is not removed by nil_str.

$ valgrind --leak-check=full bin/fluent-bit -c ~/git/fluentbit-issue-conf/7708/a.conf 
==73292== Memcheck, a memory error detector
==73292== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==73292== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==73292== Command: bin/fluent-bit -c /home/taka/git/fluentbit-issue-conf/7708/a.conf
==73292== 
Fluent Bit v2.1.9
* Copyright (C) 2015-2022 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2023/08/06 13:25:51] [ info] [fluent bit] version=2.1.9, commit=5686334b56, pid=73292
[2023/08/06 13:25:51] [ info] [storage] ver=1.2.0, type=memory, sync=normal, checksum=off, max_chunks_up=128
[2023/08/06 13:25:51] [ info] [cmetrics] version=0.6.3
[2023/08/06 13:25:51] [ info] [ctraces ] version=0.3.1
[2023/08/06 13:25:51] [ info] [input:dummy:dummy.0] initializing
[2023/08/06 13:25:51] [ info] [input:dummy:dummy.0] storage_strategy='memory' (memory only)
[2023/08/06 13:25:51] [ info] [output:stdout:stdout.0] worker #0 started
[2023/08/06 13:25:51] [ info] [sp] stream processor started
{"date":1691295951.670228,"bbb":"cccc","extra":{},"aaa":null}
{"date":1691295952.684662,"bbb":"cccc","extra":{},"aaa":null}
^C[2023/08/06 13:25:53] [engine] caught signal (SIGINT)
{"date":1691295953.647959,"bbb":"cccc","extra":{},"aaa":null}
[2023/08/06 13:25:53] [ warn] [engine] service will shutdown in max 5 seconds
[2023/08/06 13:25:53] [ info] [input] pausing dummy.0
[2023/08/06 13:25:54] [ info] [engine] service has stopped (0 pending tasks)
[2023/08/06 13:25:54] [ info] [input] pausing dummy.0
[2023/08/06 13:25:54] [ info] [output:stdout:stdout.0] thread worker #0 stopping...
[2023/08/06 13:25:54] [ info] [output:stdout:stdout.0] thread worker #0 stopped
==73292== 
==73292== HEAP SUMMARY:
==73292==     in use at exit: 0 bytes in 0 blocks
==73292==   total heap usage: 1,841 allocs, 1,841 frees, 1,636,032 bytes allocated
==73292== 
==73292== All heap blocks were freed -- no leaks are possible
==73292== 
==73292== For lists of detected and suppressed errors, rerun with: -s
==73292== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

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.

Nil value is to delete key/value from associative array in Lua.
It means a key/value is removed from record if its value is null.
nil_str is to replace null by nil_str string to prevent it.

Signed-off-by: Takahiro Yamashita <[email protected]>
@nokute78 nokute78 temporarily deployed to pr August 6, 2023 04:31 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr August 6, 2023 04:31 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr August 6, 2023 04:31 — with GitHub Actions Inactive
@nokute78 nokute78 temporarily deployed to pr August 6, 2023 04:58 — with GitHub Actions Inactive
@edsiper
Copy link
Member

edsiper commented Aug 29, 2023

@tarruda can you take a look pls ?

@edsiper edsiper added this to the Fluent Bit v2.1.9 milestone Aug 29, 2023
@edsiper
Copy link
Member

edsiper commented Aug 31, 2023

@tarruda ping

@tarruda
Copy link

tarruda commented Sep 1, 2023

@nokute78 have you considered using a light userdata value to represent null? This pattern is what cjson uses. We could have a global NULL userdata value which can be used by scripts and also by fluent-bit when converting from/to msgpack. There are some advantages to userdata over string:

  • All strings continue to work normally
  • Checking if a user is null doesn't require using strncmp, it is just an instant pointer comparison.
  • You can store the userdata as a global value (it is just a dummy pointer), which would make it immediately accessible to the msgpack conversion functions (no need to change them to receive a new parameter)

You can still keep backwards compatibility by exposing a boolean option (enable_nulls)

Here's how you create a lightuserdata: https://www.lua.org/pil/28.5.html

@nokute78
Copy link
Collaborator Author

nokute78 commented Sep 1, 2023

@tarruda Thank you for information. I'll check it.

@nokute78 nokute78 mentioned this pull request Sep 8, 2023
5 tasks
@nokute78
Copy link
Collaborator Author

nokute78 commented Sep 8, 2023

I created a new PR and close this PR.
#7906

@nokute78 nokute78 closed this Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants