ROSAENG-64957: Reset ocm_agent_response_failure gauge on alert resolution and resend window - #265
Conversation
…tion and resend window The fleet-mode RHOBS webhook handler sets the ocm_agent_response_failure gauge to 1 on service log send failure but never resets it to 0 in two code paths, causing persistent false OCMAgentResponseFailureServiceLogsSRE alerts until the pod is restarted. Add gauge resets for: - Resolved non-limited-support alerts (early return path) - Firing alerts within the resend window (!canSend path) This matches the existing behavior in the classic handler (webhookreceiver.go). Co-authored-by: Cursor <cursoragent@cursor.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@aliceh: This pull request references ROSAENG-64957 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Warning Review limit reached
Next review available in: 14 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe webhook handler now resets service-specific response failure metrics when non-limited-support alerts resolve or when firing alerts cannot be sent. ChangesResponse metric reset behavior
Estimated code review effort: 1 (Trivial) | ~5 minutes Mergeability Score: 🔵 Low · up to The PR correctly clears the failure gauge in the two identified fleet-handler paths, but a support-mode change during an active alert could leave the earlier labeled gauge set. The change is mergeable with explicit owner awareness or follow-up to confirm or cover that transition behavior. Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/handlers/webhookrhobsreceiver.go (1)
395-395: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for both reset paths.
Extend
pkg/handlers/webhookrhobsreceiver_test.gowith tests for a resolved non-limited-support alert and a firing alert wherecanSendis false. Assert thatMetricResponseFailureresets the expectedocm_service,notification_name, andalert_namelabels. Include a support-mode transition case ifLimitedSupportis mutable during an alert lifecycle.Also applies to: 481-488
🤖 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 `@pkg/handlers/webhookrhobsreceiver.go` at line 395, Add regression tests in webhookrhobsreceiver_test.go covering both MetricResponseFailure reset paths: a resolved non-limited-support alert and a firing alert with canSend false. Assert resets use the expected ocm_service, notification_name, and alert_name labels, and cover a LimitedSupport transition during the alert lifecycle if that field is mutable.
🤖 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.
Nitpick comments:
In `@pkg/handlers/webhookrhobsreceiver.go`:
- Line 395: Add regression tests in webhookrhobsreceiver_test.go covering both
MetricResponseFailure reset paths: a resolved non-limited-support alert and a
firing alert with canSend false. Assert resets use the expected ocm_service,
notification_name, and alert_name labels, and cover a LimitedSupport transition
during the alert lifecycle if that field is mutable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 81cc3921-6a14-4e9f-945d-ad5ce0cc4e2a
📒 Files selected for processing (1)
pkg/handlers/webhookrhobsreceiver.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #265 +/- ##
==========================================
+ Coverage 56.28% 56.46% +0.18%
==========================================
Files 23 23
Lines 1926 1934 +8
==========================================
+ Hits 1084 1092 +8
Misses 786 786
Partials 56 56
🚀 New features to boost your workflow:
|
The CI lint job exceeds the 5m timeout on cold cache runs. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@aliceh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aliceh, chamalabey The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
6 participants
Summary
ocm_agent_response_failuregauge in the fleet RHOBS webhook handler (webhookrhobsreceiver.go) which never resets to 0 after a transient service-log send failure, causing persistent falseOCMAgentResponseFailureServiceLogsSREalerts until the pod is restarted.webhookreceiver.go.Details
The fleet handler sets
ocm_agent_response_failureto 1 on failure but omits the reset in two paths:!fleetNotification.LimitedSupport && !isCurrentlyFiring → return nil) without clearing the gauge. This change addsmetrics.ResetResponseMetricFailure()before the early return.elsebranch of thecanSendcheck.Jira: https://issues.redhat.com/browse/ROSAENG-64957
Test plan
Made with Cursor
Summary by CodeRabbit