Skip to content

chore(bpf): cleanup usages of helper_t buffers - #1818

Open
Molter73 wants to merge 1 commit into
mainfrom
mauro/cleanup/helper-usage
Open

Molter73 wants to merge 1 commit into
mainfrom
mauro/cleanup/helper-usage

Conversation

@Molter73

@Molter73 Molter73 commented Sep 17, 2026

Copy link
Copy Markdown
Member

Description

Remove some unused instances of helper_t variables and update a leftover bpf_map_lookup_elem to get_helper.

Requested in #1569 (review)

Checklist

  • Patch has a change log entry OR does not need one.
  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

Changes are simple enough CI should be enough validation.

Summary by CodeRabbit

  • Refactor
    • Simplified internal event processing and helper retrieval.
    • Existing process-fill error handling remains unchanged.

Remove some unused instances of helper_t variables and update a leftover
`bpf_map_lookup_elem` to `get_helper`.

Requested in #1569 (review)
@Molter73
Molter73 requested a review from a team as a code owner September 17, 2026 15:27
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: 6d77a28f-1060-4e83-8d8c-fa6f67362cfd

📥 Commits

Reviewing files that changed from the base of the PR and between c2d0c40 and 69dcfe4.

📒 Files selected for processing (2)
  • fact-ebpf/src/bpf/events.h
  • fact-ebpf/src/bpf/process.h
💤 Files with no reviewable changes (1)
  • fact-ebpf/src/bpf/events.h

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The helper lookup moved from event submission into process_fill. process_fill_lineage no longer accepts a helper argument. Event submission now calls process_fill directly, while existing process-fill error handling remains.

Changes

Helper Lookup Flow

Layer / File(s) Summary
Process-fill helper lookup
fact-ebpf/src/bpf/process.h
process_fill now calls get_helper(). The obsolete map key was removed. process_fill_lineage and its call site no longer use a helper argument.
Direct event submission
fact-ebpf/src/bpf/events.h
__submit_event now calls process_fill directly. The separate helper lookup and null-check error path were removed.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Suggested reviewers: erthalion

Merge Risk: ⚪ Minimal · up to 69dcf

The cleanup preserves event processing behavior, including discarding events when the helper lookup fails.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: cleanup of unused helper_t buffer usages and related helper lookup updates.
Description check ✅ Passed The description includes the required Description, Checklist, Automated testing, and Testing Performed sections. It explains the change, records CI validation, and states why no additional tests were …
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33.40%. Comparing base (c2d0c40) to head (69dcfe4).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1818   +/-   ##
=======================================
  Coverage   33.40%   33.40%           
=======================================
  Files          22       22           
  Lines        3628     3628           
  Branches     3628     3628           
=======================================
  Hits         1212     1212           
  Misses       2407     2407           
  Partials        9        9           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants