*: additional test for redact log and a fix (#52359)

close #52364
pingcap · Apr 7, 2024 · 8f985c7 · 8f985c7
1 parent d2eb902
commit 8f985c7
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 6 deletions.
diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go
@@ -263,7 +263,7 @@ func initFlagSet() *flag.FlagSet {
 func main() {
 	fset := initFlagSet()
 	if args := fset.Args(); len(args) != 0 {
-		if args[0] == "collect-logs" && len(args) > 1 {
+		if args[0] == "collect-log" && len(args) > 1 {
 			output := "-"
 			if len(args) > 2 {
 				output = args[2]

diff --git a/pkg/server/conn.go b/pkg/server/conn.go
@@ -102,7 +102,6 @@ import (
 	"github.com/pingcap/tidb/pkg/util/hack"
 	"github.com/pingcap/tidb/pkg/util/intest"
 	"github.com/pingcap/tidb/pkg/util/logutil"
-	"github.com/pingcap/tidb/pkg/util/redact"
 	"github.com/pingcap/tidb/pkg/util/resourcegrouptag"
 	tlsutil "github.com/pingcap/tidb/pkg/util/tls"
 	"github.com/pingcap/tidb/pkg/util/topsql"
@@ -1164,11 +1163,11 @@ func (cc *clientConn) Run(ctx context.Context) {
 }
 
 func errStrForLog(err error, redactMode string) string {
-	if redactMode == errors.RedactLogEnable {
+	if redactMode != errors.RedactLogDisable {
 		// currently, only ErrParse is considered when enableRedactLog because it may contain sensitive information like
 		// password or accesskey
 		if parser.ErrParse.Equal(err) {
-			return "fail to parse SQL and can't redact when enable log redaction"
+			return "fail to parse SQL, and must redact the whole error when enable log redaction"
 		}
 	}
 	var ret string
@@ -1178,7 +1177,7 @@ func errStrForLog(err error, redactMode string) string {
 	} else {
 		ret = errors.ErrorStack(err)
 	}
-	return redact.String(redactMode, ret)
+	return ret
 }
 
 func (cc *clientConn) addMetrics(cmd byte, startTime time.Time, err error) {

diff --git a/pkg/session/session.go b/pkg/session/session.go
@@ -735,7 +735,7 @@ func (s *session) handleAssertionFailure(ctx context.Context, err error) error {
 	}
 	if store, ok := s.store.(helper.Storage); ok {
 		content := consistency.GetMvccByKey(store, key, decodeFunc)
-		logutil.Logger(ctx).Error("assertion failed", zap.String("message", redact.String(rmode, newErr.Error())), zap.String("mvcc history", redact.String(rmode, content)))
+		logutil.Logger(ctx).Error("assertion failed", zap.String("message", newErr.Error()), zap.String("mvcc history", redact.String(rmode, content)))
 	}
 	return newErr
 }

diff --git a/pkg/table/tables/tables_test.go b/pkg/table/tables/tables_test.go
@@ -841,6 +841,14 @@ func TestTxnAssertion(t *testing.T) {
 			err = tk.ExecToErr("insert into t values (?, 10, 100, 1000, '10000')", id1)
 			expectAssertionErr(level, err)
 		})
+
+		tk.MustExec("set @@tidb_redact_log=MARKER")
+		withFailpoint(fpAdd, func() {
+			err = tk.ExecToErr("insert into t values (?, 10, 100, 1000, '10000')", id1)
+			require.Contains(t, err.Error(), "‹")
+		})
+		tk.MustExec("set @@tidb_redact_log=0")
+
 		withFailpoint(fpUpdate, func() {
 			err = tk.ExecToErr("update t set v = v + 1 where id = ?", id2)
 			expectAssertionErr(level, err)