HYPERFLEET-363 | feat(logger): Integrate database logging with LOG_LEVEL and fix mixed output format #35
Conversation
WalkthroughThis PR makes logging environment-driven and runtime-reconfigurable across the app and database layers. It adds LOG_LEVEL, LOG_FORMAT, and LOG_OUTPUT handling in cmd/hyperfleet-api, introduces DB_DEBUG and BindEnv for DatabaseConfig to derive a GORM log level, implements a Go-based GormLogger in pkg/logger, adds LoggerReconfigurable and ReconfigureLogger methods to DB session implementations, and extends the text log handler to carry attributes. Several call sites are updated to bind env flags and reconfigure DB loggers after global logger initialization. Sequence Diagram(s)sequenceDiagram
participant Main as Application Startup
participant Env as Environment
participant AppLogger as Application Logger
participant DBConfig as Database Config
participant DBSession as Database Session
participant GormLogger as GORM Logger
Main->>Env: Read LOG_LEVEL, LOG_FORMAT, LOG_OUTPUT
Env-->>Main: Return values
Main->>AppLogger: Initialize global logger (level/format/output)
AppLogger-->>Main: Logger ready
Main->>DBConfig: Bind env flags (DB_DEBUG)
DBConfig-->>Main: Provide DB log level (GetGormLogLevel)
Main->>DBSession: Initialize session with DBConfig
DBSession->>GormLogger: Create NewGormLogger(level, threshold)
GormLogger-->>DBSession: Attached to GORM
Main->>AppLogger: Reconfigure (post-init)
AppLogger->>DBSession: If supported, call ReconfigureLogger(gormLevel)
DBSession->>GormLogger: Update logger level
GormLogger-->>DBSession: Logger reconfigured
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![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
🤖 Fix all issues with AI agents
In `@pkg/logger/gorm_logger.go`:
- Around line 82-87: formatMessage currently ignores the data slice so
contextual args passed to Info/Warn/Error are lost; change it to return
fmt.Sprintf(msg, data...) when len(data) > 0 (and keep returning msg when
empty), and add an import for "fmt" if missing so callers of formatMessage
(e.g., methods Info, Warn, Error) receive the formatted string with provided
parameters.
🧹 Nitpick comments (2)
pkg/db/db_session/testcontainer.go (1)
101-106: Inconsistent default log level between production and test implementations.
Testcontainer.Initusesgormlogger.Silentwhenconfig.Debugis false, whileDefault.Initusesgormlogger.Warn. This means slow queries (>200ms) and errors won't be logged in test environments but will be in production.If this is intentional for quieter test output, consider adding a comment. Otherwise, align the behavior:
♻️ Suggested fix for consistency
var gormLog gormlogger.Interface if config.Debug { gormLog = logger.NewGormLogger(gormlogger.Info, slowQueryThreshold) } else { - gormLog = logger.NewGormLogger(gormlogger.Silent, slowQueryThreshold) + gormLog = logger.NewGormLogger(gormlogger.Warn, slowQueryThreshold) }pkg/config/db.go (1)
138-153: Consider adding "info" and "warn" cases for explicit clarity.The
GetGormLogLevelfunction handles"debug","error", and defaults toWarn. Adding explicit cases for"info"and"warn"would make the mapping self-documenting and prevent accidental behavior changes if the default is modified later.♻️ Suggested improvement
func (c *DatabaseConfig) GetGormLogLevel(logLevel string) logger.LogLevel { if c.Debug { return logger.Info } switch strings.ToLower(strings.TrimSpace(logLevel)) { case "debug": return logger.Info + case "info", "warn": + return logger.Warn case "error": return logger.Silent default: return logger.Warn } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cmd/hyperfleet-api/main.gocmd/hyperfleet-api/migrate/cmd.gocmd/hyperfleet-api/servecmd/cmd.godocs/logging.mdpkg/config/db.gopkg/dao/generic.gopkg/db/db_session/db_session.gopkg/db/db_session/default.gopkg/db/db_session/test.gopkg/db/db_session/testcontainer.gopkg/logger/gorm_logger.gopkg/logger/text_handler.go
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Use structured logging via logger.NewOCMLogger(ctx) with context-aware log fields including operation ID, account ID, and transaction ID
Files:
pkg/dao/generic.gopkg/db/db_session/db_session.gocmd/hyperfleet-api/servecmd/cmd.gopkg/config/db.gocmd/hyperfleet-api/main.gopkg/db/db_session/testcontainer.gocmd/hyperfleet-api/migrate/cmd.gopkg/db/db_session/default.gopkg/db/db_session/test.gopkg/logger/gorm_logger.gopkg/logger/text_handler.go
pkg/dao/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/dao/**/*.go: All database sessions must be retrieved from context via db.NewContext(ctx) rather than creating new database connections
All DAO methods must accept context.Context as the first parameter for transaction propagation
Always retrieve database sessions from context in DAO methods; never create direct gorm.Open() connections
All DAO interface methods must include Create, Get, List, Update, Delete operations with consistent signatures taking context.Context
Files:
pkg/dao/generic.go
pkg/{dao,services}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Files:
pkg/dao/generic.go
pkg/{dao,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use polymorphic status tables with owner_type + owner_id to support multiple resource types without creating separate status tables
Files:
pkg/dao/generic.gopkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.gopkg/db/db_session/default.gopkg/db/db_session/test.go
pkg/{dao,handlers}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Files:
pkg/dao/generic.go
pkg/{handlers,services,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Files:
pkg/dao/generic.go
pkg/{db/migrations,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
NodePool owner_references.id must enforce foreign key relationship to parent cluster to maintain referential integrity
Files:
pkg/dao/generic.go
pkg/db/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use PostgreSQL 14.2 with GORM ORM for database operations
Files:
pkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.gopkg/db/db_session/default.gopkg/db/db_session/test.go
pkg/{handlers,services,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Files:
pkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.gopkg/db/db_session/default.gopkg/db/db_session/test.go
🧠 Learnings (19)
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
pkg/dao/generic.gocmd/hyperfleet-api/servecmd/cmd.gopkg/db/db_session/test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,handlers}/**/*.go : List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Applied to files:
pkg/dao/generic.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for database operations
Applied to files:
pkg/dao/generic.gopkg/db/db_session/db_session.godocs/logging.mdcmd/hyperfleet-api/servecmd/cmd.gopkg/config/db.gopkg/db/db_session/testcontainer.gopkg/db/db_session/default.gopkg/db/db_session/test.gopkg/logger/gorm_logger.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : Always retrieve database sessions from context in DAO methods; never create direct gorm.Open() connections
Applied to files:
pkg/dao/generic.gopkg/db/db_session/db_session.gocmd/hyperfleet-api/servecmd/cmd.gopkg/config/db.gopkg/db/db_session/testcontainer.gopkg/db/db_session/default.gopkg/db/db_session/test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : All DAO interface methods must include Create, Get, List, Update, Delete operations with consistent signatures taking context.Context
Applied to files:
pkg/dao/generic.go
📚 Learning: 2026-01-13T04:16:08.743Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:08.743Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger(ctx) is deprecated. Switch to the slog-based logger and use logger.With(ctx, ...) to produce structured, context-aware logs. Replace occurrences of NewOCMLogger(ctx) with logger.With(ctx, ...). Update imports to use the new logger and adjust any tests to expect structured logging behavior.
Applied to files:
pkg/dao/generic.gopkg/db/db_session/db_session.gocmd/hyperfleet-api/servecmd/cmd.gopkg/config/db.gocmd/hyperfleet-api/main.gopkg/db/db_session/testcontainer.gocmd/hyperfleet-api/migrate/cmd.gopkg/db/db_session/default.gopkg/db/db_session/test.gopkg/logger/gorm_logger.gopkg/logger/text_handler.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : All database sessions must be retrieved from context via db.NewContext(ctx) rather than creating new database connections
Applied to files:
pkg/db/db_session/db_session.gocmd/hyperfleet-api/servecmd/cmd.gopkg/db/db_session/testcontainer.gopkg/db/db_session/default.gopkg/db/db_session/test.go
📚 Learning: 2026-01-08T10:03:05.524Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/logger/logger.go:176-187
Timestamp: 2026-01-08T10:03:05.524Z
Learning: In pkg/logger/logger.go, InitGlobalLogger uses atomic.Value without sync.Once for concurrent initialization. This is an intentional design choice: all current call sites are serialized (single-threaded main() and sync.Once in tests), and the last Store() winning is acceptable. Callers can wrap with sync.Once if stricter guarantees are needed.
Applied to files:
pkg/db/db_session/db_session.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Applied to files:
docs/logging.mdcmd/hyperfleet-api/servecmd/cmd.gocmd/hyperfleet-api/main.gocmd/hyperfleet-api/migrate/cmd.go
📚 Learning: 2026-01-13T04:16:17.444Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:17.444Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger has been deprecated. The new logging pattern uses the slog-based logger with logger.With(ctx, ...) for structured, context-aware logging instead of NewOCMLogger(ctx).
Applied to files:
docs/logging.md
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{handlers,services,db}/**/*.go : Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Applied to files:
docs/logging.mdcmd/hyperfleet-api/servecmd/cmd.gocmd/hyperfleet-api/main.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Database transactions are automatically managed by TransactionMiddleware for all HTTP requests; store session in context and auto-commit on success, rollback on error
Applied to files:
cmd/hyperfleet-api/servecmd/cmd.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
cmd/hyperfleet-api/servecmd/cmd.gopkg/config/db.gocmd/hyperfleet-api/main.gocmd/hyperfleet-api/migrate/cmd.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
cmd/hyperfleet-api/servecmd/cmd.gocmd/hyperfleet-api/main.gocmd/hyperfleet-api/migrate/cmd.go
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
cmd/hyperfleet-api/servecmd/cmd.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
cmd/hyperfleet-api/servecmd/cmd.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/migrations/**/*.go : Store spec and conditions as JSONB in PostgreSQL to enable complex field queries
Applied to files:
pkg/config/db.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : Integration tests must use Testcontainers to create isolated PostgreSQL instances for each test suite to ensure no state leakage
Applied to files:
pkg/db/db_session/testcontainer.go
📚 Learning: 2026-01-09T13:53:50.367Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/logger/text_handler.go:86-97
Timestamp: 2026-01-09T13:53:50.367Z
Learning: In pkg/logger/text_handler.go, the HyperFleetTextHandler.WithAttrs method intentionally returns self without storing attributes, keeping the implementation simple until there's a concrete requirement for attribute persistence.
Applied to files:
pkg/logger/text_handler.go
🧬 Code graph analysis (5)
cmd/hyperfleet-api/servecmd/cmd.go (4)
cmd/hyperfleet-api/environments/framework.go (1)
Environment(55-57)cmd/hyperfleet-api/environments/types.go (1)
Database(36-38)pkg/db/session.go (1)
SessionFactory(12-20)pkg/db/db_session/db_session.go (1)
LoggerReconfigurable(16-18)
pkg/config/db.go (1)
pkg/logger/logger.go (3)
Debug(247-249)Info(253-255)Warn(259-261)
pkg/db/db_session/default.go (3)
pkg/logger/logger.go (3)
Debug(247-249)Info(253-255)Warn(259-261)pkg/logger/gorm_logger.go (1)
NewGormLogger(16-21)pkg/client/ocm/client.go (1)
Config(17-24)
pkg/db/db_session/test.go (3)
pkg/logger/logger.go (2)
Debug(247-249)Info(253-255)pkg/logger/gorm_logger.go (1)
NewGormLogger(16-21)pkg/client/ocm/client.go (1)
Config(17-24)
pkg/logger/gorm_logger.go (1)
pkg/logger/logger.go (4)
Info(253-255)With(276-281)Warn(259-261)Error(265-267)
🪛 LanguageTool
docs/logging.md
[grammar] ~524-~524: Ensure spelling is correct
Context: ...y log SQL queries that take longer than 200ms. ## Testing ### Unit Tests ```go fun...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (25)
pkg/logger/text_handler.go (4)
19-27: LGTM!The
attrsfield addition properly enables attribute accumulation for theslog.Handlerinterface. The nil zero value is idiomatic Go and works correctly with the slice operations inWithAttrs.
61-63: LGTM!The attribute rendering follows a logical order: context fields → handler attrs → record attrs. Since
WithAttrscreates a new handler instance,h.attrsis effectively immutable duringHandleexecution, avoiding any race conditions.
92-105: LGTM!The
WithAttrsimplementation now correctly follows theslog.Handlercontract by returning a new handler instance with merged attributes rather than mutating the existing one. This enables proper attribute accumulation when usingslog.Logger.With(). Based on learnings, this is an intentional improvement over the previous self-returning implementation.
108-111: Note:WithGroupstill returns self.This is unchanged behavior and acceptable per HyperFleet spec. However, if nested loggers with groups become a requirement in the future, this will need updating to prefix attribute keys with the group name.
pkg/dao/generic.go (1)
68-70: LGTM! Correctly removes hardcoded debug logging.Removing
.Debug()from the query chain is the right approach. Debug logging is now controlled by the environment-drivenDB_DEBUGconfiguration and the newLoggerReconfigurableinterface, which provides consistent logging behavior across the application.cmd/hyperfleet-api/servecmd/cmd.go (2)
41-42: LGTM! Correct ordering for environment variable binding.Binding database environment variables before
Initialize()ensures the database configuration is properly set before the session factory is created. This is essential for the environment-drivenDB_DEBUGand log level configuration to take effect during database initialization.
147-157: LGTM! Clean implementation of runtime logger reconfiguration.The implementation correctly:
- Checks for nil session factory before proceeding
- Uses the proper Go type assertion idiom to check for
LoggerReconfigurableinterface support- Derives the GORM log level from the application's log level via
GetGormLogLevelThis ensures the database logger follows the same log level as the application logger.
pkg/db/db_session/db_session.go (1)
15-17: LGTM! Well-designed interface for runtime logger reconfiguration.The
LoggerReconfigurableinterface is appropriately minimal with a single method. Using GORM's nativelogger.LogLeveltype ensures type safety and seamless integration with GORM's logging infrastructure.cmd/hyperfleet-api/main.go (1)
52-74: LGTM! Robust environment-driven logger configuration with safe defaults.The implementation correctly:
- Reads environment variables with appropriate fallbacks (Info level, JSON format, stdout output)
- Uses the parser functions for validation
- Silently falls back on parse errors, which is appropriate at this early startup stage before the logger is fully configured
Production-safe defaults (JSON format, Info level) are good choices.
pkg/db/db_session/test.go (3)
195-204: LGTM! Simplified session creation.Removing the conditional
Debug()wrapper and returning the session directly is consistent with the PR's goal of delegating logging configuration to the environment-driven settings andReconfigureLoggermethod.
227-234: LGTM! Correct implementation of LoggerReconfigurable interface.The implementation correctly:
- Guards against nil
g2with an early return- Creates a new GormLogger with the specified level
- Assigns directly to
f.g2.Loggerfor runtime reconfigurationThis enables the serve command to dynamically adjust the database log level after initialization.
129-141: Logger initialization correctly uses environment-driven log levels and GORM configuration.The code properly:
- Uses
config.Debugto select log level (Info vs Silent)- Leverages
logger.NewGormLoggerwith the definedslowQueryThresholdconstant (200ms)- Configures GORM logger during connection setup
pkg/db/db_session/default.go (3)
69-79: LGTM: Environment-driven GORM logger initialization.The conditional logger creation based on
config.Debugis clean and follows the pattern documented in the PR. Usinggormlogger.Infofor debug mode andgormlogger.Warnotherwise aligns with the documented LOG_LEVEL integration behavior.
151-155: LGTM: Simplified session creation.The simplified
New()method correctly delegates logging configuration to the GORM instance's pre-configured logger rather than toggling logging per-session.
172-179: Remove this comment - ReconfigureLogger is called only once during startup initialization, not at runtime.
ReconfigureLoggeris invoked ininitLogger()during server startup (runServe()at line 53), before any servers begin accepting requests. This is a single, serialized initialization operation that completes beforeNew()is ever called to return sessions referencingf.g2. No concurrent write scenario exists during active request processing.Likely an incorrect or invalid review comment.
pkg/db/db_session/testcontainer.go (2)
138-142: LGTM: Simplified session creation matches production implementation.
203-211: LGTM: ReconfigureLogger mirrors production implementation.The implementation is consistent with
Default.ReconfigureLogger, maintaining interface parity.docs/logging.md (2)
345-348: LGTM: Backward compatibility clearly documented.The note about existing
--enable-db-debugandDB_DEBUGbehavior is helpful for users upgrading.
266-272: Documentation is accurate. The LOG_LEVEL to GORM level mapping in the table aligns with the implementation inpkg/config/db.go. Specifically:
debugcorrectly maps tologger.Infoerrorcorrectly maps tologger.Silentinfoandwarncorrectly default tologger.Warncmd/hyperfleet-api/migrate/cmd.go (1)
32-35: LGTM: Environment variable binding for migration command.Calling
BindEnvbeforerunMigrateWithError()ensures DB_DEBUG from environment is respected during migrations, enabling consistent logging behavior across serve and migrate commands.pkg/config/db.go (1)
61-72: LGTM: Environment binding with correct priority.The
BindEnvmethod correctly implements the documented priority (flags > env vars > defaults) by checkingfs.Changed("enable-db-debug")before applying the environment variable value.pkg/logger/gorm_logger.go (4)
1-14: LGTM!Package structure, imports, and struct definition are appropriate. The struct correctly encapsulates the GORM log level and slow query threshold configuration.
16-21: LGTM!Constructor is clean and correctly initializes the logger with provided configuration.
23-28: LGTM!
LogModecorrectly implements thegormlogger.Interfacecontract with an immutable approach, preserving the slow threshold configuration when changing log levels.
30-46: LGTM on structure; note dependency onformatMessage.The log level checks and use of
logger.With(ctx, ...)align with the slog-based logging pattern per the project guidelines. However, the effectiveness of these methods depends onformatMessagebeing correctly implemented (see comment below).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| func (l *GormLogger) Trace(ctx context.Context, begin time.Time, fc func() (sql string, rowsAffected int64), err error) { | ||
| if l.logLevel <= gormlogger.Silent { | ||
| return | ||
| } | ||
|
|
||
| elapsed := time.Since(begin) | ||
| sql, rows := fc() | ||
|
|
||
| switch { | ||
| case err != nil && l.logLevel >= gormlogger.Error && !errors.Is(err, gormlogger.ErrRecordNotFound): | ||
| With(ctx, | ||
| "error", err.Error(), | ||
| "duration_ms", float64(elapsed.Nanoseconds())/1e6, | ||
| "rows", rows, | ||
| "sql", sql, | ||
| ).Error("GORM query error") | ||
|
|
||
| case elapsed > l.slowThreshold && l.slowThreshold != 0 && l.logLevel >= gormlogger.Warn: | ||
| With(ctx, | ||
| "duration_ms", float64(elapsed.Nanoseconds())/1e6, | ||
| "threshold_ms", float64(l.slowThreshold.Nanoseconds())/1e6, | ||
| "rows", rows, | ||
| "sql", sql, | ||
| ).Warn("GORM slow query") | ||
|
|
||
| case l.logLevel >= gormlogger.Info: | ||
| With(ctx, | ||
| "duration_ms", float64(elapsed.Nanoseconds())/1e6, | ||
| "rows", rows, | ||
| "sql", sql, | ||
| ).Info("GORM query") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Explore the logger package structure
find pkg/logger -type f -name "*.go" | head -20Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 332
🏁 Script executed:
# Check the full gorm_logger.go file to understand the complete implementation
wc -l pkg/logger/gorm_logger.goRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 107
🏁 Script executed:
# Look for GORM configuration in the codebase
rg -i "gorm.*config|preparedstmt|parameterized" --type go | head -30Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 741
🏁 Script executed:
# Search for where the GORM logger is instantiated
rg "NewGormLogger|NewLogger.*gorm" --type go -A 3 | head -40Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 1989
🏁 Script executed:
# Check if there's any SQL sanitization or parameter binding utilities
rg -i "sanitize|sensitive|password|token|redact" pkg/logger --type goRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 508
🏁 Script executed:
# Read the full gorm_logger.go file
cat -n pkg/logger/gorm_logger.goRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 2843
🏁 Script executed:
# Check the With() function implementation in logger.go
cat -n pkg/logger/logger.go | head -100Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 3354
🏁 Script executed:
# Check GORM configuration in default.go
sed -n '1,100p' pkg/db/db_session/default.go | cat -nRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 3626
🏁 Script executed:
# Verify GORM behavior with PrepareStmt=false by checking documentation or implementation
rg "PrepareStmt" --type go -B 2 -A 2Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 901
🏁 Script executed:
# Check if there are any existing tests or comments about sensitive data handling
rg -i "sensitive|password|token|credential" --type go pkg/logger pkg/dbRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1131
SQL query parameters should be sanitized before logging to prevent exposing credentials and sensitive data.
The Trace method logs full SQL queries directly (lines 62, 70, 77). Since GORM is configured with PrepareStmt: false across all database configurations (default.go, testcontainer.go, test.go), parameter values are interpolated directly into the SQL string rather than using placeholders. This means logged queries may contain passwords, API tokens, or PII.
The codebase already demonstrates awareness of this risk (e.g., redactPassword() in testcontainer.go), so apply the same principle to SQL query logging:
- Enable parameterized queries: Change
PrepareStmt: true(or use parameterized logging that separates SQL from parameters). - Sanitize sensitive parameters: If parameterized SQL cannot be used, implement a sanitization function to redact known sensitive fields before logging.
There was a problem hiding this comment.
Keep it as-is as there is no sensitive valued in SQL
There was a problem hiding this comment.
@yasun1, understood! I'll note that if your data model evolves in the future to include sensitive information (e.g., credentials, tokens, or PII), you may want to revisit the logging approach at that time.
🧠 Learnings used
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for database operations
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:17.444Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger has been deprecated. The new logging pattern uses the slog-based logger with logger.With(ctx, ...) for structured, context-aware logging instead of NewOCMLogger(ctx).
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : Always retrieve database sessions from context in DAO methods; never create direct gorm.Open() connections
… output format update update
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/logging.md`:
- Around line 293-343: Update the slow-query JSON example to match the actual
GORM logger output: change the "message" value from "GORM query" to "GORM slow
query" (as emitted at pkg/logger/gorm_logger.go line 72) and add the
"threshold_ms" field (as emitted at line 69) with the configured threshold value
(e.g., 200.0) so the example reflects the real emitted keys and values.
🧹 Nitpick comments (4)
pkg/logger/text_handler.go (1)
107-111: Consider documenting the limitation or returning a new instance for consistency.
WithGroupstill returnsself, which is inconsistent with the updatedWithAttrsbehavior. If a caller chainsWithGroup("db").WithAttrs(...), the group is silently ignored while attrs are applied. Given that groups aren't needed per the HyperFleet spec, this is acceptable, but consider adding a brief comment noting the asymmetry withWithAttrsfor future maintainers.pkg/config/db.go (1)
64-70: Consider logging a warning whenDB_DEBUGcontains an unparseable value.If
DB_DEBUGis set to an invalid boolean (e.g.,"yes","1.0"), the error is silently ignored and the default value persists. This could cause confusion during debugging.♻️ Suggested improvement
if val := os.Getenv("DB_DEBUG"); val != "" { if fs == nil || !fs.Changed("enable-db-debug") { debug, err := strconv.ParseBool(val) if err == nil { c.Debug = debug + } else { + // Log warning about invalid DB_DEBUG value + // Using fmt since logger may not be initialized yet + fmt.Fprintf(os.Stderr, "Warning: invalid DB_DEBUG value %q, using default\n", val) } } }pkg/db/db_session/testcontainer.go (2)
101-106: Note: Default log level differs from production (SilentvsWarn).When
Debug=false, testcontainer usesSilentwhiledefault.gousesWarn. This appears intentional to keep test output clean, but a brief comment would clarify the design decision.📝 Suggested documentation
+ // Use Silent by default in test environments to reduce noise + // (production uses Warn to capture slow queries) var gormLog gormlogger.Interface if config.Debug { gormLog = logger.NewGormLogger(gormlogger.Info, slowQueryThreshold) } else { gormLog = logger.NewGormLogger(gormlogger.Silent, slowQueryThreshold) }
204-211: Consider extracting sharedReconfigureLoggerlogic.The implementation is identical across
Default,Test, andTestcontainer. While the duplication is minimal, you could extract this into a helper function to reduce maintenance burden.♻️ Example helper function
// In db_session package or a shared location func reconfigureGormLogger(g2 *gorm.DB, level gormlogger.LogLevel, threshold time.Duration) { if g2 == nil { return } g2.Logger = logger.NewGormLogger(level, threshold) }Each implementation would then call:
func (f *Testcontainer) ReconfigureLogger(level gormlogger.LogLevel) { reconfigureGormLogger(f.g2, level, slowQueryThreshold) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cmd/hyperfleet-api/main.gocmd/hyperfleet-api/migrate/cmd.gocmd/hyperfleet-api/servecmd/cmd.godocs/logging.mdpkg/config/db.gopkg/dao/generic.gopkg/db/db_session/db_session.gopkg/db/db_session/default.gopkg/db/db_session/test.gopkg/db/db_session/testcontainer.gopkg/logger/gorm_logger.gopkg/logger/text_handler.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/db/db_session/test.go
- cmd/hyperfleet-api/main.go
- cmd/hyperfleet-api/servecmd/cmd.go
- pkg/logger/gorm_logger.go
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Use structured logging via logger.NewOCMLogger(ctx) with context-aware log fields including operation ID, account ID, and transaction ID
Files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gocmd/hyperfleet-api/migrate/cmd.gopkg/logger/text_handler.gopkg/db/db_session/testcontainer.gopkg/config/db.gopkg/dao/generic.go
pkg/db/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use PostgreSQL 14.2 with GORM ORM for database operations
Files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.go
pkg/{dao,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use polymorphic status tables with owner_type + owner_id to support multiple resource types without creating separate status tables
Files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.gopkg/dao/generic.go
pkg/{handlers,services,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.go
pkg/dao/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/dao/**/*.go: All database sessions must be retrieved from context via db.NewContext(ctx) rather than creating new database connections
All DAO methods must accept context.Context as the first parameter for transaction propagation
Always retrieve database sessions from context in DAO methods; never create direct gorm.Open() connections
All DAO interface methods must include Create, Get, List, Update, Delete operations with consistent signatures taking context.Context
Files:
pkg/dao/generic.go
pkg/{dao,services}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Files:
pkg/dao/generic.go
pkg/{dao,handlers}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Files:
pkg/dao/generic.go
pkg/{handlers,services,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Files:
pkg/dao/generic.go
pkg/{db/migrations,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
NodePool owner_references.id must enforce foreign key relationship to parent cluster to maintain referential integrity
Files:
pkg/dao/generic.go
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:17.444Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger has been deprecated. The new logging pattern uses the slog-based logger with logger.With(ctx, ...) for structured, context-aware logging instead of NewOCMLogger(ctx).
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for database operations
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{handlers,services,db}/**/*.go : Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : Always retrieve database sessions from context in DAO methods; never create direct gorm.Open() connections
Applied to files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.gopkg/dao/generic.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : All database sessions must be retrieved from context via db.NewContext(ctx) rather than creating new database connections
Applied to files:
pkg/db/db_session/default.gopkg/db/db_session/testcontainer.gopkg/dao/generic.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for database operations
Applied to files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gopkg/db/db_session/testcontainer.gopkg/config/db.godocs/logging.md
📚 Learning: 2026-01-13T04:16:08.743Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:08.743Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger(ctx) is deprecated. Switch to the slog-based logger and use logger.With(ctx, ...) to produce structured, context-aware logs. Replace occurrences of NewOCMLogger(ctx) with logger.With(ctx, ...). Update imports to use the new logger and adjust any tests to expect structured logging behavior.
Applied to files:
pkg/db/db_session/default.gopkg/db/db_session/db_session.gocmd/hyperfleet-api/migrate/cmd.gopkg/logger/text_handler.gopkg/db/db_session/testcontainer.gopkg/config/db.gopkg/dao/generic.go
📚 Learning: 2026-01-08T10:03:05.524Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/logger/logger.go:176-187
Timestamp: 2026-01-08T10:03:05.524Z
Learning: In pkg/logger/logger.go, InitGlobalLogger uses atomic.Value without sync.Once for concurrent initialization. This is an intentional design choice: all current call sites are serialized (single-threaded main() and sync.Once in tests), and the last Store() winning is acceptable. Callers can wrap with sync.Once if stricter guarantees are needed.
Applied to files:
pkg/db/db_session/db_session.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Applied to files:
cmd/hyperfleet-api/migrate/cmd.godocs/logging.md
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
cmd/hyperfleet-api/migrate/cmd.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
cmd/hyperfleet-api/migrate/cmd.gopkg/config/db.go
📚 Learning: 2026-01-09T13:53:50.367Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/logger/text_handler.go:86-97
Timestamp: 2026-01-09T13:53:50.367Z
Learning: In pkg/logger/text_handler.go, the HyperFleetTextHandler.WithAttrs method intentionally returns self without storing attributes, keeping the implementation simple until there's a concrete requirement for attribute persistence.
Applied to files:
pkg/logger/text_handler.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : Integration tests must use Testcontainers to create isolated PostgreSQL instances for each test suite to ensure no state leakage
Applied to files:
pkg/db/db_session/testcontainer.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{handlers,services,db}/**/*.go : Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Applied to files:
pkg/config/db.godocs/logging.md
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/migrations/**/*.go : Store spec and conditions as JSONB in PostgreSQL to enable complex field queries
Applied to files:
pkg/config/db.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
pkg/config/db.gopkg/dao/generic.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,handlers}/**/*.go : List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Applied to files:
pkg/dao/generic.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : All DAO interface methods must include Create, Get, List, Update, Delete operations with consistent signatures taking context.Context
Applied to files:
pkg/dao/generic.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/dao/**/*.go : All DAO methods must accept context.Context as the first parameter for transaction propagation
Applied to files:
pkg/dao/generic.go
📚 Learning: 2026-01-13T04:16:17.444Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:17.444Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger has been deprecated. The new logging pattern uses the slog-based logger with logger.With(ctx, ...) for structured, context-aware logging instead of NewOCMLogger(ctx).
Applied to files:
docs/logging.md
🧬 Code graph analysis (3)
pkg/db/db_session/default.go (3)
pkg/logger/logger.go (3)
Debug(247-249)Info(253-255)Warn(259-261)pkg/logger/gorm_logger.go (1)
NewGormLogger(17-22)pkg/client/ocm/client.go (1)
Config(17-24)
pkg/db/db_session/testcontainer.go (3)
pkg/logger/logger.go (2)
Debug(247-249)Info(253-255)pkg/logger/gorm_logger.go (1)
NewGormLogger(17-22)pkg/client/ocm/client.go (1)
Config(17-24)
pkg/config/db.go (1)
pkg/logger/logger.go (3)
Debug(247-249)Info(253-255)Warn(259-261)
🪛 LanguageTool
docs/logging.md
[grammar] ~524-~524: Ensure spelling is correct
Context: ...y log SQL queries that take longer than 200ms. ## Testing ### Unit Tests ```go fun...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (15)
pkg/dao/generic.go (1)
68-70: LGTM! Correctly removes hard-coded Debug() in favor of centralized logger.The removal of
.Debug()aligns with the PR objective of controlling SQL logging through the new configurable GormLogger rather than hard-coding verbose output. Query execution behavior remains unchanged.pkg/logger/text_handler.go (3)
25-25: LGTM!The new
attrsfield correctly enables per-handler attribute storage, addressing the previous limitation. Leaving it uninitialized (nil) in the constructor is appropriate since nil slices work correctly withlen()andrange.
61-63: LGTM!Handler-level attributes are correctly rendered after context fields and before record-level attributes, maintaining consistent ordering. This ensures attributes set via
WithAttrs()(e.g., by the GORM logger) are properly included in the text output.
92-105: LGTM - Correct slog.Handler immutability pattern.The refactored
WithAttrs()now properly returns a new handler instance with merged attributes, following the standardslog.Handlercontract. This fixes the mixed JSON/text output issue when the GORM logger (or other components) useWithAttrs()to add context.pkg/db/db_session/db_session.go (1)
15-18: LGTM! Clean interface definition for runtime logger reconfiguration.The
LoggerReconfigurableinterface is minimal and well-scoped, enabling consistent runtime logger reconfiguration across alldb_sessionimplementations.cmd/hyperfleet-api/migrate/cmd.go (1)
32-35: LGTM! Environment binding added correctly before migration.The
BindEnvcall ensuresDB_DEBUGis read from the environment before proceeding with migration, maintaining the expected priority order (flags > env vars > defaults).pkg/config/db.go (1)
138-153: LGTM! Clear and well-documented log level mapping.The mapping logic is sound:
DB_DEBUG=truetakes precedence, returningInfofor full SQL loggingdebug→Info(all queries)error→Silent(suppress SQL logging)- Default →
Warn(slow queries + errors only)The use of
strings.ToLowerandstrings.TrimSpaceensures robust input handling.pkg/db/db_session/default.go (3)
69-79: LGTM! Logger initialization centralized in Init.Moving logger configuration from
New()toInit()is a good design choice—it avoids redundant logger creation on every session request and aligns with the runtime reconfiguration pattern.
151-155: LGTM! Simplified session creation.The
New()method now cleanly returns a session with context propagation, delegating logger configuration toInit()andReconfigureLogger().
172-179: Consider thread-safety forReconfigureLogger.Directly assigning to
f.g2.Loggerwhile concurrent goroutines may be executing queries could cause a data race. If this method is only called during startup (afterInitbut before serving requests), this is fine. However, if runtime reconfiguration is intended during active request handling, consider using a mutex or atomic swap.Given that the PR description mentions "runtime logger reconfiguration," please confirm that
ReconfigureLoggeris only called during initialization phases (e.g., inservecmdbefore the server starts accepting requests), not during active traffic.pkg/db/db_session/testcontainer.go (1)
138-142: LGTM! Consistent session creation pattern.Matches the simplified approach in
default.go.docs/logging.md (4)
70-74: LGTM! Clear documentation of DB_DEBUG.The DB_DEBUG environment variable is documented clearly with appropriate context about its behavior and default value.
452-470: Excellent best practices guidance.The reorganization into "Application Logging" and "Database Logging" subsections improves clarity. The database logging best practices provide practical guidance for different scenarios (production, development, debugging, high-traffic).
501-524: Helpful troubleshooting guidance for database logging.The three new database logging troubleshooting sections provide practical solutions for common scenarios. The guidance is clear and actionable.
Note: The static analysis warning about "200ms" on line 524 is a false positive - this is standard notation for milliseconds.
266-271: No issues found - documentation is accurate.All claims in the table have been verified against the implementation:
- info/warn mapping: Both correctly map to the same GORM Warn level by design (pkg/config/db.go lines 145-151, default case returns logger.Warn)
- error → Silent behavior: Correctly disables all SQL logging; Silent level causes early exit before any query/error logging (pkg/logger/gorm_logger.go line 50-51)
- 200ms threshold: Matches the constant definition (pkg/db/db_session/default.go line 20:
const slowQueryThreshold = 200 * time.Millisecond)- Warn level output: Confirmed to log slow queries (>200ms) and errors (pkg/logger/gorm_logger.go lines 58-72)
- Info level output: Confirmed to log all queries (pkg/logger/gorm_logger.go lines 74-79)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| JSON format: | ||
| ```json | ||
| { | ||
| "timestamp": "2026-01-14T11:31:29.788683+08:00", | ||
| "level": "info", | ||
| "message": "GORM query", | ||
| "duration_ms": 9.052167, | ||
| "rows": 1, | ||
| "sql": "INSERT INTO \"clusters\" (\"id\",\"created_time\",...) VALUES (...)", | ||
| "component": "api", | ||
| "version": "0120ac6-modified", | ||
| "hostname": "yasun-mac", | ||
| "request_id": "38EOuujxBDUduP0hYLxVGMm69Dq", | ||
| "transaction_id": 1157 | ||
| } | ||
| ``` | ||
|
|
||
| Text format: | ||
| ```text | ||
| 2026-01-14T11:34:23+08:00 INFO [api] [0120ac6-modified] [yasun-mac] GORM query request_id=38EPGnassU9SLNZ82XiXZLiWS4i duration_ms=10.135875 rows=1 sql="INSERT INTO \"clusters\" (\"id\",\"created_time\",...) VALUES (...)" | ||
| ``` | ||
|
|
||
| **Slow query (>200ms, visible at all log levels except error):** | ||
|
|
||
| ```json | ||
| { | ||
| "timestamp": "2026-01-14T12:00:00Z", | ||
| "level": "warn", | ||
| "message": "GORM query", | ||
| "duration_ms": 250.5, | ||
| "rows": 1000, | ||
| "sql": "SELECT * FROM clusters WHERE ...", | ||
| "request_id": "...", | ||
| "transaction_id": 1234 | ||
| } | ||
| ``` | ||
|
|
||
| **Database error (visible at all log levels):** | ||
|
|
||
| ```json | ||
| { | ||
| "timestamp": "2026-01-14T12:00:00Z", | ||
| "level": "error", | ||
| "message": "GORM query error", | ||
| "error": "pq: duplicate key value violates unique constraint \"idx_clusters_name\"", | ||
| "duration_ms": 10.5, | ||
| "rows": 0, | ||
| "sql": "INSERT INTO \"clusters\" ...", | ||
| "request_id": "..." | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "gorm_logger.go" -type fRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 106
🏁 Script executed:
cat -n ./pkg/logger/gorm_logger.goRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 2879
🏁 Script executed:
rg -n "transaction_id|With\(" ./pkg/logger/ --type=go -B2 -A2 | head -100Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 5272
🏁 Script executed:
sed -n '293,343p' docs/logging.mdRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1372
Update slow query example to match actual GORM logger output.
The slow query example shows incorrect message and is missing a field:
- Message should be
"GORM slow query"(not"GORM query") — see line 72 ofpkg/logger/gorm_logger.go - Missing
"threshold_ms"field that is emitted at line 69
Corrected slow query example
{
"timestamp": "2026-01-14T12:00:00Z",
"level": "warn",
"message": "GORM slow query",
"duration_ms": 250.5,
"threshold_ms": 200.0,
"rows": 1000,
"sql": "SELECT * FROM clusters WHERE ...",
"request_id": "...",
"transaction_id": 1234
}🤖 Prompt for AI Agents
In `@docs/logging.md` around lines 293 - 343, Update the slow-query JSON example
to match the actual GORM logger output: change the "message" value from "GORM
query" to "GORM slow query" (as emitted at pkg/logger/gorm_logger.go line 72)
and add the "threshold_ms" field (as emitted at line 69) with the configured
threshold value (e.g., 200.0) so the example reflects the real emitted keys and
values.
|
|
||
| ### Backward Compatibility | ||
|
|
||
| The existing `--enable-db-debug` CLI flag and `DB_DEBUG` environment variable continue to work exactly as before. The new functionality only adds automatic integration with `LOG_LEVEL` when `DB_DEBUG` is not explicitly set. |
There was a problem hiding this comment.
Should we keep and old config for backward compatibility?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 893f82f
into
openshift-hyperfleet:main
2 participants
Database logs now automatically follow the application LOG_LEVEL setting,
with DB_DEBUG as an override for database-specific debugging.
Changes:
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.