From 8af3a7bfd8437bf81df0a61e66ccdbfa84572935 Mon Sep 17 00:00:00 2001
From: ti-srebot <66930949+ti-srebot@users.noreply.github.com>
Date: Fri, 13 Aug 2021 01:21:17 +0800
Subject: [PATCH] metrics: Add err label for TiFlashQueryTotalCounter (#25317)
 (#25327)

---
 executor/adapter.go | 10 +++++-----
 metrics/server.go   |  2 +-
 server/conn.go      |  2 +-
 session/session.go  |  2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/executor/adapter.go b/executor/adapter.go
index 066a1c81aeb97..573e046817d70 100644
--- a/executor/adapter.go
+++ b/executor/adapter.go
@@ -821,8 +821,7 @@ func FormatSQL(sql string, pps variable.PreparedParams) stringutil.StringerFunc
 var (
 	sessionExecuteRunDurationInternal = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblInternal)
 	sessionExecuteRunDurationGeneral  = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblGeneral)
-	totalTiFlashQueryFailCounter      = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblError)
-	totalTiFlashQuerySuccCounter      = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblOK)
+	totalTiFlashQuerySuccCounter      = metrics.TiFlashQueryTotalCounter.WithLabelValues("", metrics.LblOK)
 )
 
 // FinishExecuteStmt is used to record some information after `ExecStmt` execution finished:
@@ -830,7 +829,7 @@ var (
 // 2. record summary statement.
 // 3. record execute duration metric.
 // 4. update the `PrevStmt` in session variable.
-func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults bool) {
+func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults bool) {
 	sessVars := a.Ctx.GetSessionVars()
 	execDetail := sessVars.StmtCtx.GetExecDetails()
 	// Attach commit/lockKeys runtime stats to executor runtime stats.
@@ -849,6 +848,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo
 		// Only record the read keys in write statement which affect row more than 0.
 		a.Ctx.GetTxnWriteThroughputSLI().AddReadKeys(execDetail.ScanDetail.ProcessedKeys)
 	}
+	succ := err == nil
 	// `LowSlowQuery` and `SummaryStmt` must be called before recording `PrevStmt`.
 	a.LogSlowQuery(txnTS, succ, hasMoreResults)
 	a.SummaryStmt(succ)
@@ -856,7 +856,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo
 		if succ {
 			totalTiFlashQuerySuccCounter.Inc()
 		} else {
-			totalTiFlashQueryFailCounter.Inc()
+			metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc()
 		}
 	}
 	prevStmt := a.GetTextToLog()
@@ -877,7 +877,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo
 
 // CloseRecordSet will finish the execution of current statement and do some record work
 func (a *ExecStmt) CloseRecordSet(txnStartTS uint64, lastErr error) {
-	a.FinishExecuteStmt(txnStartTS, lastErr == nil, false)
+	a.FinishExecuteStmt(txnStartTS, lastErr, false)
 	a.logAudit()
 	// Detach the Memory and disk tracker for the previous stmtCtx from GlobalMemoryUsageTracker and GlobalDiskUsageTracker
 	if stmtCtx := a.Ctx.GetSessionVars().StmtCtx; stmtCtx != nil {
diff --git a/metrics/server.go b/metrics/server.go
index 3f01eec30bea8..f83c7afee2732 100644
--- a/metrics/server.go
+++ b/metrics/server.go
@@ -227,7 +227,7 @@ var (
 			Subsystem: "server",
 			Name:      "tiflash_query_total",
 			Help:      "Counter of TiFlash queries.",
-		}, []string{LblResult})
+		}, []string{LblType, LblResult})
 )
 
 // ExecuteErrorToLabel converts an execute error to label.
diff --git a/server/conn.go b/server/conn.go
index 7a674c57532dd..ffad5d84263b8 100644
--- a/server/conn.go
+++ b/server/conn.go
@@ -1662,7 +1662,7 @@ func (cc *clientConn) handleStmt(ctx context.Context, stmt ast.StmtNode, warns [
 		if handled {
 			execStmt := cc.ctx.Value(session.ExecStmtVarKey)
 			if execStmt != nil {
-				execStmt.(*executor.ExecStmt).FinishExecuteStmt(0, err == nil, false)
+				execStmt.(*executor.ExecStmt).FinishExecuteStmt(0, err, false)
 			}
 		}
 		if err != nil {
diff --git a/session/session.go b/session/session.go
index 9d520b3d735ce..da871b1179de9 100644
--- a/session/session.go
+++ b/session/session.go
@@ -1546,7 +1546,7 @@ func runStmt(ctx context.Context, se *session, s sqlexec.Statement) (rs sqlexec.
 	} else {
 		// If it is not a select statement or special query, we record its slow log here,
 		// then it could include the transaction commit time.
-		s.(*executor.ExecStmt).FinishExecuteStmt(origTxnCtx.StartTS, err == nil, false)
+		s.(*executor.ExecStmt).FinishExecuteStmt(origTxnCtx.StartTS, err, false)
 	}
 	return nil, err
 }