Print DB log #480
Print DB log #480
Conversation
WalkthroughThis pull request introduces a new Changes
Possibly related PRs
Suggested reviewers
✨ 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 (2)
pkg/db/pgx.go (1)
143-144: Enhance logging with more context and precision.The log message could be more precise and include additional context:
- Clarify which step was successful (namespace creation vs. DB connection)
- Include more context like DSN (with sensitive info redacted)
- logger.Info("Successfully connected to DB", zap.String("namespace", namespace)) + logger.Info("Successfully created database namespace", + zap.String("namespace", namespace), + zap.String("dsn", redactDSN(config.ConnConfig.ConnString())), + )Add this helper function at the package level:
// redactDSN removes sensitive information from the DSN func redactDSN(dsn string) string { // Use a regex to replace password and sensitive query params return regexp.MustCompile(`password=([^&\s]+)`).ReplaceAllString(dsn, "password=xxxxx") }Also consider adding another log message after successful DB connection:
db, err := newPGXDB(ctx, config, waitForDB) if err != nil { return nil, err } + logger.Info("Successfully connected to database", zap.String("namespace", namespace))pkg/db/pgx_test.go (1)
19-20: Reduce logger initialization duplication and ensure proper cleanup.The logger initialization is duplicated across test functions. Consider:
- Creating a helper function to initialize the logger
- Using a test-specific logger configuration
- Ensuring proper cleanup
Add this helper function at the package level:
func newTestLogger(t *testing.T) *zap.Logger { // Use nop logger for tests to reduce noise config := zap.NewDevelopmentConfig() config.Level = zap.NewAtomicLevelAt(zap.WarnLevel) logger, err := config.Build() require.NoError(t, err) t.Cleanup(func() { _ = logger.Sync() }) return logger }Then replace all logger initializations with:
- logger, err := zap.NewDevelopment() - require.NoError(t, err) + logger := newTestLogger(t)Also applies to: 47-48, 77-78, 91-92, 143-144
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/replication/main.go(1 hunks)pkg/db/pgx.go(3 hunks)pkg/db/pgx_test.go(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test (Node)
🔇 Additional comments (3)
cmd/replication/main.go (1)
75-77: LGTM!The logger is properly initialized and passed to the
NewNamespacedDBfunction, enhancing the visibility of database operations.pkg/db/pgx.go (1)
8-8: LGTM!The zap logger import is correctly added.
pkg/db/pgx_test.go (1)
6-6: LGTM!The zap logger import is correctly added.
Make it easier to find the DB name in deployed environments.
Summary by CodeRabbit
New Features
Tests