[Health] Reduce alert spamming #2350
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new mechanism for handling delisted symbols across multiple packages. A new Changes
Sequence DiagramsequenceDiagram
participant Checker
participant DAL
participant Event
Checker->>Checker: Define SymbolsToBeDelisted
DAL->>Checker: Check if symbol is delisted
Event->>Checker: Check if symbol is delisted
alt Symbol is delisted
DAL-->>DAL: Skip processing
Event-->>Event: Skip processing
end
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
node/pkg/checker/dal/app.go (1)
Line range hint
37-175: Consider adding logging for filtered symbols.While the filtering logic is correct, adding debug logging when symbols are filtered would help with monitoring and troubleshooting.
if slices.Contains(checker.SymbolsToBeDelisted, symbol) { + log.Debug().Str("symbol", symbol).Msg("Skipping processing for delisted symbol") continue }node/pkg/checker/event/app.go (1)
58-60: Consider adding logging for filtered symbols.While the filtering logic is correct, adding debug logging when symbols are filtered would help with monitoring and troubleshooting.
if slices.Contains(checker.SymbolsToBeDelisted, config.Name) { + log.Debug().Str("symbol", config.Name).Msg("Skipping feed setup for delisted symbol") continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
node/pkg/checker/dal/app.go(3 hunks)node/pkg/checker/delisted.go(1 hunks)node/pkg/checker/event/app.go(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- node/pkg/checker/delisted.go
🔇 Additional comments (6)
node/pkg/checker/dal/app.go (3)
9-9: LGTM! Required imports added.The new imports support the symbol filtering functionality.
Also applies to: 15-15
37-39: LGTM! Filtering logic for delisted symbols.Early continue for delisted symbols effectively reduces alert spamming by preventing unnecessary processing.
173-175: LGTM! Consistent filtering implementation.The filtering logic is consistently applied across different processing paths, maintaining uniform behavior for delisted symbols.
node/pkg/checker/event/app.go (3)
7-7: LGTM! Required imports added.The new imports support the symbol filtering functionality.
Also applies to: 12-12
58-60: LGTM! Early filtering of delisted symbols.Prevents delisted symbols from being included in the FeedsToCheck list, effectively reducing unnecessary processing and alerts.
Line range hint
1-350: Verify the impact on existing alerts.The changes effectively filter out delisted symbols, but we should verify that other critical alerts remain functional.
✅ Verification successful
Alert functionality remains intact and properly handles symbol filtering
The changes correctly filter out delisted symbols during setup while preserving all critical alert functionality:
- Feed submission delay alerts continue to work for all active symbols
- POR (Proof of Reserve) alerts are unaffected as they use separate configuration
- VRF (Verifiable Random Function) alerts are unaffected as they use separate configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any alert-related code that might be affected rg -A 3 "SlackAlert\(" --type goLength of output: 3721
Description
Remove alert for the symbol to be delisted
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment