feat: added querylog-mode to log only errors if set to 'error'

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

go/flags/endtoend/vtcombo.txt

@@ -269,6 +269,7 @@ Flags:
       --querylog-buffer-size int                                         Maximum number of buffered query logs before throttling log output (default 10)
       --querylog-filter-tag string                                       string that must be present in the query for it to be logged; if using a value as the tag, you need to disable query normalization
       --querylog-format string                                           format for query logs ("text" or "json") (default "text")
+      --querylog-mode string                                             Mode for logging queries. 'error' will only log queries that return an error. Otherwise all queries will be logged.
       --querylog-row-threshold uint                                      Number of rows a query has to return or affect before being logged; not useful for streaming queries. 0 means all queries will be logged.
       --querylog-sample-rate float                                       Sample rate for logging queries. Value must be between 0.0 (no logging) and 1.0 (all queries)
       --queryserver-config-acl-exempt-acl string                         an acl that exempt from table acl checking (this acl is free to access any vitess tables).

go/flags/endtoend/vtgate.txt

@@ -177,6 +177,7 @@ Flags:
       --querylog-buffer-size int                                         Maximum number of buffered query logs before throttling log output (default 10)
       --querylog-filter-tag string                                       string that must be present in the query for it to be logged; if using a value as the tag, you need to disable query normalization
       --querylog-format string                                           format for query logs ("text" or "json") (default "text")
+      --querylog-mode string                                             Mode for logging queries. 'error' will only log queries that return an error. Otherwise all queries will be logged.
       --querylog-row-threshold uint                                      Number of rows a query has to return or affect before being logged; not useful for streaming queries. 0 means all queries will be logged.
       --querylog-sample-rate float                                       Sample rate for logging queries. Value must be between 0.0 (no logging) and 1.0 (all queries)
       --redact-debug-ui-queries                                          redact full queries and bind variables from debug UI

go/flags/endtoend/vttablet.txt

@@ -261,6 +261,7 @@ Flags:
       --query-log-stream-handler string                                  URL handler for streaming queries log (default "/debug/querylog")
       --querylog-filter-tag string                                       string that must be present in the query for it to be logged; if using a value as the tag, you need to disable query normalization
       --querylog-format string                                           format for query logs ("text" or "json") (default "text")
+      --querylog-mode string                                             Mode for logging queries. 'error' will only log queries that return an error. Otherwise all queries will be logged.
       --querylog-row-threshold uint                                      Number of rows a query has to return or affect before being logged; not useful for streaming queries. 0 means all queries will be logged.
       --querylog-sample-rate float                                       Sample rate for logging queries. Value must be between 0.0 (no logging) and 1.0 (all queries)
       --queryserver-config-acl-exempt-acl string                         an acl that exempt from table acl checking (this acl is free to access any vitess tables).

go/streamlog/streamlog.go

@@ -47,12 +47,15 @@ var (
 		[]string{"Log", "Subscriber"})
 )
 
