-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Logging: use terminal format with full hashes #1770
Conversation
WalkthroughThe update introduces a new custom log formatting functionality, Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- go/common/log/log.go (1 hunks)
- go/common/log/logformat.go (1 hunks)
- integration/common/testlog/testlog.go (1 hunks)
- tools/faucet/faucet/faucet.go (2 hunks)
- tools/walletextension/common/common.go (2 hunks)
Additional comments: 8
integration/common/testlog/testlog.go (1)
- 48-48: The change from
gethlog.LogfmtFormat()
tolog.TenLogFormat()
in theSetup
function aligns with the PR's objective to enhance log readability by adopting a terminal format that includes full transaction hashes. This modification ensures that logs are now more informative and user-friendly, particularly for tracking transactions on blockchain explorers.go/common/log/log.go (2)
- 60-60: The replacement of
gethlog.TerminalFormat
withTenLogFormat
for theStreamHandler
in theNew
function is a crucial part of implementing the custom "Ten log Format." This change ensures that logs output tostdout
will use the new format, enhancing readability and information richness.- 62-62: The modification to use
TenLogFormat
in theFileHandler
within theNew
function is consistent with the PR's goal to improve log formatting across the project. This ensures that logs written to files will also benefit from the enhanced format, making them more useful for debugging and analysis.tools/walletextension/common/common.go (2)
- 11-11: The addition of the
log
package fromgithub.com/ten-protocol/go-ten/go/common
is necessary to access theTenLogFormat
function. This change is part of the broader effort to standardize log formatting across the project.- 96-96: Replacing
gethlog.TerminalFormat
withlog.TenLogFormat
in theNewFileLogger
function ensures that the wallet extension's logs will now use the custom "Ten log Format." This modification aligns with the PR's objectives to enhance log readability and utility.tools/faucet/faucet/faucet.go (2)
- 16-16: The import of the
tenlog
package is essential for utilizing theTenLogFormat
function within the faucet tool. This change is part of the effort to adopt the new log format across different components of the project.- 41-41: Initializing the logger with
tenlog.TenLogFormat()
in theNewFaucet
function ensures that the faucet's logs will adhere to the new, more informative and readable log format. This is in line with the PR's goal to improve the logging experience.go/common/log/logformat.go (1)
- 1-324: The introduction of the
logformat.go
file and theTenLogFormat
function represents a significant enhancement to the project's logging system. This custom log format is designed to replicate the Geth terminal format's appearance and functionality, including full transaction hashes, which improves log readability and utility. The implementation details, such as the handling of various data types and the alignment of log messages, are well thought out and align with the PR's objectives. However, it's important to ensure that this new format is thoroughly tested, especially in edge cases where log messages might contain unusual characters or extremely long values that could affect the formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- go/common/log/logformat.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- go/common/log/logformat.go
Why this change is needed
In recent PR I tried to fix the shortened hashes in our logs, to make them searchable in etherscan etc. But actually what I did had no effect on testnet logs and put our sim logs into a different, machine readable format.
We wanted to use the terminal format everywhere but with full hashes.
What changes were made as part of this PR
In this screenshot the new format is above and the old format below. You can see the
L1 transaction successful
message now has searchable hashes:PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks