chore(zetaclient): improve logging by adding and moving data to attributes #2875
Conversation
gartnera
requested review from
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
WalkthroughWalkthroughThe changes in this pull request focus on enhancing the logging functionality of the Changes
Possibly related PRs
Tip OpenAI O1 model for chat
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
Documentation and Community
|
gartnera
force-pushed
the
improve-zetaclient-logging
branch
from
cc78c1e to
ce119a1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2875 +/- ##
===========================================
- Coverage 66.94% 66.87% -0.07%
===========================================
Files 373 373
Lines 21032 21054 +22
===========================================
Hits 14080 14080
- Misses 6288 6310 +22
Partials 664 664
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
zetaclient/chains/bitcoin/observer/observer.go (1)
243-243: Improve error messages and logging.Consider enhancing the error messages and logging in the
WatchRPCStatusfunction:
- Include the chain ID in the error messages and success log to provide more context.
- For failed RPC calls, include the specific RPC method that failed in the error message.
For example:
ob.logger.Chain.Error().Err(err). Int64("chain_id", ob.Chain().ChainId). Msg("RPC status check failed: GetBlockCount")Additionally, consider logging the TSS address in the error message when no UTXOs are found for better debugging.
Also applies to: 249-251, 257-260, 265-271
Tools
GitHub Check: codecov/patch
[warning] 243-243: zetaclient/chains/bitcoin/observer/observer.go#L243
Added line #L243 was not covered by tests
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- zetaclient/chains/bitcoin/observer/observer.go (2 hunks)
- zetaclient/chains/evm/observer/observer.go (1 hunks)
Additional context used
Path-based instructions (2)
zetaclient/chains/evm/observer/observer.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/observer.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
GitHub Check: codecov/patch
zetaclient/chains/evm/observer/observer.go
[warning] 220-220: zetaclient/chains/evm/observer/observer.go#L220
Added line #L220 was not covered by tests
[warning] 225-227: zetaclient/chains/evm/observer/observer.go#L225-L227
Added lines #L225 - L227 were not covered by tests
[warning] 232-234: zetaclient/chains/evm/observer/observer.go#L232-L234
Added lines #L232 - L234 were not covered by tests
[warning] 239-240: zetaclient/chains/evm/observer/observer.go#L239-L240
Added lines #L239 - L240 were not covered by tests
[warning] 242-242: zetaclient/chains/evm/observer/observer.go#L242
Added line #L242 was not covered by tests
[warning] 246-250: zetaclient/chains/evm/observer/observer.go#L246-L250
Added lines #L246 - L250 were not covered by testszetaclient/chains/bitcoin/observer/observer.go
[warning] 243-243: zetaclient/chains/bitcoin/observer/observer.go#L243
Added line #L243 was not covered by tests
[warning] 249-251: zetaclient/chains/bitcoin/observer/observer.go#L249-L251
Added lines #L249 - L251 were not covered by tests
[warning] 257-260: zetaclient/chains/bitcoin/observer/observer.go#L257-L260
Added lines #L257 - L260 were not covered by tests
[warning] 265-271: zetaclient/chains/bitcoin/observer/observer.go#L265-L271
Added lines #L265 - L271 were not covered by tests
[warning] 292-297: zetaclient/chains/bitcoin/observer/observer.go#L292-L297
Added lines #L292 - L297 were not covered by tests
| @@ -240,26 +240,35 @@ | |||
|
|
|||
| bn, err := ob.btcClient.GetBlockCount() | |||
| if err != nil { | |||
| ob.logger.Chain.Error().Err(err).Msg("RPC status check: RPC down? ") | |||
| ob.logger.Chain.Error().Err(err).Msg("RPC status check failed: GetBlockCount") | |||
There was a problem hiding this comment.
Add unit tests for WatchRPCStatus.
The static analysis hints indicate that the added lines in the WatchRPCStatus function are not covered by tests. To ensure the correctness of the RPC status checks and error handling, consider adding unit tests for this function.
Some test cases to consider:
- Test successful RPC status check with valid block and TSS address details.
- Test error handling for each failed RPC call (
GetBlockCount,GetBlockHash,GetBlockHeader,ListUnspentMinMaxAddresses). - Test error handling for old blocks and missing TSS address UTXOs.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Also applies to: 249-251, 257-260, 265-271, 292-297
Tools
GitHub Check: codecov/patch
[warning] 243-243: zetaclient/chains/bitcoin/observer/observer.go#L243
Added line #L243 was not covered by tests
|
superseded by #2751 which does at least add more descriptive error messages. |
We see a lot of "RPC Status Check error: RPC down?" with error "not found".
block_numberlog attribute whenever possibleSummary by CodeRabbit
New Features
Bug Fixes