Skip to content

Comments

fix issue with deduping#28

Merged
YakirOren merged 2 commits intomainfrom
fix/procfs-read-dedupe
Feb 5, 2026
Merged

fix issue with deduping#28
YakirOren merged 2 commits intomainfrom
fix/procfs-read-dedupe

Conversation

@YakirOren
Copy link
Contributor

@YakirOren YakirOren commented Feb 4, 2026

Summary by CodeRabbit

  • Updates

    • Changed the rule's uniqueness identifier to use the process name only (removed the file path), which may cause events from the same process reading multiple environment files to be treated as the same occurrence, potentially increasing collisions or deduplication.
  • Tests

    • Updated test cases to parameterize process name and align expected unique identifiers with the new uniqueness behavior.

@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

A rule's uniqueness expression was changed to use only event.comm instead of concatenating event.comm and event.path, altering how rule matches are deduplicated. Tests were updated to provide a processName and to expect the new UniqueID format.

Changes

Cohort / File(s) Summary
Rule Configuration
pkg/rules/r0008-read-environment-variables-procfs/read-environment-variables-procfs.yaml
Changed spec.rules[0].expressions[0].uniqueId from event.comm + '_' + event.path to event.comm, removing the path portion from the uniqueness key.
Tests
pkg/rules/r0008-read-environment-variables-procfs/rule_test.go
Added processName parameter to test event factory and test cases; updated events to set Comm from processName; adjusted expected UniqueID values to use only Comm.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through lines where changes dwell,
Trimmed a path from a uniqueness spell.
Comm alone now marks the clue,
Tests updated — a tidy view.
The Code Rabbit 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and generic, using 'issue' and 'deduping' without specifying what the actual problem or solution is, leaving reviewers uncertain about the specific technical change. Use a more descriptive title that specifies the deduplication fix, e.g., 'Use process name only for environment variable rule deduplication' or 'Fix procfs environment variable rule uniqueId calculation'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/procfs-read-dedupe

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.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@pkg/rules/r0008-read-environment-variables-procfs/read-environment-variables-procfs.yaml`:
- Around line 15-16: The current dedup key uses only event.comm which can
collapse alerts; change the rule's uniqueId to include event.comm plus
event.path and/or event.containerId (e.g., combine event.comm + '|' + event.path
+ '|' + event.containerId) so alerts are deduped per process+target-file and
container context; update the uniqueId field and adjust any dependent message
formatting to reference the same fields (message, uniqueId, event.comm,
event.path, event.containerId).

Signed-off-by: Yakir Oren <yakiroren@gmail.com>
@YakirOren YakirOren force-pushed the fix/procfs-read-dedupe branch from f740e42 to f85acd9 Compare February 4, 2026 15:58
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
@YakirOren YakirOren merged commit 9882e5b into main Feb 5, 2026
4 checks passed
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.

3 participants