Skip to content

Conversation

VladimirFilonov
Copy link
Contributor

Closes #5115

πŸ“‘ Description

βœ… Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

β„Ή Additional Information

@vercel
Copy link

vercel bot commented Jul 1, 2025

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
keep ⬜️ Ignored (Inspect) Visit Preview Jul 1, 2025 0:03am

@VladimirFilonov VladimirFilonov marked this pull request as ready for review July 1, 2025 09:07
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jul 1, 2025
@cursor
Copy link

cursor bot commented Jul 1, 2025

🚨 BugBot couldn't run

BugBot is experiencing high demand right now. Try again in a few minutes by commenting "bugbot run" (requestId: serverGenReqId_aee548f4-b3d0-4686-be70-42dc65d7690b).

@dosubot dosubot bot added the Bug Something isn't working label Jul 1, 2025
@VladimirFilonov VladimirFilonov force-pushed the fix/5115-bug-correlation-rule-threshold-counts-already-resolved-alerts branch from 863ab7b to f020533 Compare July 1, 2025 09:07
@cursor
Copy link

cursor bot commented Jul 1, 2025

🚨 BugBot couldn't run

BugBot is experiencing high demand right now. Try again in a few minutes by commenting "bugbot run" (requestId: serverGenReqId_d340d18c-ff81-495e-8bb0-8bf0125e33a0).

Kiryous
Kiryous previously approved these changes Jul 1, 2025
Copy link
Contributor

@Kiryous Kiryous left a comment

Choose a reason for hiding this comment

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

LGTM, apart from minor style issue

@Kiryous
Copy link
Contributor

Kiryous commented Jul 1, 2025

@greptileai

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Implements a new unresolvedCounter system to fix correlation rule thresholds counting resolved alerts, ensuring incidents only trigger based on currently active alerts.

  • Added unresolvedCounter field to AlertDto model in keep/api/models/alert.py to track unresolved alerts separately from total firingCounter
  • Modified keep/rulesengine/rulesengine.py to use unresolvedCounter instead of firingCounter when evaluating incident thresholds
  • Added calculated_unresolved_counter helper in keep/api/utils/enrichment_helpers.py that resets to 0 when alerts are resolved
  • Added comprehensive test coverage in tests/test_counting_integration.py and tests/test_rules_engine.py validating threshold behavior with grouped and same-fingerprint alerts

8 files reviewed, 7 comments
Edit PR Review Bot Settings | Greptile

cursor[bot]

This comment was marked as outdated.

This commit introduces a new test to verify incident visibility and alert grouping behavior when thresholds are applied. It ensures incidents start automatically based on alert thresholds, validating grouping functionality and enrichment logic.
Introduce `unresolvedCounter` to track the number of unresolved alerts before resolution. Updated alert processing logic, added helper functions, and extended test coverage to ensure the counter increments/decrements as expected in various scenarios.
Updated a docstring to clarify the calculation of the unresolved counter in `enrichment_helpers.py`. Also adjusted import formatting in `process_event_task.py` for better readability.
Corrected the unresolved counter calculation and adjusted test cases to ensure accurate validation of resolved alert statuses. Updated comments and assertions to align with the clarified counter behavior.
@VladimirFilonov VladimirFilonov force-pushed the fix/5115-bug-correlation-rule-threshold-counts-already-resolved-alerts branch from f20d388 to a83758c Compare July 1, 2025 12:03
@VladimirFilonov VladimirFilonov requested a review from Kiryous July 1, 2025 12:03
Copy link
Contributor

@Kiryous Kiryous left a comment

Choose a reason for hiding this comment

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

LGTM

@VladimirFilonov VladimirFilonov merged commit dbfbe5a into main Jul 1, 2025
18 checks passed
@VladimirFilonov VladimirFilonov deleted the fix/5115-bug-correlation-rule-threshold-counts-already-resolved-alerts branch July 1, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[πŸ› Bug]: Correlation rule Threshold counts already resolved alerts

2 participants