diff --git a/CHANGELOG.md b/CHANGELOG.md index b0ecf95e9..924f64911 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/grpcclient/grpcclient.go b/grpcclient/grpcclient.go index bf95a1f34..7616c2071 100644 --- a/grpcclient/grpcclient.go +++ b/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 diff --git a/grpcclient/grpcclient_test.go b/grpcclient/grpcclient_test.go index 9a8cdd3b1..0965e0587 100644 --- a/grpcclient/grpcclient_test.go +++ b/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))