From effe78ceac5b786a9e7260b4188f4041af549146 Mon Sep 17 00:00:00 2001 From: Dan Kortschak <90160302+efd6@users.noreply.github.com> Date: Tue, 24 Oct 2023 20:52:48 +1030 Subject: [PATCH] x-pack/filebeat/input/{cel,httpjson}: elide unneeded retryable HTTP client construction (#36916) --- CHANGELOG-developer.next.asciidoc | 1 + x-pack/filebeat/input/cel/input.go | 22 +++++----- x-pack/filebeat/input/httpjson/input.go | 53 +++++++++++++------------ 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/CHANGELOG-developer.next.asciidoc b/CHANGELOG-developer.next.asciidoc index c3414579d70..7fefb8d14c2 100644 --- a/CHANGELOG-developer.next.asciidoc +++ b/CHANGELOG-developer.next.asciidoc @@ -176,6 +176,7 @@ The list below covers the major changes between 7.0.0-rc2 and main only. - Make Filebeat HTTPJSON input process responses sequentially. {pull}36493[36493] - Add initial infrastructure for a caching enrichment processor. {pull}36619[36619] - Add file-backed cache for cache enrichment processor. {pull}36686[36686] {pull}36696[36696] +- Elide retryable HTTP client construction in Filebeat HTTPJSON and CEL inputs if not needed. {pull}36916[36916] ==== Deprecated diff --git a/x-pack/filebeat/input/cel/input.go b/x-pack/filebeat/input/cel/input.go index 499ae97f7fa..03d716e0c6c 100644 --- a/x-pack/filebeat/input/cel/input.go +++ b/x-pack/filebeat/input/cel/input.go @@ -721,25 +721,27 @@ func newClient(ctx context.Context, cfg config, log *logp.Logger) (*http.Client, c.CheckRedirect = checkRedirect(cfg.Resource, log) - client := &retryablehttp.Client{ - HTTPClient: c, - Logger: newRetryLog(log), - RetryWaitMin: cfg.Resource.Retry.getWaitMin(), - RetryWaitMax: cfg.Resource.Retry.getWaitMax(), - RetryMax: cfg.Resource.Retry.getMaxAttempts(), - CheckRetry: retryablehttp.DefaultRetryPolicy, - Backoff: retryablehttp.DefaultBackoff, + if cfg.Resource.Retry.getMaxAttempts() > 1 { + c = (&retryablehttp.Client{ + HTTPClient: c, + Logger: newRetryLog(log), + RetryWaitMin: cfg.Resource.Retry.getWaitMin(), + RetryWaitMax: cfg.Resource.Retry.getWaitMax(), + RetryMax: cfg.Resource.Retry.getMaxAttempts(), + CheckRetry: retryablehttp.DefaultRetryPolicy, + Backoff: retryablehttp.DefaultBackoff, + }).StandardClient() } if cfg.Auth.OAuth2.isEnabled() { - authClient, err := cfg.Auth.OAuth2.client(ctx, client.StandardClient()) + authClient, err := cfg.Auth.OAuth2.client(ctx, c) if err != nil { return nil, err } return authClient, nil } - return client.StandardClient(), nil + return c, nil } func wantClient(cfg config) bool { diff --git a/x-pack/filebeat/input/httpjson/input.go b/x-pack/filebeat/input/httpjson/input.go index 592f9bce86d..928c056d2d3 100644 --- a/x-pack/filebeat/input/httpjson/input.go +++ b/x-pack/filebeat/input/httpjson/input.go @@ -192,37 +192,38 @@ func sanitizeFileName(name string) string { } func newHTTPClient(ctx context.Context, config config, log *logp.Logger, reg *monitoring.Registry) (*httpClient, error) { - // Make retryable HTTP client - netHTTPClient, err := newNetHTTPClient(ctx, config.Request, log, reg) + client, err := newNetHTTPClient(ctx, config.Request, log, reg) if err != nil { return nil, err } - client := &retryablehttp.Client{ - HTTPClient: netHTTPClient, - Logger: newRetryLogger(log), - RetryWaitMin: config.Request.Retry.getWaitMin(), - RetryWaitMax: config.Request.Retry.getWaitMax(), - RetryMax: config.Request.Retry.getMaxAttempts(), - CheckRetry: retryablehttp.DefaultRetryPolicy, - Backoff: retryablehttp.DefaultBackoff, + if config.Request.Retry.getMaxAttempts() > 1 { + // Make retryable HTTP client if needed. + client = (&retryablehttp.Client{ + HTTPClient: client, + Logger: newRetryLogger(log), + RetryWaitMin: config.Request.Retry.getWaitMin(), + RetryWaitMax: config.Request.Retry.getWaitMax(), + RetryMax: config.Request.Retry.getMaxAttempts(), + CheckRetry: retryablehttp.DefaultRetryPolicy, + Backoff: retryablehttp.DefaultBackoff, + }).StandardClient() } limiter := newRateLimiterFromConfig(config.Request.RateLimit, log) if config.Auth.OAuth2.isEnabled() { - authClient, err := config.Auth.OAuth2.client(ctx, client.StandardClient()) + authClient, err := config.Auth.OAuth2.client(ctx, client) if err != nil { return nil, err } return &httpClient{client: authClient, limiter: limiter}, nil } - return &httpClient{client: client.StandardClient(), limiter: limiter}, nil + return &httpClient{client: client, limiter: limiter}, nil } func newNetHTTPClient(ctx context.Context, cfg *requestConfig, log *logp.Logger, reg *monitoring.Registry) (*http.Client, error) { - // Make retryable HTTP client netHTTPClient, err := cfg.Transport.Client(clientOptions(cfg.URL.URL, cfg.KeepAlive.settings())...) if err != nil { return nil, err @@ -255,8 +256,7 @@ func newNetHTTPClient(ctx context.Context, cfg *requestConfig, log *logp.Logger, } func newChainHTTPClient(ctx context.Context, authCfg *authConfig, requestCfg *requestConfig, log *logp.Logger, reg *monitoring.Registry, p ...*Policy) (*httpClient, error) { - // Make retryable HTTP client - netHTTPClient, err := newNetHTTPClient(ctx, requestCfg, log, reg) + client, err := newNetHTTPClient(ctx, requestCfg, log, reg) if err != nil { return nil, err } @@ -268,27 +268,30 @@ func newChainHTTPClient(ctx context.Context, authCfg *authConfig, requestCfg *re retryPolicyFunc = retryablehttp.DefaultRetryPolicy } - client := &retryablehttp.Client{ - HTTPClient: netHTTPClient, - Logger: newRetryLogger(log), - RetryWaitMin: requestCfg.Retry.getWaitMin(), - RetryWaitMax: requestCfg.Retry.getWaitMax(), - RetryMax: requestCfg.Retry.getMaxAttempts(), - CheckRetry: retryPolicyFunc, - Backoff: retryablehttp.DefaultBackoff, + if requestCfg.Retry.getMaxAttempts() > 1 { + // Make retryable HTTP client if needed. + client = (&retryablehttp.Client{ + HTTPClient: client, + Logger: newRetryLogger(log), + RetryWaitMin: requestCfg.Retry.getWaitMin(), + RetryWaitMax: requestCfg.Retry.getWaitMax(), + RetryMax: requestCfg.Retry.getMaxAttempts(), + CheckRetry: retryPolicyFunc, + Backoff: retryablehttp.DefaultBackoff, + }).StandardClient() } limiter := newRateLimiterFromConfig(requestCfg.RateLimit, log) if authCfg != nil && authCfg.OAuth2.isEnabled() { - authClient, err := authCfg.OAuth2.client(ctx, client.StandardClient()) + authClient, err := authCfg.OAuth2.client(ctx, client) if err != nil { return nil, err } return &httpClient{client: authClient, limiter: limiter}, nil } - return &httpClient{client: client.StandardClient(), limiter: limiter}, nil + return &httpClient{client: client, limiter: limiter}, nil } // clientOption returns constructed client configuration options, including