elastic / elastic/integrations

[Greenhouse] Improve rate limiting behavior

Open
#18,339 5 comments 0 reactions 1 assignee Claimed by @narph View on GitHub
enhancement Integration:greenhouse Team:Security-Service Integrations
Dominant language
Handlebars
Stars
333
Forks
647
Avg merge
2d 17h
Merged PRs (30d)
225

Description

## Summary

Mock testing of the Greenhouse Audit Log integration revealed ~~two issues: an incorrect OAuth token request parameter and~~ missing rate limit handling that leads to excessive API requests.

## ~~OAuth Token Request~~

> [!NOTE]
> This section has been retracted — see [follow-up comment](#issuecomment) with test matrix. Body-based `grant_type` is correct for this integration's usage.

~~The Greenhouse Harvest V3 OAuth [documentation](https://harvestdocs.greenhouse.io/reference/generate-token) specifies that the `grant_type` parameter must be sent as a **query parameter** on the token endpoint:~~

> ~~`POST https://auth.greenhouse.io/token?grant_type=client_credentials`~~

~~The current implementation sends `grant_type` as a form-encoded POST body parameter instead. While the API appears to accept both forms today, the integration should match the documented contract. The optional `sub` parameter for service account impersonation is correctly documented as a body parameter and should remain there.~~

## Rate Limiting

The Greenhouse Audit Log API enforces two rate limits:

- **General:** 50 requests per 10 seconds
- **Pagination:** 3 requests per 30 seconds

The API returns `X-RateLimit-Limit` and `X-RateLimit-Remaining` headers on successful (200) responses. However, when the limit is exceeded, the 429 response includes **no rate limit headers** — no `Retry-After`, no `X-RateLimit-Reset`, and no `X-RateLimit-Remaining`.

### Observed behavior

During testing with pagination, the integration quickly exceeds the 3/30s pagination limit. Once a 429 is returned, the CEL input's underlying HTTP client (`retryablehttp.DefaultRetryPolicy`) automatically retries the request with exponential backoff. These retries are invisible to the CEL program and consume additional rate limit budget, compounding the problem. Below is a sample from the API request log showing the pattern — rapid 200s followed by a cascade of 429 retries that eventually succeed once the rate limit window resets:

```
14:42:00 GET /events 200 ← initial requests succeed
14:42:00 GET /events 200
14:42:00 GET /events 200
14:42:01 GET /events 429 ← rate limit hit
14:42:02 GET /events 429 ← retryablehttp retry (~1s backoff)
14:42:04 GET /events 429 ← retry (~2s backoff)
14:42:08 GET /events 429 ← retry (~4s backoff)
14:42:16 GET /events 200 ← window resets, burst of retried requests succeed
14:42:16 GET /events 200
14:42:16 GET /events 200
... (15 total requests at 14:42:16)
```

The CEL program does handle 429 explicitly, but that handler never executes because the HTTP-level retries mask the 429 from the program.

### Recommended strategy

**Proactive pacing with `resource.rate_limit`:** Configure `resource.rate_limit.limit` and `resource.rate_limit.burst` in the agent config template to enforce a token-bucket rate of 1 request per 10 seconds. This stays within the 3/30s pagination limit and applies at the HTTP transport layer before requests are sent.

**Response header budget check:** On each successful `/events` response, read `X-RateLimit-Remaining` and stop pagination (`want_more: false`) when the remaining budget is exhausted. The cursor should preserve pagination state (`pit_id`, `search_after`) so collection can resume on the next interval.

**Why not `rate_limit()` from Mito?** The `rate_limit()` CEL function requires a `Reset` header (or synthesized equivalent) to compute when the rate limit window resets. The Greenhouse API does not provide this header. Synthesizing a value is possible but problematic because `rate_limit()` updates the same `rate.Limiter` instance used by `resource.rate_limit`, and the computed `next` rate after reset would override the static pagination pacing. The simpler two-layer approach (static pacing + header budget check) avoids this interaction.

**Why not `resource.retry.max_attempts: 1`?** Disabling HTTP-level retries would prevent the 429 retry cascade, but it also disables retries for transient 5xx errors where retries are genuinely useful. The proactive approach above makes this unnecessary since 429s should not occur in the first place.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.