Remove fileshare logs #1814
Remove fileshare logs #1814
Conversation
WalkthroughThe pull request introduces changes to logging mechanisms in two different Go packages: In the ✨ 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 (3)
pkg/wshrpc/wshremote/wshremote.go (3)
337-337: Consider adding file size to copy operation logs.While the current logs are helpful, adding file sizes would provide better visibility into the scope of copy operations.
-logPrintfDev("copying %q to %q\n", srcUri, destUri) +logPrintfDev("copying %q (%d bytes) to %q\n", srcUri, srcInfo.Size(), destUri)Also applies to: 343-343
Line range hint
1-808: Standardize logging approach across the file.Some functions still use direct
log.Printfcalls (e.g., line 309:log.Printf("RemoteTarStreamCommand: path=%s\n", path)). Consider replacing all debug logging withlogPrintfDevfor consistency.
Line range hint
1-808: Consider sanitizing sensitive information in logs.File paths in logs might contain sensitive information. Consider implementing path sanitization for logs to mask sensitive parts of paths (e.g., usernames in home directories).
Example implementation:
func sanitizePath(path string) string { // Mask username in home directory paths home := os.Getenv("HOME") if strings.HasPrefix(path, home) { return strings.Replace(path, home, "~/", 1) } return path }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/remote/fileshare/fileshare.go(0 hunks)pkg/wshrpc/wshremote/wshremote.go(10 hunks)
💤 Files with no reviewable changes (1)
- pkg/remote/fileshare/fileshare.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (5)
pkg/wshrpc/wshremote/wshremote.go (5)
804-808: LGTM! Good practice to conditionally log based on environment.The new
logPrintfDevfunction effectively gates logging behind a development mode check, which helps reduce noise in production logs.
108-108: LGTM! Helpful debug logs for chunked operations.The logs provide good visibility into the chunking process during development.
Also applies to: 114-114
257-257: LGTM! Comprehensive logging for tar stream operations.The logs provide good coverage of the tar stream lifecycle and progress.
Also applies to: 259-259, 291-291, 300-300, 302-302
657-657: LGTM! Good progression logging for move operations.The logs provide clear visibility into each step of the move operation.
Also applies to: 683-683, 688-688, 690-690, 692-692
779-779: LGTM! Comprehensive write operation logging.The log includes all essential information (bytes written, file path, offset) for debugging write operations.
No description provided.