fix(torii): improve log messages #2463
Conversation
WalkthroughOhayo, sensei! The changes in this pull request focus on enhancing logging functionality across several files in the Changes
Possibly related PRs
Suggested reviewers
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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
|
![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
Outside diff range and nitpick comments (3)
crates/torii/core/src/engine.rs (3)
215-226: Ohayo, sensei! Consider adjusting log levels for timing info.The addition of timing measurements is excellent for performance monitoring. However, logging these durations at the
infolevel might be too verbose, especially if this operation occurs frequently.Consider changing these new timing logs to the
debuglevel:- info!(target: LOG_TARGET, duration = ?instant.elapsed(), from = %from, to = %latest_block_number, "Fetched data for range."); + debug!(target: LOG_TARGET, duration = ?instant.elapsed(), from = %from, to = %latest_block_number, "Fetched data for range."); - info!(target: LOG_TARGET, duration = ?instant.elapsed(), latest_block_number = %latest_block_number, "Fetched pending data."); + debug!(target: LOG_TARGET, duration = ?instant.elapsed(), latest_block_number = %latest_block_number, "Fetched pending data.");This change will reduce log verbosity while still allowing detailed timing information when needed.
Line range hint
174-192: Ohayo again, sensei! Let's fine-tune the logging for processing too.The addition of timing measurements for data processing is a great improvement. However, similar to the fetching logs, this timing information might be too verbose at the
infolevel.Consider changing this new timing log to the
debuglevel:- info!(target: LOG_TARGET, duration = ?instant.elapsed(), "Processed fetched data."); + debug!(target: LOG_TARGET, duration = ?instant.elapsed(), "Processed fetched data.");This adjustment will maintain consistency with the previous suggestion and help manage log verbosity.
Line range hint
1-738: Ohayo once more, sensei! Overall, these changes are sugoi!The introduction of timing measurements for key operations is a valuable addition to the codebase. It enhances observability without impacting core functionality, which is excellent.
For future enhancements, consider implementing a more structured approach to performance monitoring:
- Create a dedicated performance monitoring module that can be easily enabled/disabled.
- Use a configuration flag to control the verbosity of timing logs.
- Consider integrating with a metrics collection system (e.g., Prometheus) for more comprehensive performance tracking.
These suggestions could further improve the system's observability and make performance tuning easier in the long run.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- crates/torii/core/src/engine.rs (4 hunks)
- crates/torii/core/src/processors/store_del_record.rs (2 hunks)
- crates/torii/core/src/processors/store_set_record.rs (2 hunks)
- crates/torii/core/src/processors/store_update_member.rs (2 hunks)
- crates/torii/core/src/processors/store_update_record.rs (2 hunks)
Additional comments not posted (6)
crates/torii/core/src/processors/store_del_record.rs (2)
Line range hint
52-56: Ohayo, sensei! LGTM: Improved log level for better verbosity control.The change from
infotodebuglog level is a good improvement. It reduces log verbosity during normal operation while still providing useful information for debugging purposes. This aligns well with the PR objective to enhance log messages.
Line range hint
1-65: Ohayo again, sensei! Overall impact: Positive and low-risk change.The modification to the log level is an isolated change that doesn't affect the core functionality of the
StoreDelRecordProcessor. It improves the logging strategy by reducing unnecessary verbosity during normal operation. This change is low-risk and aligns well with the PR's objective to enhance log messages.crates/torii/core/src/processors/store_set_record.rs (1)
Line range hint
53-57: Ohayo, sensei! LGTM: Logging level adjustment approved.The change from
info!todebug!for the "Store set record" log message is a good improvement. This adjustment reduces log verbosity in production environments while still maintaining the ability to access detailed information when needed during debugging.This modification aligns well with the PR objective to enhance log messages and adjust logging levels. It doesn't affect the functionality of the code, only its logging behavior.
crates/torii/core/src/processors/store_update_record.rs (1)
Line range hint
55-59: Ohayo, sensei! Nice log level adjustment!The change from
info!todebug!macro is a good improvement. It reduces log verbosity in production while still allowing detailed logging when needed during debugging sessions.This change aligns well with the PR objective of improving log messages. It maintains the valuable information about store update records without cluttering the default log output.
crates/torii/core/src/processors/store_update_member.rs (1)
Line range hint
73-79: Ohayo, sensei! LGTM on the logging level change.The modification from
info!todebug!for the "Store update member" log is a good improvement. It reduces log verbosity while still maintaining valuable debugging information.To ensure consistency, let's verify if similar logging statements in related files have been updated:
Verification successful
All 'info!' Logs Successfully Updated
Ohayo, sensei! I’ve verified that there are no remaining
info!logs related to store updates in the codebase. The logging level change frominfo!todebug!has been successfully implemented.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining 'info!' logs related to store updates rg --type rust 'info!.*"Store update' crates/toriiLength of output: 52
crates/torii/core/src/engine.rs (1)
21-21: Ohayo, sensei! Nice import consolidation!The change consolidates the
sleepimport and addsInstantfor time measurements. This is a good practice for keeping imports organized and introducing necessary timing functionality.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2463 +/- ##
==========================================
+ Coverage 68.45% 68.47% +0.01%
==========================================
Files 368 368
Lines 48162 48164 +2
==========================================
+ Hits 32971 32979 +8
+ Misses 15191 15185 -6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@lambda-0x shouldn't be more debug or even trace log level for those?
commit-id:88796bb8
lambda-0x
force-pushed
the
spr/main/88796bb8
branch
from
880661d to
95ebf64
Compare
Summary by CodeRabbit
Bug Fixes
Chores
infotodebugin several components to reduce verbosity and focus on essential logs.