Merge pull request #108959 from jaylim-crl/backport23.1-108914

release-23.1: sqlproxyccl: do not report BackendDown metrics on throttle and routing errors

pkg/ccl/sqlproxyccl/backend_dialer.go

@@ -38,7 +38,7 @@ var BackendDial = func(
 	if err != nil {
 		return nil, withCode(
 			errors.Wrap(err, "unable to reach backend SQL server"),
-			codeBackendDown)
+			codeBackendDialFailed)
 	}
 
 	// Ensure that conn is closed whenever BackendDial returns an error.
@@ -54,18 +54,18 @@ var BackendDial = func(
 		if err := binary.Write(conn, binary.BigEndian, pgSSLRequest); err != nil {
 			return nil, withCode(
 				errors.Wrap(err, "sending SSLRequest to target server"),
-				codeBackendDown)
+				codeBackendDialFailed)
 		}
 		response := make([]byte, 1)
 		if _, err = io.ReadFull(conn, response); err != nil {
 			return nil, withCode(
 				errors.New("reading response to SSLRequest"),
-				codeBackendDown)
+				codeBackendDialFailed)
 		}
 		if response[0] != pgAcceptSSLRequest {
 			return nil, withCode(
 				errors.New("target server refused TLS connection"),
-				codeBackendRefusedTLS)
+				codeBackendDialFailed)
 		}
 		conn = tls.Client(conn, tlsConfig.Clone())
 	}
@@ -74,7 +74,7 @@ var BackendDial = func(
 	if _, err := conn.Write(msg.Encode(nil)); err != nil {
 		return nil, withCode(
 			errors.Wrapf(err, "relaying StartupMessage to target server %v", serverAddress),
-			codeBackendDown)
+			codeBackendDialFailed)
 	}
 	return conn, nil
 }

pkg/ccl/sqlproxyccl/backend_dialer_test.go

@@ -94,22 +94,22 @@ func TestBackendDialTLS(t *testing.T) {
 		name:     "tenant10To11",
 		addr:     sql11.SQLAddr(),
 		tenantID: 10,
-		errCode:  codeBackendDown,
+		errCode:  codeBackendDialFailed,
 	}, {
 		name:     "tenant11To10",
 		addr:     sql10.SQLAddr(),
 		tenantID: 11,
-		errCode:  codeBackendDown,
+		errCode:  codeBackendDialFailed,
 	}, {
 		name:     "tenant10ToStorage",
 		addr:     storageServer.ServingSQLAddr(),
 		tenantID: 10,
-		errCode:  codeBackendDown,
+		errCode:  codeBackendDialFailed,
 	}, {
 		name:     "tenantWithNodeIDToStoage",
 		addr:     storageServer.ServingSQLAddr(),
 		tenantID: uint64(storageServer.NodeID()),
-		errCode:  codeBackendDown,
+		errCode:  codeBackendDialFailed,
 	}}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

pkg/ccl/sqlproxyccl/connector.go

@@ -365,7 +365,7 @@ func (c *connector) dialSQLServer(
 
 	conn, err := BackendDial(c.StartupMsg, serverAssignment.Addr(), tlsConf)
 	if err != nil {
-		if getErrorCode(err) == codeBackendDown {
+		if getErrorCode(err) == codeBackendDialFailed {
 			return nil, markAsRetriableConnectorError(err)
 		}
 		return nil, err

pkg/ccl/sqlproxyccl/connector_test.go

@@ -888,14 +888,14 @@ func TestConnector_dialSQLServer(t *testing.T) {
 				require.Equal(t, c.StartupMsg, msg)
 				require.Equal(t, "127.0.0.2:4567", serverAddress)
 				require.Nil(t, tlsConfig)
-				return nil, withCode(errors.New("bar"), codeBackendDown)
+				return nil, withCode(errors.New("bar"), codeBackendDialFailed)
 			},
 		)()
 		sa := balancer.NewServerAssignment(tenantID, tracker, nil, "127.0.0.2:4567")
 		defer sa.Close()
 
 		conn, err := c.dialSQLServer(sa)
-		require.EqualError(t, err, "codeBackendDown: bar")
+		require.EqualError(t, err, "codeBackendDialFailed: bar")
 		require.True(t, isRetriableConnectorError(err))
 		require.Nil(t, conn)
 	})

