[cli] [tabletserver/streamlog] Migrate streamlog flags to pflags (vit…

…essio#11125)

* Update streamlog to replace flags with pflags

Signed-off-by: Rohit Nayak <[email protected]>

* Address review comment: export streamlog flag vars via getter/setter

Signed-off-by: Rohit Nayak <[email protected]>

Signed-off-by: Rohit Nayak <[email protected]>

go/flags/endtoend/vtctld.txt

@@ -132,9 +132,6 @@ Usage of vtctld:
       --proxy_tablets                                                    Setting this true will make vtctld proxy the tablet status instead of redirecting to them
       --purge_logs_interval duration                                     how often try to remove old logs (default 1h0m0s)
       --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-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.
       --queryserver-config-acl-exempt-acl string                         an acl that exempt from table acl checking (this acl is free to access any vitess tables).
       --queryserver-config-annotate-queries                              prefix queries to MySQL backend with comment indicating vtgate principal (user) and target tablet type
       --queryserver-config-enable-table-acl-dry-run                      If this flag is enabled, tabletserver will emit monitoring metrics and let the request pass regardless of table acl check results
@@ -167,7 +164,6 @@ Usage of vtctld:
       --queryserver-config-txpool-waiter-cap int                         query server transaction pool waiter limit, this is the maximum number of transactions that can be queued waiting to get a connection (default 5000)
       --queryserver-config-warn-result-size int                          query server result size warning threshold, warn if number of rows returned from vttablet for non-streaming queries exceeds this
       --queryserver_enable_online_ddl                                    Enable online DDL. (default true)
-      --redact-debug-ui-queries                                          redact full queries and bind variables from debug UI
       --remote_operation_timeout duration                                time to wait for a remote operation (default 30s)
       --replication_connect_retry duration                               how long to wait in between replica reconnect attempts. Only precise to the second. (default 10s)
       --s3_backup_aws_endpoint string                                    endpoint of the S3 backend (region must be provided).

go/flags/endtoend/vtexplain.txt

@@ -149,9 +149,6 @@ Usage of vtexplain:
       --purge_logs_interval duration                                     how often try to remove old logs (default 1h0m0s)
       --query-log-stream-handler string                                  URL handler for streaming queries log (default "/debug/querylog")
       --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-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.
       --queryserver-config-acl-exempt-acl string                         an acl that exempt from table acl checking (this acl is free to access any vitess tables).
       --queryserver-config-annotate-queries                              prefix queries to MySQL backend with comment indicating vtgate principal (user) and target tablet type
       --queryserver-config-enable-table-acl-dry-run                      If this flag is enabled, tabletserver will emit monitoring metrics and let the request pass regardless of table acl check results
@@ -184,7 +181,6 @@ Usage of vtexplain:
       --queryserver-config-txpool-waiter-cap int                         query server transaction pool waiter limit, this is the maximum number of transactions that can be queued waiting to get a connection (default 5000)
       --queryserver-config-warn-result-size int                          query server result size warning threshold, warn if number of rows returned from vttablet for non-streaming queries exceeds this
       --queryserver_enable_online_ddl                                    Enable online DDL. (default true)
-      --redact-debug-ui-queries                                          redact full queries and bind variables from debug UI
       --remote_operation_timeout duration                                time to wait for a remote operation (default 30s)
       --replication-mode string                                          The replication mode to simulate -- must be set to either ROW or STATEMENT (default "ROW")
       --replication_connect_retry duration                               how long to wait in between replica reconnect attempts. Only precise to the second. (default 10s)

go/streamlog/streamlog.go

@@ -18,7 +18,6 @@ limitations under the License.
 package streamlog
 
 import (
-	"flag"
 	"fmt"
 	"io"
 	"net/http"
@@ -29,24 +28,15 @@ import (
 	"sync"
 	"syscall"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/stats"
 	"vitess.io/vitess/go/vt/log"
+	"vitess.io/vitess/go/vt/servenv"
 )
 
 var (
-	// RedactDebugUIQueries controls whether full queries and bind variables are suppressed from debug UIs.
-	RedactDebugUIQueries = flag.Bool("redact-debug-ui-queries", false, "redact full queries and bind variables from debug UI")
-
-	// QueryLogFormat controls the format of the query log (either text or json)
-	QueryLogFormat = flag.String("querylog-format", "text", "format for query logs (\"text\" or \"json\")")
-
-	// QueryLogFilterTag contains an optional string that must be present in the query for it to be logged
-	QueryLogFilterTag = flag.String("querylog-filter-tag", "", "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")
-
-	// QueryLogRowThreshold only log queries returning or affecting this many rows
-	QueryLogRowThreshold = flag.Uint64("querylog-row-threshold", 0, "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.")
-
 	sendCount      = stats.NewCountersWithSingleLabel("StreamlogSend", "stream log send count", "logger_names")
 	deliveredCount = stats.NewCountersWithMultiLabels(
 		"StreamlogDelivered",
@@ -58,6 +48,66 @@ var (
 		[]string{"Log", "Subscriber"})
 )
 
+var (
+	redactDebugUIQueries bool
+	queryLogFilterTag    string
+	queryLogRowThreshold uint64
+	queryLogFormat       = "text"
+)
+
+func GetRedactDebugUIQueries() bool {
+	return redactDebugUIQueries
+}
+
+func SetRedactDebugUIQueries(newRedactDebugUIQueries bool) {
+	redactDebugUIQueries = newRedactDebugUIQueries
+}
+
+func GetQueryLogFilterTag() string {
+	return queryLogFilterTag
+}
+
+func SetQueryLogFilterTag(newQueryLogFilterTag string) {
+	queryLogFilterTag = newQueryLogFilterTag
+}
+
+func GetQueryLogRowThreshold() uint64 {
+	return queryLogRowThreshold
+}
+
+func SetQueryLogRowThreshold(newQueryLogRowThreshold uint64) {
+	queryLogRowThreshold = newQueryLogRowThreshold
+}
+
+func GetQueryLogFormat() string {
+	return queryLogFormat
+}
+
+func SetQueryLogFormat(newQueryLogFormat string) {
+	queryLogFormat = newQueryLogFormat
+}
+
+func init() {
+	servenv.OnParseFor("vtcombo", registerStreamLogFlags)
+	servenv.OnParseFor("vttablet", registerStreamLogFlags)
+	servenv.OnParseFor("vtgate", registerStreamLogFlags)
+}
+
+func registerStreamLogFlags(fs *pflag.FlagSet) {
+	// RedactDebugUIQueries controls whether full queries and bind variables are suppressed from debug UIs.
+	fs.BoolVar(&redactDebugUIQueries, "redact-debug-ui-queries", redactDebugUIQueries, "redact full queries and bind variables from debug UI")
+
+	// QueryLogFormat controls the format of the query log (either text or json)
+	fs.StringVar(&queryLogFormat, "querylog-format", queryLogFormat, "format for query logs (\"text\" or \"json\")")
+
+	// QueryLogFilterTag contains an optional string that must be present in the query for it to be logged
+	fs.StringVar(&queryLogFilterTag, "querylog-filter-tag", queryLogFilterTag, "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")
+
+	// QueryLogRowThreshold only log queries returning or affecting this many rows
+	fs.Uint64Var(&queryLogRowThreshold, "querylog-row-threshold", queryLogRowThreshold, "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.")
+
+}
+
 const (
 	// QueryLogFormatText is the format specifier for text querylog output
 	QueryLogFormatText = "text"
@@ -212,11 +262,11 @@ func GetFormatter(logger *StreamLogger) LogFormatter {
 // ShouldEmitLog returns whether the log with the given SQL query
 // should be emitted or filtered
 func ShouldEmitLog(sql string, rowsAffected, rowsReturned uint64) bool {
-	if *QueryLogRowThreshold > maxUint64(rowsAffected, rowsReturned) && *QueryLogFilterTag == "" {
+	if queryLogRowThreshold > maxUint64(rowsAffected, rowsReturned) && queryLogFilterTag == "" {
 		return false
 	}
-	if *QueryLogFilterTag != "" {
-		return strings.Contains(sql, *QueryLogFilterTag)
+	if queryLogFilterTag != "" {
+		return strings.Contains(sql, queryLogFilterTag)
 	}
 	return true
 }

go/vt/vtgate/logstats/logstats.go

@@ -141,20 +141,20 @@ func (stats *LogStats) Logf(w io.Writer, params url.Values) error {
 	}()
 
 	formattedBindVars := "\"[REDACTED]\""
-	if !*streamlog.RedactDebugUIQueries {
+	if !streamlog.GetRedactDebugUIQueries() {
 		_, fullBindParams := params["full"]
 		formattedBindVars = sqltypes.FormatBindVariables(
 			stats.BindVariables,
 			fullBindParams,
-			*streamlog.QueryLogFormat == streamlog.QueryLogFormatJSON,
+			streamlog.GetQueryLogFormat() == streamlog.QueryLogFormatJSON,
 		)
 	}
 
 	// TODO: remove username here we fully enforce immediate caller id
 	remoteAddr, username := stats.RemoteAddrUsername()
 
 	var fmtString string
-	switch *streamlog.QueryLogFormat {
+	switch streamlog.GetQueryLogFormat() {
 	case streamlog.QueryLogFormatText:
 		fmtString = "%v\t%v\t%v\t'%v'\t'%v'\t%v\t%v\t%.6f\t%.6f\t%.6f\t%.6f\t%v\t%q\t%v\t%v\t%v\t%q\t%q\t%q\t%q\t%q\t%v\t%v\t%q\n"
 	case streamlog.QueryLogFormatJSON:

go/vt/vtgate/logstats/logstats_test.go

@@ -55,8 +55,8 @@ func testFormat(t *testing.T, stats *LogStats, params url.Values) string {
 
 func TestLogStatsFormat(t *testing.T) {
 	defer func() {
-		*streamlog.RedactDebugUIQueries = false
-		*streamlog.QueryLogFormat = "text"
+		streamlog.SetRedactDebugUIQueries(false)
+		streamlog.SetQueryLogFormat("text")
 	}()
 	logStats := NewLogStats(context.Background(), "test", "sql1", "suuid", nil)
 	logStats.StartTime = time.Date(2017, time.January, 1, 1, 2, 3, 0, time.UTC)
@@ -126,8 +126,8 @@ func TestLogStatsFormat(t *testing.T) {
 			for _, variable := range logStats.BindVariables {
 				fmt.Println("->" + fmt.Sprintf("%v", variable))
 			}
-			*streamlog.RedactDebugUIQueries = test.redact
-			*streamlog.QueryLogFormat = test.format
+			streamlog.SetRedactDebugUIQueries(test.redact)
+			streamlog.SetQueryLogFormat(test.format)
 			if test.format == "text" {
 				got := testFormat(t, logStats, params)
 				assert.Equal(t, test.expected, got)
@@ -150,7 +150,7 @@ func TestLogStatsFormat(t *testing.T) {
 }
 
 func TestLogStatsFilter(t *testing.T) {
-	defer func() { *streamlog.QueryLogFilterTag = "" }()
+	defer func() { streamlog.SetQueryLogFilterTag("") }()
 
 	logStats := NewLogStats(context.Background(), "test", "sql1 /* LOG_THIS_QUERY */", "", map[string]*querypb.BindVariable{"intVal": sqltypes.Int64BindVariable(1)})
 	logStats.StartTime = time.Date(2017, time.January, 1, 1, 2, 3, 0, time.UTC)
@@ -161,19 +161,19 @@ func TestLogStatsFilter(t *testing.T) {
 	want := "test\t\t\t''\t''\t2017-01-01 01:02:03.000000\t2017-01-01 01:02:04.000001\t1.000001\t0.000000\t0.000000\t0.000000\t\t\"sql1 /* LOG_THIS_QUERY */\"\tmap[intVal:type:INT64 value:\"1\"]\t0\t0\t\"\"\t\"\"\t\"\"\t\"\"\t\"\"\tfalse\t[]\t\"\"\n"
 	assert.Equal(t, want, got)
 
-	*streamlog.QueryLogFilterTag = "LOG_THIS_QUERY"
+	streamlog.SetQueryLogFilterTag("LOG_THIS_QUERY")
 	got = testFormat(t, logStats, params)
 	want = "test\t\t\t''\t''\t2017-01-01 01:02:03.000000\t2017-01-01 01:02:04.000001\t1.000001\t0.000000\t0.000000\t0.000000\t\t\"sql1 /* LOG_THIS_QUERY */\"\tmap[intVal:type:INT64 value:\"1\"]\t0\t0\t\"\"\t\"\"\t\"\"\t\"\"\t\"\"\tfalse\t[]\t\"\"\n"
 	assert.Equal(t, want, got)
 
-	*streamlog.QueryLogFilterTag = "NOT_THIS_QUERY"
+	streamlog.SetQueryLogFilterTag("NOT_THIS_QUERY")
 	got = testFormat(t, logStats, params)
 	want = ""
 	assert.Equal(t, want, got)
 }
 
 func TestLogStatsRowThreshold(t *testing.T) {
-	defer func() { *streamlog.QueryLogRowThreshold = 0 }()
+	defer func() { streamlog.SetQueryLogRowThreshold(0) }()
 
 	logStats := NewLogStats(context.Background(), "test", "sql1 /* LOG_THIS_QUERY */", "", map[string]*querypb.BindVariable{"intVal": sqltypes.Int64BindVariable(1)})
 	logStats.StartTime = time.Date(2017, time.January, 1, 1, 2, 3, 0, time.UTC)
@@ -184,11 +184,11 @@ func TestLogStatsRowThreshold(t *testing.T) {
 	want := "test\t\t\t''\t''\t2017-01-01 01:02:03.000000\t2017-01-01 01:02:04.000001\t1.000001\t0.000000\t0.000000\t0.000000\t\t\"sql1 /* LOG_THIS_QUERY */\"\tmap[intVal:type:INT64 value:\"1\"]\t0\t0\t\"\"\t\"\"\t\"\"\t\"\"\t\"\"\tfalse\t[]\t\"\"\n"
 	assert.Equal(t, want, got)
 
-	*streamlog.QueryLogRowThreshold = 0
+	streamlog.SetQueryLogRowThreshold(0)
 	got = testFormat(t, logStats, params)
 	want = "test\t\t\t''\t''\t2017-01-01 01:02:03.000000\t2017-01-01 01:02:04.000001\t1.000001\t0.000000\t0.000000\t0.000000\t\t\"sql1 /* LOG_THIS_QUERY */\"\tmap[intVal:type:INT64 value:\"1\"]\t0\t0\t\"\"\t\"\"\t\"\"\t\"\"\t\"\"\tfalse\t[]\t\"\"\n"
 	assert.Equal(t, want, got)
-	*streamlog.QueryLogRowThreshold = 1
+	streamlog.SetQueryLogRowThreshold(1)
 	got = testFormat(t, logStats, params)
 	assert.Empty(t, got)
 }

go/vt/vtgate/querylogz_test.go

@@ -151,8 +151,8 @@ func TestQuerylogzHandlerFormatting(t *testing.T) {
 	checkQuerylogzHasStats(t, slowQueryPattern, logStats, body)
 
 	// ensure querylogz is not affected by the filter tag
-	*streamlog.QueryLogFilterTag = "XXX_SKIP_ME"
-	defer func() { *streamlog.QueryLogFilterTag = "" }()
+	streamlog.SetQueryLogFilterTag("XXX_SKIP_ME")
+	defer func() { streamlog.SetQueryLogFilterTag("") }()
 	ch = make(chan any, 1)
 	ch <- logStats
 	querylogzHandler(ch, response, req)

go/vt/vttablet/filelogger/filelogger_test.go

@@ -75,9 +75,9 @@ func TestFileLog(t *testing.T) {
 
 // TestFileLog sends a stream of five query records to the plugin, and verifies that they are logged.
 func TestFileLogRedacted(t *testing.T) {
-	*streamlog.RedactDebugUIQueries = true
+	streamlog.SetRedactDebugUIQueries(true)
 	defer func() {
-		*streamlog.RedactDebugUIQueries = false
+		streamlog.SetRedactDebugUIQueries(false)
 	}()
 
 	dir := t.TempDir()

go/vt/vttablet/sysloglogger/sysloglogger_test.go

@@ -141,9 +141,9 @@ func TestSyslog(t *testing.T) {
 // when redaction is enabled.
 func TestSyslogRedacted(t *testing.T) {
 	// Overwrite the usual syslog writer and StatsLogger subscription channel with mocks
-	*streamlog.RedactDebugUIQueries = true
+	streamlog.SetRedactDebugUIQueries(true)
 	defer func() {
-		*streamlog.RedactDebugUIQueries = false
+		streamlog.SetRedactDebugUIQueries(false)
 	}()
 	mock := newFakeWriter()
 	writer = mock

go/vt/vttablet/tabletserver/query_engine.go

@@ -548,7 +548,7 @@ func (qe *QueryEngine) handleHTTPConsolidations(response http.ResponseWriter, re
 	response.Write([]byte(fmt.Sprintf("Length: %d\n", len(items))))
 	for _, v := range items {
 		var query string
-		if *streamlog.RedactDebugUIQueries {
+		if streamlog.GetRedactDebugUIQueries() {
 			query, _ = sqlparser.RedactSQLQuery(v.Query)
 		} else {
 			query = v.Query

go/vt/vttablet/tabletserver/query_engine_test.go

@@ -340,7 +340,7 @@ func runConsolidatedQuery(t *testing.T, sql string) *QueryEngine {
 func TestConsolidationsUIRedaction(t *testing.T) {
 	// Reset to default redaction state.
 	defer func() {
-		*streamlog.RedactDebugUIQueries = false
+		streamlog.SetRedactDebugUIQueries(false)
 	}()
 
 	request, _ := http.NewRequest("GET", "/debug/consolidations", nil)
@@ -349,7 +349,7 @@ func TestConsolidationsUIRedaction(t *testing.T) {
 	redactedSQL := "select * from test_db_01 where col = :redacted1"
 
 	// First with the redaction off
-	*streamlog.RedactDebugUIQueries = false
+	streamlog.SetRedactDebugUIQueries(false)
 	unRedactedResponse := httptest.NewRecorder()
 	qe := runConsolidatedQuery(t, sql)
 
@@ -359,7 +359,7 @@ func TestConsolidationsUIRedaction(t *testing.T) {
 	}
 
 	// Now with the redaction on
-	*streamlog.RedactDebugUIQueries = true
+	streamlog.SetRedactDebugUIQueries(true)
 	redactedResponse := httptest.NewRecorder()
 	qe.handleHTTPConsolidations(redactedResponse, request)
 

go/vt/vttablet/tabletserver/query_list.go

@@ -122,7 +122,7 @@ func (ql *QueryList) AppendQueryzRows(rows []QueryDetailzRow) []QueryDetailzRow
 	ql.mu.Lock()
 	for _, qd := range ql.queryDetails {
 		query := qd.conn.Current()
-		if *streamlog.RedactDebugUIQueries {
+		if streamlog.GetRedactDebugUIQueries() {
 			query, _ = sqlparser.RedactSQLQuery(query)
 		}
 		row := QueryDetailzRow{

go/vt/vttablet/tabletserver/querylogz_test.go

@@ -156,8 +156,8 @@ func TestQuerylogzHandler(t *testing.T) {
 	checkQuerylogzHasStats(t, slowQueryPattern, logStats, body)
 
 	// ensure querylogz is not affected by the filter tag
-	*streamlog.QueryLogFilterTag = "XXX_SKIP_ME"
-	defer func() { *streamlog.QueryLogFilterTag = "" }()
+	streamlog.SetQueryLogFilterTag("XXX_SKIP_ME")
+	defer func() { streamlog.SetQueryLogFilterTag("") }()
 	ch = make(chan any, 1)
 	ch <- logStats
 	querylogzHandler(ch, response, req)

go/vt/vttablet/tabletserver/tabletenv/config.go

@@ -208,11 +208,11 @@ func Init() {
 	currentConfig.Healthcheck.UnhealthyThresholdSeconds.Set(unhealthyThreshold)
 	currentConfig.GracePeriods.TransitionSeconds.Set(transitionGracePeriod)
 
-	switch *streamlog.QueryLogFormat {
+	switch streamlog.GetQueryLogFormat() {
 	case streamlog.QueryLogFormatText:
 	case streamlog.QueryLogFormatJSON:
 	default:
-		log.Exitf("Invalid querylog-format value %v: must be either text or json", *streamlog.QueryLogFormat)
+		log.Exitf("Invalid querylog-format value %v: must be either text or json", streamlog.GetQueryLogFormat())
 	}
 
 	if *queryLogHandler != "" {

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

@@ -184,14 +184,14 @@ func (stats *LogStats) Logf(w io.Writer, params url.Values) error {
 	rewrittenSQL := "[REDACTED]"
 	formattedBindVars := "\"[REDACTED]\""
 
-	if !*streamlog.RedactDebugUIQueries {
+	if !streamlog.GetRedactDebugUIQueries() {
 		rewrittenSQL = stats.RewrittenSQL()
 
 		_, fullBindParams := params["full"]
 		formattedBindVars = sqltypes.FormatBindVariables(
 			stats.BindVariables,
 			fullBindParams,
-			*streamlog.QueryLogFormat == streamlog.QueryLogFormatJSON,
+			streamlog.GetQueryLogFormat() == streamlog.QueryLogFormatJSON,
 		)
 	}
 
@@ -200,7 +200,7 @@ func (stats *LogStats) Logf(w io.Writer, params url.Values) error {
 
 	// Valid options for the QueryLogFormat are text or json
 	var fmtString string
-	switch *streamlog.QueryLogFormat {
+	switch streamlog.GetQueryLogFormat() {
 	case streamlog.QueryLogFormatText:
 		fmtString = "%v\t%v\t%v\t'%v'\t'%v'\t%v\t%v\t%.6f\t%v\t%q\t%v\t%v\t%q\t%v\t%.6f\t%.6f\t%v\t%v\t%v\t%q\t\n"
 	case streamlog.QueryLogFormatJSON: