Properly handle grpc dial errors in the throttler metric aggregation (vitessio#18073)

Signed-off-by: Arthur Schreiber <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Co-authored-by: Mohamed Hamza <[email protected]>

Author: arthurschreiber

go/vt/vttablet/tabletserver/throttle/base/throttle_metric.go

@@ -44,6 +44,9 @@ package base
 import (
 	"errors"
 	"strings"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // MetricResult is what we expect our probes to return. This can be a numeric result, or
@@ -70,6 +73,11 @@ func IsDialTCPError(e error) bool {
 	if e == nil {
 		return false
 	}
+
+	if s, ok := status.FromError(e); ok {
+		return s.Code() == codes.Unavailable || s.Code() == codes.DeadlineExceeded
+	}
+
 	return strings.HasPrefix(e.Error(), "dial tcp")
 }
 

go/vt/vttablet/tabletserver/throttle/throttler.go

@@ -859,7 +859,7 @@ func (throttler *Throttler) generateTabletProbeFunction(ctx context.Context, clu
 		req := &tabletmanagerdatapb.CheckThrottlerRequest{} // We leave AppName empty; it will default to VitessName anyway, and we can save some proto space
 		resp, gRPCErr := tmClient.CheckThrottler(ctx, probe.Tablet, req)
 		if gRPCErr != nil {
-			mySQLThrottleMetric.Err = fmt.Errorf("gRPC error accessing tablet %v. Err=%v", probe.Alias, gRPCErr)
+			mySQLThrottleMetric.Err = fmt.Errorf("gRPC error accessing tablet %v. Err=%w", probe.Alias, gRPCErr)
 			return mySQLThrottleMetric
 		}
 		mySQLThrottleMetric.Value = resp.Value

go/vt/vttablet/tabletserver/throttle/throttler_test.go

@@ -29,10 +29,15 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+
+	"vitess.io/vitess/go/vt/grpcclient"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/vtenv"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/connpool"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv"
+	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/base"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/config"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/mysql"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/throttlerapp"
@@ -191,6 +196,21 @@ func TestInitThrottler(t *testing.T) {
 	assert.EqualValues(t, 3, throttler.recentCheckDiff)
 }
 
+func TestIsDialTCPError(t *testing.T) {
+	// Verify that IsDialTCPError actually recognizes grpc dial errors
+	cc, err := grpcclient.DialContext(context.Background(), ":0", true, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	require.NoError(t, err)
+	defer cc.Close()
+
+	err = cc.Invoke(context.Background(), "/Fail", nil, nil)
+
+	require.True(t, base.IsDialTCPError(err))
+	require.True(t, base.IsDialTCPError(fmt.Errorf("wrapped: %w", err)))
+
+	nonDialErr := fmt.Errorf("rpc error: code = NotFound desc = method not found")
+	require.False(t, base.IsDialTCPError(nonDialErr))
+}
+
 func TestIsAppThrottled(t *testing.T) {
 	throttler := Throttler{
 		throttledApps:   cache.New(cache.NoExpiration, 0),