From 513b31e7df2f86641675101f34ac6f03c53c3531 Mon Sep 17 00:00:00 2001 From: Atishyy27 <142108881+Atishyy27@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:46:01 +0530 Subject: [PATCH 1/2] [logger] implement Database.Trace so SQL logging is not swallowed Database.Trace had an empty body, so every SQL statement GORM traced along with its execution time, row count and error was silently discarded. Info, Warn and Error in the same file all forward to the underlying logger, Trace did not. Forward the statement to the logger the way the neighbouring methods do: errors go to the error handler, everything else to the default handler. The message format matches GORM's own logger, including its convention of reporting a row count of -1 as a dash when a count does not apply to the statement. Fixes #1063 Signed-off-by: Atishyy27 <142108881+Atishyy27@users.noreply.github.com> --- logger/database.go | 24 +++++++++++++++++++ logger/database_test.go | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 logger/database_test.go diff --git a/logger/database.go b/logger/database.go index d2ebb3b3d..6074f2bc6 100644 --- a/logger/database.go +++ b/logger/database.go @@ -2,6 +2,8 @@ package logger import ( "context" + "fmt" + "strconv" "time" "github.com/sirupsen/logrus" @@ -38,5 +40,27 @@ func (c *Database) Error(ctx context.Context, msg string, data ...interface{}) { msg, data, ) } + +// Trace is called by GORM after every SQL statement. It forwards the statement, its execution +// time, the affected row count and any error to the underlying logger, matching the format used +// by GORM's own logger. func (c *Database) Trace(ctx context.Context, begin time.Time, fc func() (string, int64), err error) { + sql, rows := fc() + elapsed := float64(time.Since(begin).Nanoseconds()) / 1e6 + + // GORM reports rows as -1 when a row count does not apply to the statement. + affected := strconv.FormatInt(rows, 10) + if rows == -1 { + affected = "-" + } + + if err != nil { + c.base.errorHandler.Log(logrus.ErrorLevel, + fmt.Sprintf("%v [%.3fms] [rows:%s] %s", err, elapsed, affected, sql), + ) + return + } + c.base.defaultHandler.Log(logrus.InfoLevel, + fmt.Sprintf("[%.3fms] [rows:%s] %s", elapsed, affected, sql), + ) } diff --git a/logger/database_test.go b/logger/database_test.go new file mode 100644 index 000000000..cc216a58a --- /dev/null +++ b/logger/database_test.go @@ -0,0 +1,51 @@ +package logger + +import ( + "bytes" + "context" + "errors" + "testing" + "time" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" +) + +func TestDatabase_Trace(t *testing.T) { + opts := Options{ + Format: TerminalLogFormat, + LogLevel: int(logrus.DebugLevel), + EnableCallerInfo: false, + } + log, err := New("testapp", opts) + assert.NoError(t, err) + l := log.(*Logger) + + var outBuffer, errBuffer bytes.Buffer + l.UpdateLogOutput(&outBuffer) + l.UpdateErrorLogOutput(&errBuffer) + + db := l.DatabaseLogger() + + // A successful query is forwarded to the default handler. + db.Trace(context.Background(), time.Now(), func() (string, int64) { + return "SELECT * FROM meshery_patterns", 3 + }, nil) + assert.Contains(t, outBuffer.String(), "SELECT * FROM meshery_patterns") + assert.Contains(t, outBuffer.String(), "[rows:3]") + outBuffer.Reset() + + // A failed query is forwarded to the error handler, along with the error. + db.Trace(context.Background(), time.Now(), func() (string, int64) { + return "SELECT * FROM missing_table", 0 + }, errors.New("no such table: missing_table")) + assert.Contains(t, errBuffer.String(), "SELECT * FROM missing_table") + assert.Contains(t, errBuffer.String(), "no such table: missing_table") + errBuffer.Reset() + + // GORM reports rows as -1 when a row count does not apply to the statement. + db.Trace(context.Background(), time.Now(), func() (string, int64) { + return "CREATE TABLE t (id int)", -1 + }, nil) + assert.Contains(t, outBuffer.String(), "[rows:-]") +} From 1dbe40982d207fbdbef571f65246303fc9172e14 Mon Sep 17 00:00:00 2001 From: Atishyy27 <142108881+Atishyy27@users.noreply.github.com> Date: Mon, 20 Jul 2026 22:15:44 +0530 Subject: [PATCH 2/2] address review: guard Trace against a nil receiver, base or fc gemini flagged that Trace could be called with a nil Database, a Database with a nil base, or a nil fc. GORM does not do this in practice, but the guard is one line and removes the possibility outright. Covered by TestDatabase_Trace_NilReceiverAndArgs, which panics without the guard. Signed-off-by: Atishyy27 <142108881+Atishyy27@users.noreply.github.com> --- logger/database.go | 3 +++ logger/database_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/logger/database.go b/logger/database.go index 6074f2bc6..333e5695f 100644 --- a/logger/database.go +++ b/logger/database.go @@ -45,6 +45,9 @@ func (c *Database) Error(ctx context.Context, msg string, data ...interface{}) { // time, the affected row count and any error to the underlying logger, matching the format used // by GORM's own logger. func (c *Database) Trace(ctx context.Context, begin time.Time, fc func() (string, int64), err error) { + if c == nil || c.base == nil || fc == nil { + return + } sql, rows := fc() elapsed := float64(time.Since(begin).Nanoseconds()) / 1e6 diff --git a/logger/database_test.go b/logger/database_test.go index cc216a58a..e4164f65e 100644 --- a/logger/database_test.go +++ b/logger/database_test.go @@ -49,3 +49,22 @@ func TestDatabase_Trace(t *testing.T) { }, nil) assert.Contains(t, outBuffer.String(), "[rows:-]") } + +func TestDatabase_Trace_NilReceiverAndArgs(t *testing.T) { + var nilDB *Database + assert.NotPanics(t, func() { + nilDB.Trace(context.Background(), time.Now(), func() (string, int64) { return "", 0 }, nil) + }) + + assert.NotPanics(t, func() { + (&Database{}).Trace(context.Background(), time.Now(), func() (string, int64) { return "", 0 }, nil) + }) + + opts := Options{Format: TerminalLogFormat, LogLevel: int(logrus.DebugLevel)} + log, err := New("testapp", opts) + assert.NoError(t, err) + db := log.(*Logger).DatabaseLogger() + assert.NotPanics(t, func() { + db.Trace(context.Background(), time.Now(), nil, nil) + }) +}