grpcclient: Make keepAliveTime* into method arguments

Signed-off-by: Arve Knudsen <[email protected]>

CHANGELOG.md

@@ -15,7 +15,7 @@
 * [CHANGE] Remove package `math`. #104
 * [CHANGE] time: Remove time package. #103
 * [CHANGE] grpcutil: Convert Resolver into concrete type. #105
-* [CHANGE] grpcclient: Make ping time and timeout into configuration parameters. #56
+* [CHANGE] grpcclient: Make ping time and timeout into `Config.DialOption` arguments (with same defaults as corresponding `google.golang.org/grpc/keepalive.ClientParameters.Time*` parameters). #56
 * [ENHANCEMENT] Add middleware package. #38
 * [ENHANCEMENT] Add the ring package #45
 * [ENHANCEMENT] Add limiter package. #41

grpcclient/grpcclient.go

@@ -23,15 +23,6 @@ type Config struct {
 	GRPCCompression string  `yaml:"grpc_compression"`
 	RateLimit       float64 `yaml:"rate_limit"`
 	RateLimitBurst  int     `yaml:"rate_limit_burst"`
-	// KeepaliveTime is the number of seconds after which the client will ping the server in case of inactivity.
-	//
-	// See `google.golang.org/grpc/keepalive.ClientParameters.Time` for reference.
-	KeepaliveTime int64 `yaml:"keepalive_time"`
-	// KeepaliveTimeout is the number of seconds the client waits after pinging the server, and if no activity is seen
-	// after that, the connection is closed.
-	//
-	// See `google.golang.org/grpc/keepalive.ClientParameters.Timeout` for reference.
-	KeepaliveTimeout int64 `yaml:"keepalive_timeout"`
 
 	BackoffOnRatelimits bool           `yaml:"backoff_on_ratelimits"`
 	BackoffConfig       backoff.Config `yaml:"backoff_config"`
@@ -81,8 +72,15 @@ func (cfg *Config) CallOptions() []grpc.CallOption {
 	return opts
 }
 
-// DialOption returns the config as a grpc.DialOptions.
-func (cfg *Config) DialOption(unaryClientInterceptors []grpc.UnaryClientInterceptor, streamClientInterceptors []grpc.StreamClientInterceptor) ([]grpc.DialOption, error) {
+// DialOption returns the config as a slice of grpc.DialOptions.
+//
+// keepaliveTime is the number of seconds after which the client will ping the server in case of inactivity.
+// See `google.golang.org/grpc/keepalive.ClientParameters.Time` for reference.
+//
+// keepaliveTimeout is the number of seconds the client waits after pinging the server, and if no activity is
+// seen after that, the connection is closed. See `google.golang.org/grpc/keepalive.ClientParameters.Timeout`
+// for reference.
+func (cfg *Config) DialOption(unaryClientInterceptors []grpc.UnaryClientInterceptor, streamClientInterceptors []grpc.StreamClientInterceptor, keepaliveTime, keepaliveTimeout int64) ([]grpc.DialOption, error) {
 	opts, err := cfg.TLS.GetGRPCDialOptions(cfg.TLSEnabled)
 	if err != nil {
 		return nil, err
@@ -102,8 +100,8 @@ func (cfg *Config) DialOption(unaryClientInterceptors []grpc.UnaryClientIntercep
 		withUnaryInterceptor(middleware.ChainUnaryClient(unaryClientInterceptors...)),
 		withStreamInterceptor(middleware.ChainStreamClient(streamClientInterceptors...)),
 		withKeepaliveParams(keepalive.ClientParameters{
-			Time:                time.Duration(cfg.KeepaliveTime) * time.Second,
-			Timeout:             time.Duration(cfg.KeepaliveTimeout) * time.Second,
+			Time:                time.Duration(keepaliveTime) * time.Second,
+			Timeout:             time.Duration(keepaliveTimeout) * time.Second,
 			PermitWithoutStream: true,
 		}),
 	), nil

grpcclient/grpcclient_test.go

@@ -89,10 +89,9 @@ func TestConfig(t *testing.T) {
 		return fakeDialOption{isInsecure: true}
 	}
 
-	cfg := Config{
-		KeepaliveTime:    10,
-		KeepaliveTimeout: 20,
-	}
+	cfg := Config{}
+	const keepaliveTime = 10
+	const keepaliveTimeout = 20
 	expOpts := []grpc.DialOption{
 		fakeDialOption{isInsecure: true},
 		fakeDialOption{callOpts: []grpc.CallOption{
@@ -107,14 +106,14 @@ func TestConfig(t *testing.T) {
 		},
 		fakeDialOption{
 			keepaliveParams: keepalive.ClientParameters{
-				Time:                10 * time.Second,
-				Timeout:             20 * time.Second,
+				Time:                keepaliveTime * time.Second,
+				Timeout:             keepaliveTimeout * time.Second,
 				PermitWithoutStream: true,
 			},
 		},
 	}
 
-	opts, err := cfg.DialOption(nil, nil)
+	opts, err := cfg.DialOption(nil, nil, keepaliveTime, keepaliveTimeout)
 	require.NoError(t, err)
 
 	assert.Empty(t, cmp.Diff(expOpts, opts))