Bring back zerolog.ConsoleWriter for local development
#312
Conversation
WalkthroughThe updates improve logging capabilities by introducing a Changes
Sequence Diagram(s)None required for these changes. Assessment against linked issues
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Makefile (1 hunks)
- config/config.go (3 hunks)
Additional context used
Gitleaks
Makefile
47-47: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
Additional comments not posted (1)
config/config.go (1)
Line range hint
92-244: Comprehensive update to configuration handling.The updates to the
FromFlagsfunction are extensive and introduce several new configuration options, including handling for multiple log levels and writers. Ensure that these new options are documented and that their interactions are tested, especially with respect to backward compatibility.Verification successful
Comprehensive update to configuration handling.
The updates to the
FromFlagsfunction are extensive and introduce several new configuration options, including handling for multiple log levels and writers. The new flags are properly integrated into the configuration handling, and there are tests associated with the usage ofbootstrap.Startand the new flags.
- The
FromFlagsfunction is utilized correctly incmd/main/main.go.- The
bootstrap.Startfunction properly uses the configuration options set byFromFlags.- Tests in
tests/helpers.goandtests/integration_test.gocover the initialization and starting process, ensuring backward compatibility and proper handling of new flags.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the backward compatibility of the new configuration options. # Test: Search for usages of the FromFlags function. Expect: Proper handling of new and old flags. rg --type go $'FromFlags'Length of output: 126
Script:
#!/bin/bash # Description: Inspect the context of FromFlags usage in cmd/main/main.go. # Display the lines around the usage of FromFlags to understand how it is being used. rg --type go --context 10 'cfg, err := config.FromFlags()' cmd/main/main.goLength of output: 467
Script:
#!/bin/bash # Description: Inspect the bootstrap.Start function to verify how the configuration is used. # Search for the definition and usage of the bootstrap.Start function. rg --type go 'func Start' --context 10Length of output: 1288
Script:
#!/bin/bash # Description: Retrieve the entire implementation of the bootstrap.Start function. # Display the complete implementation of the bootstrap.Start function. rg --type go 'func Start' --context 50 bootstrap/bootstrap.goLength of output: 2610
Script:
#!/bin/bash # Description: Verify if there are any tests associated with bootstrap.Start and the new flags. # Search for test files and functions related to bootstrap.Start. rg --type go 'bootstrap.Start' --context 10 # Search for test files and functions related to the new flags. rg --type go 'coa-key' --context 10 rg --type go 'coa-key-file' --context 10 rg --type go 'coa-cloud-kms-keys' --context 10Length of output: 21559
| @@ -44,4 +44,4 @@ start: | |||
| .PHONY: start-local | |||
| start-local: | |||
| rm -rf db/ | |||
| go run cmd/main/main.go --flow-network-id=flow-emulator --coinbase=FACF71692421039876a5BB4F10EF7A439D8ef61E --coa-address=f8d6e0586b0a20c7 --coa-key=2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21 --coa-resource-create=true --gas-price=0 | |||
| go run cmd/main/main.go --flow-network-id=flow-emulator --coinbase=FACF71692421039876a5BB4F10EF7A439D8ef61E --coa-address=f8d6e0586b0a20c7 --coa-key=2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21 --coa-resource-create=true --gas-price=0 --log-writer=console | |||
There was a problem hiding this comment.
Potential security issue: Exposed sensitive key in the Makefile.
The command includes a hardcoded key, which is a security risk. This should be replaced with an environment variable or a secure vault solution.
- go run cmd/main/main.go --flow-network-id=flow-emulator --coinbase=FACF71692421039876a5BB4F10EF7A439D8ef61E --coa-address=f8d6e0586b0a20c7 --coa-key=2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21 --coa-resource-create=true --gas-price=0 --log-writer=console
+ go run cmd/main/main.go --flow-network-id=flow-emulator --coinbase=$COINBASE --coa-address=$COA_ADDRESS --coa-key=$COA_KEY --coa-resource-create=true --gas-price=0 --log-writer=consoleCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| go run cmd/main/main.go --flow-network-id=flow-emulator --coinbase=FACF71692421039876a5BB4F10EF7A439D8ef61E --coa-address=f8d6e0586b0a20c7 --coa-key=2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21 --coa-resource-create=true --gas-price=0 --log-writer=console | |
| go run cmd/main/main.go --flow-network-id=flow-emulator --coinbase=$COINBASE --coa-address=$COA_ADDRESS --coa-key=$COA_KEY --coa-resource-create=true --gas-price=0 --log-writer=console |
Tools
Gitleaks
47-47: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
| if logWriter == "stderr" { | ||
| cfg.LogWriter = os.Stderr | ||
| } else { | ||
| cfg.LogWriter = zerolog.NewConsoleWriter() | ||
| } |
There was a problem hiding this comment.
Improvement in log writer configuration.
The conditional handling of the logWriter to decide between stderr and console is a good addition. Consider adding a default case for safety to handle unexpected values.
if logWriter == "stderr" {
cfg.LogWriter = os.Stderr
} else if logWriter == "console" {
cfg.LogWriter = zerolog.NewConsoleWriter()
} else {
// Default case to handle unexpected values
cfg.LogWriter = os.Stderr
fmt.Println("Warning: Unrecognized log writer option, defaulting to stderr")
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if logWriter == "stderr" { | |
| cfg.LogWriter = os.Stderr | |
| } else { | |
| cfg.LogWriter = zerolog.NewConsoleWriter() | |
| } | |
| if logWriter == "stderr" { | |
| cfg.LogWriter = os.Stderr | |
| } else if logWriter == "console" { | |
| cfg.LogWriter = zerolog.NewConsoleWriter() | |
| } else { | |
| // Default case to handle unexpected values | |
| cfg.LogWriter = os.Stderr | |
| fmt.Println("Warning: Unrecognized log writer option, defaulting to stderr") | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- api/server.go (7 hunks)
- tests/helpers.go (4 hunks)
- tests/integration_test.go (2 hunks)
Additional context used
Gitleaks
tests/helpers.go
51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
Additional comments not posted (6)
tests/integration_test.go (2)
74-74: Logging Configuration UpdatedThe logging output has been switched to
zerolog.NewConsoleWriter()to enhance readability during local development. This change is consistent with the PR's objectives.
204-204: Consistent Logging EnhancementThe update to
zerolog.NewConsoleWriter()here ensures consistent logging across different test scenarios, aiding in better debugging and development.tests/helpers.go (2)
42-44: Logger Initialization UpdatedThe logger has been initialized with
zerolog.NewConsoleWriter(), which is suitable for local development. Additionally, the handling of theLOG_OUTPUTenvironment variable to toggle logging is a flexible approach.
Line range hint
51-51: False Positive: API Key ExposureThe detected API key exposure issues are false positives. The keys in question are for testing purposes only and not real API keys.
Also applies to: 54-54
api/server.go (2)
214-214: Enhanced Logging for RPC ServicesLogging statements have been added to indicate when JSON-RPC over HTTP and WebSocket are enabled, providing clear runtime diagnostics.
Also applies to: 219-219
305-311: Server Shutdown Handling EnhancedThe changes improve the server's shutdown process by adding logging to indicate when the server has stopped and if the shutdown was not graceful. This is crucial for operational transparency.
Closes: #162
Description
This has been already fixed:
{ "level": "debug", "component": "API", "IP": "127.0.0.1:60314", "url": "/", "id": 1, "jsonrpc": "2.0", "method": "eth_blockNumber", "params": [], "is-ws": false, "time": "2024-06-21T11:39:02Z", "message": "API request" } { "level": "debug", "component": "API", "IP": "127.0.0.1:33588", "url": "/", "id": "abc-123-abd", // id is a string, and it does not error out "jsonrpc": "2.0", "method": "eth_blockNumber", "params": [], "is-ws": false, "time": "2024-06-21T11:39:14Z", "message": "API request" }This has been brought back.
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Makefileto include enhanced logging options for local execution.