refactor(metrics): reduce per-hook boilerplate with declarative macros#806
refactor(metrics): reduce per-hook boilerplate with declarative macros#806Molter73 wants to merge 1 commit into
Conversation
Use macro_rules! to generate repetitive per-hook code from a single list of hook names, reducing the number of places that must be updated when adding a new LSM hook. - fact-ebpf/src/lib.rs: impl_metrics_t! macro generates metrics_t::accumulate() from the hook list. - fact/src/metrics/kernel_metrics.rs: define_kernel_metrics! macro generates the KernelMetrics struct, constructor, registration, and collect() method. Also refactors refresh_labels to iterate over (label, value) tuples instead of 5 separate blocks. - fact/src/metrics/mod.rs: Extract inc_label/inc_label_by helpers in EventCounter to deduplicate the label lookup pattern. Widen visibility of counter field and MetricEvents to pub(crate). - fact-ebpf/src/bpf/types.h: Reorder metrics_t fields to group all path_* hooks together. Assisted-by: claude-opus-4-6@default <noreply@opencode.ai>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
==========================================
+ Coverage 27.96% 28.72% +0.76%
==========================================
Files 21 21
Lines 2596 2527 -69
Branches 2596 2527 -69
==========================================
Hits 726 726
+ Misses 1867 1798 -69
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fact-ebpf/src/lib.rs`:
- Around line 149-158: The impl_metrics_t! macro invocation has two fields in
the wrong order compared to the C metrics_t struct definition in
fact-ebpf/src/bpf/types.h. The fields d_instantiate and path_rmdir are currently
swapped at the end of the macro argument list. Reorder the fields in the
impl_metrics_t! macro invocation to match the C struct field order exactly:
file_open, path_unlink, path_chmod, path_chown, path_rename, path_mkdir,
d_instantiate, path_rmdir. This ensures correct metric accumulation between Rust
and C code.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 8421f968-219e-4e3c-a235-06d0f88ba51a
📒 Files selected for processing (3)
fact-ebpf/src/lib.rsfact/src/metrics/kernel_metrics.rsfact/src/metrics/mod.rs
| impl_metrics_t!( | ||
| file_open, | ||
| path_unlink, | ||
| path_chmod, | ||
| path_chown, | ||
| path_rename, | ||
| path_mkdir, | ||
| path_rmdir, | ||
| d_instantiate, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check that metrics_t fields match the macro invocation list
# First, find the metrics_t definition in the generated bindings or C header
rg -A 20 "struct metrics_t" fact-ebpf/src/bpf/types.h
# Also verify the hook lists match between lib.rs and kernel_metrics.rs
echo "=== lib.rs hook list ==="
rg -A 10 "impl_metrics_t!" fact-ebpf/src/lib.rs
echo "=== kernel_metrics.rs hook list ==="
rg -A 10 "define_kernel_metrics!" fact/src/metrics/kernel_metrics.rsRepository: stackrox/fact
Length of output: 768
Reorder fields in the macro invocation to match the C metrics_t struct definition.
All fields are present, but their order does not match the C struct. The C struct defines (in order): file_open, path_unlink, path_chmod, path_chown, path_rename, path_mkdir, d_instantiate, path_rmdir. The Rust macro has path_rmdir and d_instantiate swapped at the end. Update lines 149-158 to match the exact field order from the C definition to ensure correct metric accumulation.
C struct field order (fact-ebpf/src/bpf/types.h)
struct metrics_t {
struct metrics_by_hook_t file_open;
struct metrics_by_hook_t path_unlink;
struct metrics_by_hook_t path_chmod;
struct metrics_by_hook_t path_chown;
struct metrics_by_hook_t path_rename;
struct metrics_by_hook_t path_mkdir;
struct metrics_by_hook_t d_instantiate;
struct metrics_by_hook_t path_rmdir;
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@fact-ebpf/src/lib.rs` around lines 149 - 158, The impl_metrics_t! macro
invocation has two fields in the wrong order compared to the C metrics_t struct
definition in fact-ebpf/src/bpf/types.h. The fields d_instantiate and path_rmdir
are currently swapped at the end of the macro argument list. Reorder the fields
in the impl_metrics_t! macro invocation to match the C struct field order
exactly: file_open, path_unlink, path_chmod, path_chown, path_rename,
path_mkdir, d_instantiate, path_rmdir. This ensures correct metric accumulation
between Rust and C code.
Source: Coding guidelines
2 participants
Description
Use macro_rules! to generate repetitive per-hook code from a single list of hook names, reducing the number of places that must be updated when adding a new LSM hook.
Assisted-by: claude-opus-4-6@default noreply@opencode.ai
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
Run locally and checked metrics were increasing.
Summary by CodeRabbit