x-pack/filebeat/input/httpjson/rate_limiter.go: Rate limit evaluation only happens on 429 responses

[marc-gr]

The rate limiter only evaluates the rate limit logic if a 429 response is found:

func (r *rateLimiter) execute(ctx context.Context, f func() (*http.Response, error)) (*http.Response, error) {
	for {
		resp, err := f()
		if err != nil {
			return nil, fmt.Errorf("failed to read http.response.body: %w", err)
		}

		if r == nil || resp.StatusCode == http.StatusOK {
			return resp, nil
		}

		if resp.StatusCode != http.StatusTooManyRequests {
			return nil, fmt.Errorf("http request was unsuccessful with a status code %d", resp.StatusCode)
		}

		if err := r.applyRateLimit(ctx, resp); err != nil {
			return nil, err
		}
	}
}

this introduces several issues:

1- Some APIs do not honor the 429 convention and in that case is impossible to trigger a rate limiting event
2- the early_limit option is never evaluated prior to the 429 status, making it useless for its initial intention

[elasticmachine]

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[chrisberkhout]

Current behavior

The code above will:

Run the request and return if its successful or fails without StatusTooManyRequests.

If it the response has StatusTooManyRequests it will check config/headers to determine how long to wait. If the checking fails it will return. If checking was successful, after any waiting is complete it will re-enter the for loop and re-run the request.

The calling code will:

Drain the request body so the connection can be released, log failures and return the response or error.

Proposed behavior

• Draining of the body should be done before the rate limiting, ideally here.
• The rate limit checking should be done for all responses, and any wait due should be performed immediately.
• After checking/waiting:
  • the request should be retried if there was an unsuccessful response (including StatusTooManyRequests) or indicated a limit/wait (ideally including cases for which the limit is immediately reset)
  • other unsuccessful or successful responses should be returned