diff --git a/go/flags/endtoend/vtcombo.txt b/go/flags/endtoend/vtcombo.txt index 052c19ecaae..b6786b29487 100644 --- a/go/flags/endtoend/vtcombo.txt +++ b/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). diff --git a/go/flags/endtoend/vtgate.txt b/go/flags/endtoend/vtgate.txt index fde17f89c49..0773190187b 100644 --- a/go/flags/endtoend/vtgate.txt +++ b/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 diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt index e2b0c30db7f..ab6b32ba4c3 100644 --- a/go/flags/endtoend/vttablet.txt +++ b/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). diff --git a/go/streamlog/streamlog.go b/go/streamlog/streamlog.go index 40d33b840b4..2b679602b10 100644 --- a/go/streamlog/streamlog.go +++ b/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 } diff --git a/go/streamlog/streamlog_test.go b/go/streamlog/streamlog_test.go index c1321c92a94..88646589924 100644 --- a/go/streamlog/streamlog_test.go +++ b/go/streamlog/streamlog_test.go @@ -287,10 +287,12 @@ 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 { @@ -298,8 +300,10 @@ func TestShouldEmitLog(t *testing.T) { qLogFilterTag string qLogRowThreshold uint64 qLogSampleRate float64 + qLogMode string rowsAffected uint64 rowsReturned uint64 + errored bool ok bool }{ { @@ -356,6 +360,18 @@ 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 { @@ -363,8 +379,9 @@ func TestShouldEmitLog(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,7 +390,7 @@ 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) { @@ -381,14 +398,14 @@ func BenchmarkShouldEmitLog(b *testing.B) { 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) } }) } diff --git a/go/vt/vtgate/logstats/logstats.go b/go/vt/vtgate/logstats/logstats.go index fdc0e69c8db..1e0722db38f 100644 --- a/go/vt/vtgate/logstats/logstats.go +++ b/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 } diff --git a/go/vt/vtgate/logstats/logstats_test.go b/go/vt/vtgate/logstats/logstats_test.go index 872b34c6964..95f9f702487 100644 --- a/go/vt/vtgate/logstats/logstats_test.go +++ b/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") +} diff --git a/go/vt/vttablet/tabletserver/tabletenv/logstats.go b/go/vt/vttablet/tabletserver/tabletenv/logstats.go index ad7e09de169..12da258ea57 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/logstats.go +++ b/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 } diff --git a/go/vt/vttablet/tabletserver/tabletenv/logstats_test.go b/go/vt/vttablet/tabletserver/tabletenv/logstats_test.go index 7412a0a436c..3a08b6eb678 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/logstats_test.go +++ b/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") +}