pkg/ccl/sqlproxyccl/error.go

@@ -50,13 +50,9 @@ const (
 	// on the client's session parameters.
 	codeParamsRoutingFailed
 
-	// codeBackendDown indicates an error establishing or maintaining a connection
-	// to the backend SQL server.
-	codeBackendDown
-
-	// codeBackendRefusedTLS indicates that the backend SQL server refused a TLS-
-	// enabled SQL connection.
-	codeBackendRefusedTLS
+	// codeBackendDialFailed indicates an error establishing a connection
+	// between the proxy and the backend SQL server.
+	codeBackendDialFailed
 
 	// codeBackendDisconnected indicates that the backend disconnected (with a
 	// connection error) while serving client traffic.

pkg/ccl/sqlproxyccl/metrics.go

@@ -292,11 +292,13 @@ func (metrics *metrics) updateForError(err error) {
 		metrics.ClientDisconnectCount.Inc(1)
 	case codeProxyRefusedConnection:
 		metrics.RefusedConnCount.Inc(1)
-		metrics.BackendDownCount.Inc(1)
 	case codeParamsRoutingFailed, codeUnavailable:
 		metrics.RoutingErrCount.Inc(1)
-		metrics.BackendDownCount.Inc(1)
-	case codeBackendDown:
+	case codeBackendDialFailed:
+		// NOTE: Historically, we had the code named codeBackendDown instead of
+		// codeBackendDialFailed. This has been renamed to codeBackendDialFailed
+		// for accuracy, and to prevent confusion by developers. We don't rename
+		// the metrics here as that may break downstream consumers.
 		metrics.BackendDownCount.Inc(1)
 	case codeAuthFailed:
 		metrics.AuthFailedCount.Inc(1)

pkg/ccl/sqlproxyccl/metrics_test.go

@@ -35,12 +35,12 @@ func TestMetricsUpdateForError(t *testing.T) {
 
 		{codeExpiredClientConnection, []*metric.Counter{m.ExpiredClientConnCount}},
 
-		{codeProxyRefusedConnection, []*metric.Counter{m.RefusedConnCount, m.BackendDownCount}},
+		{codeProxyRefusedConnection, []*metric.Counter{m.RefusedConnCount}},
 
-		{codeParamsRoutingFailed, []*metric.Counter{m.RoutingErrCount, m.BackendDownCount}},
-		{codeUnavailable, []*metric.Counter{m.RoutingErrCount, m.BackendDownCount}},
+		{codeParamsRoutingFailed, []*metric.Counter{m.RoutingErrCount}},
+		{codeUnavailable, []*metric.Counter{m.RoutingErrCount}},
 
-		{codeBackendDown, []*metric.Counter{m.BackendDownCount}},
+		{codeBackendDialFailed, []*metric.Counter{m.BackendDownCount}},
 
 		{codeAuthFailed, []*metric.Counter{m.AuthFailedCount}},
 	}

pkg/ccl/sqlproxyccl/proxy.go

@@ -34,7 +34,7 @@ func toPgError(err error) *pgproto3.ErrorResponse {
 		switch getErrorCode(err) {
 		// These are send as is.
 		case codeExpiredClientConnection,
-			codeBackendDown,
+			codeBackendDialFailed,
 			codeParamsRoutingFailed,
 			codeClientDisconnected,
 			codeBackendDisconnected,

pkg/ccl/sqlproxyccl/proxy_handler_test.go

@@ -595,7 +595,7 @@ func TestBackendDownRetry(t *testing.T) {
 		if callCount >= 3 {
 			directoryServer.DeleteTenant(roachpb.MustMakeTenantID(28))
 		}
-		return nil, withCode(errors.New("SQL pod is down"), codeBackendDown)
+		return nil, withCode(errors.New("SQL pod is down"), codeBackendDialFailed)
 	})()
 
 	// Valid connection, but no backend server running.
@@ -1358,7 +1358,7 @@ func TestDirectoryConnect(t *testing.T) {
 			if countFailures >= 3 {
 				return nil, withCode(errors.New("backend disconnected"), codeBackendDisconnected)
 			}
-			return nil, withCode(errors.New("backend down"), codeBackendDown)
+			return nil, withCode(errors.New("backend down"), codeBackendDialFailed)
 		})()
 
 		// Ensure that Directory.ReportFailure is being called correctly.