+const QueryLogModeError = "error"
+
 var (
 	redactDebugUIQueries bool
 	queryLogFilterTag    string
 	queryLogRowThreshold uint64
 	queryLogFormat       = "text"
 	queryLogSampleRate   float64
+	queryLogMode         string
 )
 
 func GetRedactDebugUIQueries() bool {
@@ -83,6 +86,14 @@ func SetQueryLogFormat(newQueryLogFormat string) {
 	queryLogFormat = newQueryLogFormat
 }
 
+func GetQueryLogMode() string {
+	return queryLogMode
+}
+
+func SetQueryLogMode(logMode string) {
+	queryLogMode = logMode
+}
+
 func init() {
 	servenv.OnParseFor("vtcombo", registerStreamLogFlags)
 	servenv.OnParseFor("vttablet", registerStreamLogFlags)
@@ -104,6 +115,9 @@ func registerStreamLogFlags(fs *pflag.FlagSet) {
 
 	// QueryLogSampleRate causes a sample of queries to be logged
 	fs.Float64Var(&queryLogSampleRate, "querylog-sample-rate", queryLogSampleRate, "Sample rate for logging queries. Value must be between 0.0 (no logging) and 1.0 (all queries)")
+
+	// QueryLogMode controls the mode for logging queries (all or error)
+	fs.StringVar(&queryLogMode, "querylog-mode", queryLogMode, `Mode for logging queries. 'error' will only log queries that return an error. Otherwise all queries will be logged.`)
 }
 
 const (
@@ -269,7 +283,7 @@ func shouldSampleQuery() bool {
 
 // ShouldEmitLog returns whether the log with the given SQL query
 // should be emitted or filtered
-func ShouldEmitLog(sql string, rowsAffected, rowsReturned uint64) bool {
+func ShouldEmitLog(sql string, rowsAffected, rowsReturned uint64, hasError bool) bool {
 	if shouldSampleQuery() {
 		return true
 	}
@@ -279,5 +293,8 @@ func ShouldEmitLog(sql string, rowsAffected, rowsReturned uint64) bool {
 	if queryLogFilterTag != "" {
 		return strings.Contains(sql, queryLogFilterTag)
 	}
+	if queryLogMode == QueryLogModeError {
+		return hasError
+	}
 	return true
 }

go/streamlog/streamlog_test.go

@@ -287,19 +287,23 @@ func TestShouldEmitLog(t *testing.T) {
 	origQueryLogFilterTag := queryLogFilterTag
 	origQueryLogRowThreshold := queryLogRowThreshold
 	origQueryLogSampleRate := queryLogSampleRate
+	origQueryLogMode := queryLogMode
 	defer func() {
 		SetQueryLogFilterTag(origQueryLogFilterTag)
 		SetQueryLogRowThreshold(origQueryLogRowThreshold)
 		SetQueryLogSampleRate(origQueryLogSampleRate)
+		SetQueryLogMode(origQueryLogMode)
 	}()
 
 	tests := []struct {
 		sql              string
 		qLogFilterTag    string
 		qLogRowThreshold uint64
 		qLogSampleRate   float64
+		qLogMode         string
 		rowsAffected     uint64
 		rowsReturned     uint64
+		errored          bool
 		ok               bool
 	}{
 		{
@@ -356,15 +360,28 @@ func TestShouldEmitLog(t *testing.T) {
 			rowsReturned:     17,
 			ok:               true,
 		},
+		{
+			sql:      "log only error - no error",
+			qLogMode: "error",
+			errored:  false,
+			ok:       false,
+		},
+		{
+			sql:      "log only error - errored",
+			qLogMode: "error",
+			errored:  true,
+			ok:       true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.sql, func(t *testing.T) {
 			SetQueryLogFilterTag(tt.qLogFilterTag)
 			SetQueryLogRowThreshold(tt.qLogRowThreshold)
 			SetQueryLogSampleRate(tt.qLogSampleRate)
+			SetQueryLogMode(tt.qLogMode)
 
-			require.Equal(t, tt.ok, ShouldEmitLog(tt.sql, tt.rowsAffected, tt.rowsReturned))
+			require.Equal(t, tt.ok, ShouldEmitLog(tt.sql, tt.rowsAffected, tt.rowsReturned, tt.errored))
 		})
 	}
 }
@@ -373,22 +390,22 @@ func BenchmarkShouldEmitLog(b *testing.B) {
 	b.Run("default", func(b *testing.B) {
 		SetQueryLogSampleRate(0.0)
 		for i := 0; i < b.N; i++ {
-			ShouldEmitLog("select * from test where user='someone'", 0, 123)
+			ShouldEmitLog("select * from test where user='someone'", 0, 123, false)
 		}
 	})
 	b.Run("filter_tag", func(b *testing.B) {
 		SetQueryLogSampleRate(0.0)
 		SetQueryLogFilterTag("LOG_QUERY")
 		defer SetQueryLogFilterTag("")
 		for i := 0; i < b.N; i++ {
-			ShouldEmitLog("select /* LOG_QUERY=1 */ * from test where user='someone'", 0, 123)
+			ShouldEmitLog("select /* LOG_QUERY=1 */ * from test where user='someone'", 0, 123, false)
 		}
 	})
 	b.Run("50%_sample_rate", func(b *testing.B) {
 		SetQueryLogSampleRate(0.5)
 		defer SetQueryLogSampleRate(0.0)
 		for i := 0; i < b.N; i++ {
-			ShouldEmitLog("select * from test where user='someone'", 0, 123)
+			ShouldEmitLog("select * from test where user='someone'", 0, 123, false)
 		}
 	})
 }

go/vt/vtgate/logstats/logstats.go

@@ -130,7 +130,7 @@ func (stats *LogStats) MirrorTargetErrorStr() string {
 // Logf formats the log record to the given writer, either as
 // tab-separated list of logged fields or as JSON.
 func (stats *LogStats) Logf(w io.Writer, params url.Values) error {
-	if !streamlog.ShouldEmitLog(stats.SQL, stats.RowsAffected, stats.RowsReturned) {
+	if !streamlog.ShouldEmitLog(stats.SQL, stats.RowsAffected, stats.RowsReturned, stats.Error != nil) {
 		return nil
 	}
 

go/vt/vtgate/logstats/logstats_test.go

@@ -253,3 +253,21 @@ func TestLogStatsRemoteAddrUsername(t *testing.T) {
 		t.Fatalf("expected to get username: %s, but got: %s", username, user)
 	}
 }
+
+// TestLogStatsErrorsOnly tests that LogStats only logs errors when the query log mode is set to errors only for VTGate.
+func TestLogStatsErrorsOnly(t *testing.T) {
+	origQLogMode := streamlog.GetQueryLogMode()
+	streamlog.SetQueryLogMode(streamlog.QueryLogModeError)
+	defer streamlog.SetQueryLogMode(origQLogMode)
+
+	logStats := NewLogStats(context.Background(), "test", "sql1", "", map[string]*querypb.BindVariable{})
+
+	// no error, should not log
+	logOutput := testFormat(t, logStats, url.Values{})
+	assert.Empty(t, logOutput)
+
+	// error, should log
+	logStats.Error = errors.New("test error")
+	logOutput = testFormat(t, logStats, url.Values{})
+	assert.Contains(t, logOutput, "test error")
+}

go/vt/vttablet/tabletserver/tabletenv/logstats.go

@@ -177,7 +177,7 @@ func (stats *LogStats) CallInfo() (string, string) {
 // Logf formats the log record to the given writer, either as
 // tab-separated list of logged fields or as JSON.
 func (stats *LogStats) Logf(w io.Writer, params url.Values) error {
-	if !streamlog.ShouldEmitLog(stats.OriginalSQL, uint64(stats.RowsAffected), uint64(len(stats.Rows))) {
+	if !streamlog.ShouldEmitLog(stats.OriginalSQL, uint64(stats.RowsAffected), uint64(len(stats.Rows)), stats.Error != nil) {
 		return nil
 	}
 

go/vt/vttablet/tabletserver/tabletenv/logstats_test.go

@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"github.com/google/safehtml/testconversions"
+	"github.com/stretchr/testify/assert"
 
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/streamlog"
@@ -253,3 +254,21 @@ func TestLogStatsCallInfo(t *testing.T) {
 		t.Fatalf("expected to get username: %s, but got: %s", username, user)
 	}
 }
+
+// TestLogStatsErrorsOnly tests that LogStats only logs errors when the query log mode is set to errors only for VTTablet.
+func TestLogStatsErrorsOnly(t *testing.T) {
+	origQLogMode := streamlog.GetQueryLogMode()
+	streamlog.SetQueryLogMode(streamlog.QueryLogModeError)
+	defer streamlog.SetQueryLogMode(origQLogMode)
+
+	logStats := NewLogStats(context.Background(), "test")
+
+	// no error, should not log
+	logOutput := testFormat(logStats, url.Values{})
+	assert.Empty(t, logOutput)
+
+	// error, should log
+	logStats.Error = errors.New("test error")
+	logOutput = testFormat(logStats, url.Values{})
+	assert.Contains(t, logOutput, "test error")
+}