diff --git a/httpclient/config.go b/httpclient/config.go index c62020a..534b6c9 100644 --- a/httpclient/config.go +++ b/httpclient/config.go @@ -9,24 +9,37 @@ package httpclient import ( "errors" "github.com/acronis/go-appkit/config" + "github.com/acronis/go-appkit/retry" + "github.com/cenkalti/backoff/v4" "time" ) const ( + // DefaultClientWaitTimeout is a default timeout for a client to wait for a request. DefaultClientWaitTimeout = 10 * time.Second + // RetryPolicyExponential is a policy for exponential retries. + RetryPolicyExponential = "exponential" + + // RetryPolicyConstant is a policy for constant retries. + RetryPolicyConstant = "constant" + // configuration properties - cfgKeyRetriesEnabled = "retries.enabled" - cfgKeyRetriesMax = "retries.maxAttempts" - cfgKeyRateLimitsEnabled = "rateLimits.enabled" - cfgKeyRateLimitsLimit = "rateLimits.limit" - cfgKeyRateLimitsBurst = "rateLimits.burst" - cfgKeyRateLimitsWaitTimeout = "rateLimits.waitTimeout" - cfgKeyLoggerEnabled = "logger.enabled" - cfgKeyLoggerMode = "logger.mode" - cfgKeyLoggerSlowRequestThreshold = "logger.slowRequestThreshold" - cfgKeyMetricsEnabled = "metrics.enabled" - cfgKeyTimeout = "timeout" + cfgKeyRetriesEnabled = "retries.enabled" + cfgKeyRetriesMax = "retries.maxAttempts" + cfgKeyRetriesPolicyStrategy = "retries.policy.strategy" + cfgKeyRetriesPolicyExponentialInitialInterval = "retries.policy.exponentialBackoffInitialInterval" + cfgKeyRetriesPolicyExponentialMultiplier = "retries.policy.exponentialBackoffMultiplier" + cfgKeyRetriesPolicyConstantInternal = "retries.policy.constantBackoffInterval" + cfgKeyRateLimitsEnabled = "rateLimits.enabled" + cfgKeyRateLimitsLimit = "rateLimits.limit" + cfgKeyRateLimitsBurst = "rateLimits.burst" + cfgKeyRateLimitsWaitTimeout = "rateLimits.waitTimeout" + cfgKeyLoggerEnabled = "logger.enabled" + cfgKeyLoggerMode = "logger.mode" + cfgKeyLoggerSlowRequestThreshold = "logger.slowRequestThreshold" + cfgKeyMetricsEnabled = "metrics.enabled" + cfgKeyTimeout = "timeout" ) var _ config.Config = (*Config)(nil) @@ -34,9 +47,16 @@ var _ config.KeyPrefixProvider = (*Config)(nil) // RateLimitConfig represents configuration options for HTTP client rate limits. type RateLimitConfig struct { - Enabled bool `mapstructure:"enabled"` - Limit int `mapstructure:"limit"` - Burst int `mapstructure:"burst"` + // Enabled is a flag that enables rate limiting. + Enabled bool `mapstructure:"enabled"` + + // Limit is the maximum number of requests that can be made. + Limit int `mapstructure:"limit"` + + // Burst allow temporary spikes in request rate. + Burst int `mapstructure:"burst"` + + // WaitTimeout is the maximum time to wait for a request to be made. WaitTimeout time.Duration `mapstructure:"waitTimeout"` } @@ -48,6 +68,10 @@ func (c *RateLimitConfig) Set(dp config.DataProvider) (err error) { } c.Enabled = enabled + if !c.Enabled { + return nil + } + limit, err := dp.GetInt(cfgKeyRateLimitsLimit) if err != nil { return err @@ -89,10 +113,104 @@ func (c *RateLimitConfig) TransportOpts() RateLimitingRoundTripperOpts { } } +// PolicyConfig represents configuration options for policy retry. +type PolicyConfig struct { + // Strategy is a strategy for retry policy. + Strategy string `mapstructure:"strategy"` + + // ExponentialBackoffInitialInterval is the initial interval for exponential backoff. + ExponentialBackoffInitialInterval time.Duration `mapstructure:"exponentialBackoffInitialInterval"` + + // ExponentialBackoffMultiplier is the multiplier for exponential backoff. + ExponentialBackoffMultiplier float64 `mapstructure:"exponentialBackoffMultiplier"` + + // ConstantBackoffInterval is the interval for constant backoff. + ConstantBackoffInterval time.Duration `mapstructure:"constantBackoffInterval"` +} + +// Set is part of config interface implementation. +func (c *PolicyConfig) Set(dp config.DataProvider) (err error) { + strategy, err := dp.GetString(cfgKeyRetriesPolicyStrategy) + if err != nil { + return err + } + c.Strategy = strategy + + if c.Strategy != "" && c.Strategy != RetryPolicyExponential && c.Strategy != RetryPolicyConstant { + return errors.New("client retry policy must be one of: [exponential, constant]") + } + + if c.Strategy == RetryPolicyExponential { + var interval time.Duration + interval, err = dp.GetDuration(cfgKeyRetriesPolicyExponentialInitialInterval) + if err != nil { + return nil + } + if interval < 0 { + return errors.New("client exponential backoff initial interval must be positive") + } + c.ExponentialBackoffInitialInterval = interval + + var multiplier float64 + multiplier, err = dp.GetFloat64(cfgKeyRetriesPolicyExponentialMultiplier) + if err != nil { + return err + } + if multiplier <= 1 { + return errors.New("client exponential backoff multiplier must be greater than 1") + } + c.ExponentialBackoffMultiplier = multiplier + + return nil + } else if c.Strategy == RetryPolicyConstant { + var interval time.Duration + interval, err = dp.GetDuration(cfgKeyRetriesPolicyConstantInternal) + if err != nil { + return err + } + if interval < 0 { + return errors.New("client constant backoff interval must be positive") + } + c.ConstantBackoffInterval = interval + } + + return nil +} + +// SetProviderDefaults is part of config interface implementation. +func (c *PolicyConfig) SetProviderDefaults(_ config.DataProvider) {} + // RetriesConfig represents configuration options for HTTP client retries policy. type RetriesConfig struct { - Enabled bool `mapstructure:"enabled"` - MaxAttempts int `mapstructure:"maxAttempts"` + // Enabled is a flag that enables retries. + Enabled bool `mapstructure:"enabled"` + + // MaxAttempts is the maximum number of attempts to retry the request. + MaxAttempts int `mapstructure:"maxAttempts"` + + // Policy of a retry: [exponential, constant]. default is exponential. + Policy PolicyConfig `mapstructure:"policy"` +} + +// GetPolicy returns a retry policy based on strategy or nil if none is provided. +func (c *RetriesConfig) GetPolicy() retry.Policy { + if c.Policy.Strategy == RetryPolicyExponential { + return retry.PolicyFunc(func() backoff.BackOff { + bf := backoff.NewExponentialBackOff() + bf.InitialInterval = c.Policy.ExponentialBackoffInitialInterval + bf.Multiplier = c.Policy.ExponentialBackoffMultiplier + bf.Reset() + return bf + }) + } else if c.Policy.Strategy == RetryPolicyConstant { + return retry.PolicyFunc(func() backoff.BackOff { + bf := backoff.NewConstantBackOff(c.Policy.ConstantBackoffInterval) + bf.Reset() + return bf + }) + } + + return nil } // Set is part of config interface implementation. @@ -103,6 +221,10 @@ func (c *RetriesConfig) Set(dp config.DataProvider) error { } c.Enabled = enabled + if !c.Enabled { + return nil + } + maxAttempts, err := dp.GetInt(cfgKeyRetriesMax) if err != nil { return err @@ -112,6 +234,11 @@ func (c *RetriesConfig) Set(dp config.DataProvider) error { } c.MaxAttempts = maxAttempts + err = c.Policy.Set(config.NewKeyPrefixedDataProvider(dp, "")) + if err != nil { + return err + } + return nil } @@ -125,9 +252,14 @@ func (c *RetriesConfig) TransportOpts() RetryableRoundTripperOpts { // LoggerConfig represents configuration options for HTTP client logs. type LoggerConfig struct { - Enabled bool `mapstructure:"enabled"` + // Enabled is a flag that enables logging. + Enabled bool `mapstructure:"enabled"` + + // SlowRequestThreshold is a threshold for slow requests. SlowRequestThreshold time.Duration `mapstructure:"slowRequestThreshold"` - Mode string `mapstructure:"mode"` + + // Mode of logging. + Mode string `mapstructure:"mode"` } // Set is part of config interface implementation. @@ -138,6 +270,10 @@ func (c *LoggerConfig) Set(dp config.DataProvider) error { } c.Enabled = enabled + if !c.Enabled { + return nil + } + slowRequestThreshold, err := dp.GetDuration(cfgKeyLoggerSlowRequestThreshold) if err != nil { return err @@ -172,6 +308,7 @@ func (c *LoggerConfig) TransportOpts() LoggingRoundTripperOpts { // MetricsConfig represents configuration options for HTTP client logs. type MetricsConfig struct { + // Enabled is a flag that enables metrics. Enabled bool `mapstructure:"enabled"` } @@ -191,12 +328,22 @@ func (c *MetricsConfig) SetProviderDefaults(_ config.DataProvider) {} // Config represents options for HTTP client configuration. type Config struct { - Retries RetriesConfig `mapstructure:"retries"` + // Retries is a configuration for HTTP client retries policy. + Retries RetriesConfig `mapstructure:"retries"` + + // RateLimits is a configuration for HTTP client rate limits. RateLimits RateLimitConfig `mapstructure:"rateLimits"` - Logger LoggerConfig `mapstructure:"logger"` - Metrics MetricsConfig `mapstructure:"metrics"` - Timeout time.Duration `mapstructure:"timeout"` + // Logger is a configuration for HTTP client logs. + Logger LoggerConfig `mapstructure:"logger"` + + // Metrics is a configuration for HTTP client metrics. + Metrics MetricsConfig `mapstructure:"metrics"` + + // Timeout is the maximum time to wait for a request to be made. + Timeout time.Duration `mapstructure:"timeout"` + + // keyPrefix is a prefix for configuration parameters. keyPrefix string } diff --git a/httpclient/config_test.go b/httpclient/config_test.go index 1cf9a8e..3cbe31a 100644 --- a/httpclient/config_test.go +++ b/httpclient/config_test.go @@ -9,30 +9,15 @@ package httpclient import ( "bytes" "github.com/acronis/go-appkit/config" + "github.com/acronis/go-appkit/retry" "github.com/stretchr/testify/require" + "strings" "testing" "time" ) func TestConfigWithLoader(t *testing.T) { - yamlData := []byte(` -retries: - enabled: true - maxAttempts: 30 -rateLimits: - enabled: true - limit: 300 - burst: 3000 - waitTimeout: 3s -logger: - enabled: true - slowRequestThreshold: 5s - mode: all -metrics: - enabled: true -timeout: 30s -`) - + yamlData := testYamlData(nil) actualConfig := &Config{} err := config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) require.NoError(t, err, "load configuration") @@ -41,6 +26,11 @@ timeout: 30s Retries: RetriesConfig{ Enabled: true, MaxAttempts: 30, + Policy: PolicyConfig{ + Strategy: RetryPolicyExponential, + ExponentialBackoffInitialInterval: 3 * time.Second, + ExponentialBackoffMultiplier: 2, + }, }, RateLimits: RateLimitConfig{ Enabled: true, @@ -60,101 +50,115 @@ timeout: 30s require.Equal(t, expectedConfig, actualConfig, "configuration does not match expected") } -func TestConfigRateLimitInvalid(t *testing.T) { - yamlData := []byte(` -retries: - enabled: true - maxAttempts: 30 -rateLimits: - enabled: true - limit: -300 - burst: 3000 - waitTimeout: 3s -timeout: 30s -`) - +func TestConfigRateLimit(t *testing.T) { + yamlData := testYamlData([][]string{{"limit: 300", "limit: -300"}}) actualConfig := &Config{} err := config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) require.Error(t, err) require.Equal(t, "client rate limit must be positive", err.Error()) - yamlData = []byte(` -retries: - enabled: true - maxAttempts: 30 -rateLimits: - enabled: true - limit: 300 - burst: -3 - waitTimeout: 3s -timeout: 30s -`) - + yamlData = testYamlData([][]string{{"burst: 3000", "burst: -3"}}) actualConfig = &Config{} err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) require.Error(t, err) require.Equal(t, "client burst must be positive", err.Error()) - yamlData = []byte(` -retries: - enabled: true - maxAttempts: 30 -rateLimits: - enabled: true - limit: 300 - burst: 3 - waitTimeout: -3s -timeout: 30s -`) - + yamlData = testYamlData([][]string{{"waitTimeout: 3s", "waitTimeout: -3s"}}) actualConfig = &Config{} err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) require.Error(t, err) require.Equal(t, "client wait timeout must be positive", err.Error()) } -func TestConfigRetriesInvalid(t *testing.T) { - yamlData := []byte(` -retries: - enabled: true - maxAttempts: -30 -timeout: 30s -`) - +func TestConfigRetries(t *testing.T) { + yamlData := testYamlData([][]string{{"maxAttempts: 30", "maxAttempts: -30"}}) actualConfig := &Config{} err := config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) require.Error(t, err) require.Equal(t, "client max retry attempts must be positive", err.Error()) } -func TestConfigLoggerInvalid(t *testing.T) { +func TestConfigLogger(t *testing.T) { + yamlData := testYamlData([][]string{{"slowRequestThreshold: 5s", "slowRequestThreshold: -5s"}}) + actualConfig := &Config{} + err := config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.Error(t, err) + require.Equal(t, "client logger slow request threshold can not be negative", err.Error()) + + yamlData = testYamlData([][]string{{"mode: all", "mode: invalid"}}) + actualConfig = &Config{} + err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.Error(t, err) + require.Equal(t, "client logger invalid mode, choose one of: [none, all, failed]", err.Error()) +} + +func TestConfigRetriesPolicy(t *testing.T) { + yamlData := testYamlData([][]string{{"strategy: exponential", "strategy: invalid"}}) + actualConfig := &Config{} + err := config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.Error(t, err) + require.Equal(t, "client retry policy must be one of: [exponential, constant]", err.Error()) + + yamlData = testYamlData([][]string{ + {"exponentialBackoffInitialInterval: 3s", "exponentialBackoffInitialInterval: -1s"}, + }) + err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.Error(t, err) + require.Equal(t, "client exponential backoff initial interval must be positive", err.Error()) + + yamlData = testYamlData([][]string{{"exponentialBackoffMultiplier: 2", "exponentialBackoffMultiplier: 1"}}) + err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.Error(t, err) + require.Equal(t, "client exponential backoff multiplier must be greater than 1", err.Error()) + + yamlData = testYamlData([][]string{ + {"strategy: exponential", "strategy: constant"}, + {"constantBackoffInterval: 2s", "constantBackoffInterval: -3s"}, + }) + err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.Error(t, err) + require.Equal(t, "client constant backoff interval must be positive", err.Error()) + + yamlData = testYamlData([][]string{ + {"strategy: exponential", "strategy:"}, + }) + err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.NoError(t, err) + require.Nil(t, actualConfig.Retries.GetPolicy()) + + yamlData = testYamlData(nil) + err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) + require.NoError(t, err) + require.Implements(t, (*retry.Policy)(nil), actualConfig.Retries.GetPolicy()) +} + +func TestConfigDisableWithLoader(t *testing.T) { yamlData := []byte(` retries: - enabled: true - maxAttempts: 30 + enabled: false rateLimits: - enabled: true - limit: 300 - burst: 3000 - waitTimeout: 3s + enabled: false logger: - enabled: true - slowRequestThreshold: -5s - mode: all + enabled: false metrics: - enabled: true + enabled: false timeout: 30s `) - actualConfig := &Config{} err := config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) - require.Error(t, err) - require.Equal(t, "client logger slow request threshold can not be negative", err.Error()) + require.NoError(t, err) +} - yamlData = []byte(` +func testYamlData(replacements [][]string) []byte { + yamlData := ` retries: enabled: true maxAttempts: 30 + policy: + strategy: exponential + exponentialBackoffInitialInterval: 3s + exponentialBackoffMultiplier: 2 + constantBackoffInterval: 2s rateLimits: enabled: true limit: 300 @@ -163,14 +167,14 @@ rateLimits: logger: enabled: true slowRequestThreshold: 5s - mode: invalid + mode: all metrics: enabled: true timeout: 30s -`) +` + for i := range replacements { + yamlData = strings.Replace(yamlData, replacements[i][0], replacements[i][1], 1) + } - actualConfig = &Config{} - err = config.NewDefaultLoader("").LoadFromReader(bytes.NewReader(yamlData), config.DataTypeYAML, actualConfig) - require.Error(t, err) - require.Equal(t, "client logger invalid mode, choose one of: [none, all, failed]", err.Error()) + return []byte(yamlData) } diff --git a/httpclient/httpclient.go b/httpclient/httpclient.go index c919889..b41ca28 100644 --- a/httpclient/httpclient.go +++ b/httpclient/httpclient.go @@ -34,7 +34,10 @@ func CloneHTTPHeader(in http.Header) http.Header { // ClientProviders for further customization of the client logging and request id. type ClientProviders struct { - Logger func(ctx context.Context) log.FieldLogger + // Logger is a function that provides a context-specific logger. + Logger func(ctx context.Context) log.FieldLogger + + // RequestID is a function that provides a request ID. RequestID func(ctx context.Context) string } @@ -75,6 +78,7 @@ func NewHTTPClient( if cfg.Retries.Enabled { opts := cfg.Retries.TransportOpts() opts.LoggerProvider = providers.Logger + opts.BackoffPolicy = cfg.Retries.GetPolicy() delegate, err = NewRetryableRoundTripperWithOpts(delegate, opts) if err != nil { return nil, fmt.Errorf("create retryable round tripper: %w", err) @@ -108,10 +112,19 @@ func MustHTTPClient( // ClientOpts provides options for NewHTTPClientWithOpts and MustHTTPClientWithOpts functions. type ClientOpts struct { - Config Config + // Config is the configuration for the HTTP client. + Config Config + + // UserAgent is a user agent string. UserAgent string - ReqType string - Delegate http.RoundTripper + + // ReqType is a type of request. + ReqType string + + // Delegate is the next RoundTripper in the chain. + Delegate http.RoundTripper + + // Providers are the functions that provide a context-specific logger and request ID. Providers ClientProviders } diff --git a/httpclient/httpclient_test.go b/httpclient/httpclient_test.go index a440d95..6f94e87 100644 --- a/httpclient/httpclient_test.go +++ b/httpclient/httpclient_test.go @@ -14,6 +14,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" ) func TestNewHTTPClientLoggingRoundTripper(t *testing.T) { @@ -114,3 +115,33 @@ func TestMustHTTPClientWithOptsRoundTripper(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, logger.Entries()) } + +func TestMustHTTPClientWithOptsRoundTripperPolicy(t *testing.T) { + var retriesCount int + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + retriesCount++ + rw.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + cfg := NewConfig() + cfg.Retries.Enabled = true + cfg.Retries.MaxAttempts = 1 + cfg.Retries.Policy.Strategy = RetryPolicyExponential + cfg.Retries.Policy.ExponentialBackoffInitialInterval = 2 * time.Millisecond + cfg.Retries.Policy.ExponentialBackoffMultiplier = 1.1 + + client := MustHTTPClientWithOpts(ClientOpts{ + Config: *cfg, + UserAgent: "test-agent", + ReqType: "test-request", + Delegate: nil, + }) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, server.URL, nil) + require.NoError(t, err) + + r, err := client.Do(req) + defer func() { _ = r.Body.Close() }() + require.NoError(t, err) + require.Equal(t, 2, retriesCount) +} diff --git a/httpclient/logging_round_tripper.go b/httpclient/logging_round_tripper.go index fddd86c..1fbf661 100644 --- a/httpclient/logging_round_tripper.go +++ b/httpclient/logging_round_tripper.go @@ -35,9 +35,14 @@ func (lm LoggerMode) IsValid() bool { // LoggingRoundTripper implements http.RoundTripper for logging requests. type LoggingRoundTripper struct { + // Delegate is the next RoundTripper in the chain. Delegate http.RoundTripper - ReqType string - Opts LoggingRoundTripperOpts + + // ReqType is a type of request. + ReqType string + + // Opts are the options for the logging round tripper. + Opts LoggingRoundTripperOpts } // LoggingRoundTripperOpts represents an options for LoggingRoundTripper. diff --git a/httpclient/metrics_round_tripper.go b/httpclient/metrics_round_tripper.go index f5c1340..faf27dd 100644 --- a/httpclient/metrics_round_tripper.go +++ b/httpclient/metrics_round_tripper.go @@ -43,8 +43,11 @@ func UnregisterMetrics() { // MetricsRoundTripper is an HTTP transport that measures requests done. type MetricsRoundTripper struct { + // Delegate is the next RoundTripper in the chain. Delegate http.RoundTripper - ReqType string + + // ReqType is a type of request. + ReqType string } // NewMetricsRoundTripper creates an HTTP transport that measures requests done. diff --git a/httpclient/request_id_round_tripper.go b/httpclient/request_id_round_tripper.go index 6d2ddea..6e51e4d 100644 --- a/httpclient/request_id_round_tripper.go +++ b/httpclient/request_id_round_tripper.go @@ -14,12 +14,16 @@ import ( // RequestIDRoundTripper for X-Request-ID header to the request. type RequestIDRoundTripper struct { + // Delegate is the next RoundTripper in the chain. Delegate http.RoundTripper - Opts RequestIDRoundTripperOpts + + // Opts are the options for the request ID round tripper. + Opts RequestIDRoundTripperOpts } // RequestIDRoundTripperOpts for X-Request-ID header to the request options. type RequestIDRoundTripperOpts struct { + // RequestIDProvider is a function that provides a request ID. RequestIDProvider func(ctx context.Context) string }