plugin_proxy: enable event_type specification for proxy plugins #11011
Conversation
jmccormick7
requested review from
edsiper,
fujimotos and
koleini
as code owners
WalkthroughThis changeset introduces proxy plugin support for logs in Fluent Bit. It adds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
If we proceed to address this issue, we need to send another PR into here https://github.com/chronosphereio/calyptia-plugin to handle event_type as well. |
@cosmo0920 I can open a PR in that repository tonight! |
|
@cosmo0920 How would you prefer I do testing beyond the unit tests? I've done some manual testing that I can document, but is there any formalized regression testing or tests for proxy plugins? |
|
How about adding runtime shell test cases with precompiled Golang shared objects here? https://github.com/fluent/fluent-bit/tree/master/tests/runtime_shell |
|
@cosmo0920 I have added a runtime test that build and test a go plugin shared object. Please let me know if there are any improvements or other additions I can provide to speed up the timeline! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
tests/runtime_shell/go_plugins/build_test_plugins.sh (1)
98-98: Consider using FLB_BUILD variable instead of hardcoded path.The CMakeLists.txt now exports FLB_BUILD to the environment. Consider using
$FLB_BUILDinstead of constructing$BUILD_DIRto ensure consistency with the test environment.Apply this diff:
- CGO_ENABLED=1 GO111MODULE=on go build -buildmode=c-shared -v -ldflags="-s -w" -o $BUILD_DIR/test_logs_go.so logs_output.go + CGO_ENABLED=1 GO111MODULE=on go build -buildmode=c-shared -v -ldflags="-s -w" -o ${FLB_BUILD:-$BUILD_DIR}/test_logs_go.so logs_output.gotests/runtime_shell/proxy_logs_expect.sh (2)
19-19: Use FLB_BUILD variable for consistent path construction.The script hardcodes
$FLB_ROOT/build/test_logs_go.so, but the CMakeLists.txt now providesFLB_BUILDin the environment for this purpose.Apply this diff:
- $FLB_BIN -e $FLB_ROOT/build/test_logs_go.so -c $FLB_RUNTIME_SHELL_CONF/proxy_logs_test.conf > "$STDOUT_OUTPUT_FILE" 2>&1 & + $FLB_BIN -e $FLB_BUILD/test_logs_go.so -c $FLB_RUNTIME_SHELL_CONF/proxy_logs_test.conf > "$STDOUT_OUTPUT_FILE" 2>&1 &
22-22: Replace hardcoded sleep with signal file polling.The hardcoded
sleep 3is fragile and can cause flakiness in tests. Consider polling for the SIGNAL_FILE_PATH or checking the process output instead of using a fixed delay.tests/runtime_shell/go_plugins/test_logs_go.h (1)
1-92: Consider excluding auto-generated CGO headers from version control.This file is marked as auto-generated by cmd/cgo and should not be manually edited. Consider adding
*.hor specificallytest_logs_go.hto .gitignore in this directory, and generate it during the build process instead.tests/runtime_shell/go_plugins/logs_output.go (1)
28-28: Consider using the timestamp from GetRecord.The timestamp returned by
GetRecord(second return value) is currently ignored. While this is acceptable for a test plugin, consider whether timestamp information would be useful for test verification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/runtime_shell/go_plugins/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
tests/runtime_shell/CMakeLists.txt(2 hunks)tests/runtime_shell/conf/proxy_logs_test.conf(1 hunks)tests/runtime_shell/go_plugins/build_test_plugins.sh(1 hunks)tests/runtime_shell/go_plugins/go.mod(1 hunks)tests/runtime_shell/go_plugins/logs_output.go(1 hunks)tests/runtime_shell/go_plugins/test_logs_go.h(1 hunks)tests/runtime_shell/proxy_logs_expect.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
tests/runtime_shell/CMakeLists.txt
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
tests/runtime_shell/CMakeLists.txt
📚 Learning: 2025-09-04T07:28:37.083Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10832
File: src/wasm/CMakeLists.txt:112-131
Timestamp: 2025-09-04T07:28:37.083Z
Learning: In fluent-bit CMake files, the user cosmo0920 prefers treating Git as a command rather than a package, emphasizing that Git is not a pkg-config retrievable package but just a command.
Applied to files:
tests/runtime_shell/CMakeLists.txt
🧬 Code graph analysis (1)
tests/runtime_shell/go_plugins/test_logs_go.h (1)
tests/runtime_shell/go_plugins/logs_output.go (4)
FLBPluginRegister(12-15)FLBPluginInit(18-20)FLBPluginFlushCtx(23-39)FLBPluginExit(42-44)
🔇 Additional comments (7)
tests/runtime_shell/CMakeLists.txt (2)
18-18: LGTM!The addition of the proxy logs test to the test suite is appropriate.
31-32: LGTM!Adding the FLB_BUILD environment variable enables tests to reference the build directory, which is useful for locating built artifacts like the test plugin shared object.
tests/runtime_shell/conf/proxy_logs_test.conf (1)
1-21: LGTM!The test configuration is well-structured for validating the proxy logs functionality. The use of environment variables for the signal file path enables flexible test execution.
tests/runtime_shell/go_plugins/logs_output.go (2)
11-15: LGTM!The registration function correctly delegates to the fluent-bit-go output package with appropriate plugin name and description.
22-39: LGTM!The flush implementation correctly decodes records and outputs them to stdout for test verification. The error handling is appropriate for a test plugin.
tests/runtime_shell/go_plugins/go.mod (2)
3-3: The Go version 1.25.1 is valid and exists.The latest stable Go release is Go 1.25.3 (released October 13, 2025), and Go 1.25.1 was released prior to that. The code's specification of
go 1.25.1is legitimate, though not the latest version available. No changes are required.Likely an incorrect or invalid review comment.
6-6: Review comment is incorrect — PR 80 is open, not merged.PR 80 ("Enable metric output plugins") in fluent-bit-go is currently open as of October 10, 2025 and has not been merged. Additionally, the plugin code (
logs_output.go) does not use or requireevent_typesupport. The current dependency version is appropriate for the code's functionality.Likely an incorrect or invalid review comment.
cosmo0920
left a comment
cosmo0920
left a comment
There was a problem hiding this comment.
The added testing mechanism is really good. We need to clarify this mechanism on Linux only.
So, we need further modifications.
|
Plus, this PR still uses outdated base branch. So, could you rebase off the current master? |
This commit adds event_type to the flb_plugin_proxy_def struct. This allows for the setting of event_type to a value other than the default log only initialization. In the flb_plugin_proxy_register function default behaviour is enforced by checking for unset event_type vlaues. When unset (a value of 0) then log behaviour is defaulted. No need for incorrect value checking since this was not a set field in the past. Unset fields are set as current behavior dicates and future usecases using incorrect values can be treated as user error. Input use case does not use event-type and is unaffected by this change. Signed-off-by: jmccormick7 <jcm258@case.edu>
… malloc Changing intialization to use calloc instead of malloc so that emtpy fields are 0 initialized. Specifically for the event_type addition. Signed-off-by: jmccormick7 <jcm258@case.edu>
This commit adds a runtime shell test for the go plugin interface. A dummy go output plugin is added to the directories, and a test that builds the plugin shared object and then tests that it works and writes the output. This test ensures that the go proxy interface is working as expected. This was tested in the devcontainer using ctest. Signed-off-by: jmccormick7 <jcm258@case.edu>
Signed-off-by: jmccormick7 <jcm258@case.edu>
jmccormick7
force-pushed
the
enable-metric-proxy-plugins
branch
from
6800215 to
ba352c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/runtime_shell/conf/proxy_logs_test.conf (1)
1-21: Config looks correct for the intended runtime-shell proxy logs testSERVICE, dummy INPUT, and dual OUTPUT setup all look valid and aligned with the described test: three dummy records tagged
test.logsare routed both to the Go proxy output (test_logs_go) and to a file sink controlled via${SIGNAL_FILE_PATH}. Flush/Grace settings are reasonable for a fast test run, andmkdir onhelps avoid path issues.If you ever see test runtime bloat or want to minimize emitted records, you could optionally reduce
Samplesfrom3to1, but it’s not necessary given how small this test is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/runtime_shell/go_plugins/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
include/fluent-bit/flb_plugin_proxy.h(1 hunks)src/flb_plugin_proxy.c(3 hunks)tests/runtime_shell/CMakeLists.txt(2 hunks)tests/runtime_shell/conf/proxy_logs_test.conf(1 hunks)tests/runtime_shell/go_plugins/build_test_plugins.sh(1 hunks)tests/runtime_shell/go_plugins/go.mod(1 hunks)tests/runtime_shell/go_plugins/logs_output.go(1 hunks)tests/runtime_shell/go_plugins/test_logs_go.h(1 hunks)tests/runtime_shell/proxy_logs_expect.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- include/fluent-bit/flb_plugin_proxy.h
- tests/runtime_shell/CMakeLists.txt
- src/flb_plugin_proxy.c
- tests/runtime_shell/go_plugins/go.mod
- tests/runtime_shell/go_plugins/build_test_plugins.sh
- tests/runtime_shell/proxy_logs_expect.sh
- tests/runtime_shell/go_plugins/logs_output.go
- tests/runtime_shell/go_plugins/test_logs_go.h
This commit makes the go proxy a test that only runs on Linux using the CMAKE_SYSTEM_NAME. It also removes an autogenerated header file from the go compile that should not have been commited. Signed-off-by: jmccormick7 <jcm258@case.edu>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/runtime_shell/CMakeLists.txt(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
tests/runtime_shell/CMakeLists.txt
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
tests/runtime_shell/CMakeLists.txt
📚 Learning: 2025-09-04T07:28:37.083Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10832
File: src/wasm/CMakeLists.txt:112-131
Timestamp: 2025-09-04T07:28:37.083Z
Learning: In fluent-bit CMake files, the user cosmo0920 prefers treating Git as a command rather than a package, emphasizing that Git is not a pkg-config retrievable package but just a command.
Applied to files:
tests/runtime_shell/CMakeLists.txt
🔇 Additional comments (1)
tests/runtime_shell/CMakeLists.txt (1)
20-22: Good: Linux-only test conditional appropriately addresses prior feedback.The conditional properly gates
proxy_logs_expect.shto Linux systems, directly implementing the review feedback requesting platform-specific support. CMake syntax is correct.
| FLB_BIN=${CMAKE_BINARY_DIR}/bin/fluent-bit;\ | ||
| FLB_BUILD=${CMAKE_BINARY_DIR}" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify environment variable usage in dependent test scripts.
The FLB_BUILD variable is correctly added to the test environment. However, confirm that proxy_logs_expect.sh and any sourced test utilities (e.g., build scripts for Go plugins) properly reference this variable to locate build artifacts.
🏁 Script executed:
#!/bin/bash
# Description: Verify that FLB_BUILD environment variable is used by proxy_logs_expect.sh
# Check if proxy_logs_expect.sh references FLB_BUILD
rg -n 'FLB_BUILD' tests/runtime_shell/
# Expected: Multiple references in proxy_logs_expect.sh and related build scriptsLength of output: 68
🏁 Script executed:
#!/bin/bash
# Check how test scripts currently locate build artifacts and directories
echo "=== Checking for CMAKE_BINARY_DIR or path references in test scripts ==="
rg -n 'CMAKE_BINARY_DIR|FLB_BIN' tests/runtime_shell/ -A 2 -B 2
echo ""
echo "=== Checking proxy_logs_expect.sh for build directory usage ==="
cat tests/runtime_shell/proxy_logs_expect.sh | head -50Length of output: 14524
Remove unused FLB_BUILD environment variable or refactor test scripts to use it.
The FLB_BUILD variable is added to the test environment (CMakeLists.txt:35) but is never referenced by any test script. Scripts like proxy_logs_expect.sh instead hardcode paths like $FLB_ROOT/build/test_logs_go.so. Either remove the unused variable or update test scripts to use FLB_BUILD consistently instead of hardcoded paths.
🤖 Prompt for AI Agents
In tests/runtime_shell/CMakeLists.txt around lines 34-35, the FLB_BUILD
environment variable is being set but never used by test scripts; either remove
the FLB_BUILD assignment from this CMake file or update test scripts to
reference it instead of hardcoded paths (e.g., replace occurrences like
$FLB_ROOT/build/... with ${FLB_BUILD}/... or $FLB_BUILD/... depending on script
quoting), ensure the variable is exported to the test environment, update any
documentation/README about test env vars, and run the tests to confirm no path
regressions.
cosmo0920
left a comment
cosmo0920
left a comment
There was a problem hiding this comment.
Looks good. With the newly added unit test case for confirming backward compatibility, this PR ensures backward compatibility.
2 participants
This commit adds event_type to the flb_plugin_proxy_def struct. This allows for the setting of event_type to a value other than the default log only initialization.
In the flb_plugin_proxy_register function default behavior is enforced by checking for unset event_type values. When unset (a value of 0) then log behavior is defaulted. No need for incorrect value checking since this was not a set field in the past. Unset fields are set as current behavior dictates and future use cases using incorrect values can be treated as user error.
Input use case does not use event-type and is unaffected by this change.
Addresses #10995
This corresponding PR in the fluent-bit-go library enables the ability to allow Go plugins to register their event types PR 80
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
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.
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.