Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an execve eBPF trace: new BPF programs and maps, a userland handler and header to encode exec events, event struct changes, trace registration and tests, plus a minor config help-text update. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Kernel
participant BPF as "eBPF: exec/bpf.c"
participant Maps as "BPF Maps: values/heap/events"
participant Userspace as "in_ebpf userspace reader"
participant Handler as "exec handler (handler.c)"
participant Encoder as "flb_log_event_encoder / flb_input"
Kernel->>BPF: tracepoint sys_enter_execve(ctx)
BPF->>Maps: store staged args by tid (filename, argv, argc, mntns_id)
Kernel->>BPF: tracepoint sys_exit_execve(ctx)
BPF->>Maps: lookup staged args, build event (ppid, error_raw, comm...), submit to events map
Maps->>Userspace: userspace reads submitted event
Userspace->>Handler: trace_exec_handler(data, size)
Handler->>Encoder: encode_exec_event(ev) -> encoded buffer
Handler->>Userspace: flb_input_log_append(encoded buffer)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da45240ff6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/in_ebpf/in_ebpf.c`:
- Line 265: Update the help text so it advertises the actual registration key
used by trace_setup: replace or augment the "exec" entry with the registered
name "trace_exec" (e.g., "exec (trace_exec)") so users can pass the correct key;
locate the help string near the trace option and ensure it matches the
registration name used when registering trace_exec in the plugin code.
In `@plugins/in_ebpf/traces/exec/bpf.c`:
- Around line 21-26: The struct execve_args currently stores raw user pointers
(args.filename and args.argv) from sys_enter_execve which are later dereferenced
in the exit hook after an exec may have replaced the caller's address space; to
fix this, read and copy the filename and each argv string into kernel-space
buffers on entry (e.g., using bpf_probe_read_str or equivalent) and store those
copied strings in the map value instead of the raw pointers, update the
execve_args/map value layout to include fixed-size char arrays or pointers to
allocated buffers, and adjust all locations that currently reference
args.filename/args.argv (including the exit hook and the areas around lines
68-84 and 131-143) to use the copied strings from the map value rather than
user-space pointers.
In `@plugins/in_ebpf/traces/exec/handler.c`:
- Around line 106-112: The current code returns immediately on
flb_input_log_append failure and leaves the committed data in encoder; update
the failure path in the block around flb_input_log_append (the call using
event_ctx->ins and encoder->output_buffer/encoder->output_length) to call
flb_log_event_encoder_reset(encoder) before returning so the encoder is cleared
on both success and error paths, preventing stale bytes from being re-emitted or
corrupting the next record.
In `@tests/runtime/in_ebpf_exec_handler.c`:
- Around line 86-122: Update the verify_decoded_values function to track and
assert presence of all required exec fields: add boolean seen_pid, seen_ppid,
seen_filename, seen_argc, seen_argv, seen_error_raw (set true when each key is
encountered) and after the loop assert each seen_* is true; also validate argv
contents (compare each argument string and count against
original->details.execve.argc). Additionally add a negative-path unit test for
trace_exec_handler that calls it with an incorrect event_type and a truncated
payload and asserts it fails/returns the expected error, ensuring missing fields
are detected. Use the functions/structures present in the diff
(verify_decoded_values and trace_exec_handler, original->details.execve) to
locate where to add the checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88223759-c29d-4031-ba07-1c24983e7897
📒 Files selected for processing (8)
plugins/in_ebpf/in_ebpf.cplugins/in_ebpf/traces/exec/bpf.cplugins/in_ebpf/traces/exec/handler.cplugins/in_ebpf/traces/exec/handler.hplugins/in_ebpf/traces/includes/common/events.hplugins/in_ebpf/traces/traces.htests/runtime/CMakeLists.txttests/runtime/in_ebpf_exec_handler.c
cosmo0920
force-pushed
the
cosmo0920-implement-exec-trace
branch
from
da45240 to
8d6c3ba
Compare
cosmo0920
force-pushed
the
cosmo0920-implement-exec-trace
branch
from
a4e6edb to
2524dbc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/in_ebpf/traces/exec/handler.c`:
- Around line 82-85: When flb_log_event_encoder_commit_record(log_encoder) fails
in exec/handler.c (and the other trace handlers vfs, tcp, signal, malloc, bind),
roll back the encoder state before returning error to avoid leaving a dirty
encoder; specifically call flb_log_event_encoder_rollback_record(log_encoder)
immediately on any non-success return from flb_log_event_encoder_commit_record()
(mirroring the existing rollback behavior on append/encode failures) and then
return -1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2cfd281c-eb9f-4bd3-a4a2-5c8883f9473a
📒 Files selected for processing (1)
plugins/in_ebpf/traces/exec/handler.c
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/runtime/in_ebpf_exec_handler.c`:
- Around line 114-127: The filename/argv checks can pass on truncated values
because they only compare kv->val.via.str.size bytes; update the checks in the
blocks that handle key_matches(kv->key, "filename") and key_matches(kv->key,
"argv") to first assert kv->val.type == MSGPACK_OBJECT_STR, then assert
kv->val.via.str.size == strlen(original->details.execve.filename) (for
"filename") or == strlen(original->details.execve.argv) (for "argv"), and only
after verifying equal lengths perform the content comparison (memcmp/strncmp)
using kv->val.via.str.size; keep seen_filename and seen_argv handling the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec715826-cd72-4e8b-8474-faf81de49cee
📒 Files selected for processing (8)
plugins/in_ebpf/in_ebpf.cplugins/in_ebpf/traces/exec/bpf.cplugins/in_ebpf/traces/exec/handler.cplugins/in_ebpf/traces/exec/handler.hplugins/in_ebpf/traces/includes/common/events.hplugins/in_ebpf/traces/traces.htests/runtime/CMakeLists.txttests/runtime/in_ebpf_exec_handler.c
✅ Files skipped from review due to trivial changes (2)
- plugins/in_ebpf/in_ebpf.c
- plugins/in_ebpf/traces/exec/handler.h
🚧 Files skipped from review as they are similar to previous changes (4)
- plugins/in_ebpf/traces/includes/common/events.h
- tests/runtime/CMakeLists.txt
- plugins/in_ebpf/traces/traces.h
- plugins/in_ebpf/traces/exec/handler.c
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/in_ebpf/traces/exec/handler.c (1)
21-80: Consider consolidating append+rollback boilerplate.The repeated
retchecks with identical rollback/return branches are easy to drift. A tiny helper/macro would make this safer to maintain.Refactor sketch
+static inline int append_or_rollback(int rc, struct flb_log_event_encoder *enc) +{ + if (rc != FLB_EVENT_ENCODER_SUCCESS) { + flb_log_event_encoder_rollback_record(enc); + return -1; + } + return 0; +} + int encode_exec_event(struct flb_input_instance *ins, struct flb_log_event_encoder *log_encoder, const struct event *ev) { int ret; @@ - ret = flb_log_event_encoder_append_body_cstring(log_encoder, "ppid"); - if (ret != FLB_EVENT_ENCODER_SUCCESS) { - flb_log_event_encoder_rollback_record(log_encoder); - return -1; - } + ret = append_or_rollback( + flb_log_event_encoder_append_body_cstring(log_encoder, "ppid"), + log_encoder); + if (ret != 0) { + return -1; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/in_ebpf/traces/exec/handler.c` around lines 21 - 80, The code repeats the same pattern of calling flb_log_event_encoder_* functions, checking ret, calling flb_log_event_encoder_rollback_record(log_encoder) and returning -1; introduce a small helper (static inline function or macro) to encapsulate this pattern (e.g., SAFE_ENCODE(call) or safe_encode(log_encoder, (call))) and replace each pair of calls in handler.c (including uses around encode_common_fields, flb_log_event_encoder_append_body_cstring, flb_log_event_encoder_append_body_uint32, flb_log_event_encoder_append_body_int32, etc.) with that helper so the call, error-check, rollback and return -1 are handled centrally to remove boilerplate and avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/in_ebpf/traces/exec/handler.c`:
- Around line 21-80: The code repeats the same pattern of calling
flb_log_event_encoder_* functions, checking ret, calling
flb_log_event_encoder_rollback_record(log_encoder) and returning -1; introduce a
small helper (static inline function or macro) to encapsulate this pattern
(e.g., SAFE_ENCODE(call) or safe_encode(log_encoder, (call))) and replace each
pair of calls in handler.c (including uses around encode_common_fields,
flb_log_event_encoder_append_body_cstring,
flb_log_event_encoder_append_body_uint32,
flb_log_event_encoder_append_body_int32, etc.) with that helper so the call,
error-check, rollback and return -1 are handled centrally to remove boilerplate
and avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d773a0be-0bb6-4036-9295-b6ab4c25d272
📒 Files selected for processing (1)
plugins/in_ebpf/traces/exec/handler.c
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
cosmo0920
force-pushed
the
cosmo0920-implement-exec-trace
branch
from
580ce47 to
17fd5bc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements exec/execve trace on in_ebpf plugin.
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
New Features
Documentation
Tests