feat: Add retries and gzip compression#77
Conversation
This will hopefully bring us up to standard, meaning we pass all the compliance tests. It's also cool since it saves us some bytess (at the cost of CPU of course)
posthog-elixir Compliance ReportDate: 2026-02-05 13:42:36 UTC
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 258ms |
| Format Validation.Event Has Uuid | ✅ | 259ms |
| Format Validation.Event Has Lib Properties | ✅ | 258ms |
| Format Validation.Distinct Id Is String | ✅ | 259ms |
| Format Validation.Token Is Present | ✅ | 259ms |
| Format Validation.Custom Properties Preserved | ✅ | 258ms |
| Format Validation.Event Has Timestamp | ✅ | 259ms |
| Retry Behavior.Retries On 503 | ✅ | 8070ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2262ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2261ms |
| Retry Behavior.Respects Retry After Header | ❌ | 5264ms |
| Retry Behavior.Implements Backoff | ✅ | 22109ms |
| Retry Behavior.Retries On 500 | ✅ | 6080ms |
| Retry Behavior.Retries On 502 | ✅ | 6081ms |
| Retry Behavior.Retries On 504 | ✅ | 6081ms |
| Retry Behavior.Max Retries Respected | ✅ | 30108ms |
| Deduplication.Generates Unique Uuids | ✅ | 269ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 6081ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 13075ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 6185ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 264ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 261ms |
| Compression.Sends Gzip When Enabled | ❌ | 259ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 259ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 256ms |
| Batch Format.Multiple Events Batched Together | ✅ | 264ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2262ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2261ms |
| Error Handling.Retries On 408 | ✅ | 6080ms |
Failures
retry_behavior.respects_retry_after_header
Expected at least 2 requests, got 1
compression.sends_gzip_when_enabled
Header 'Content-Encoding' with value 'gzip' not found in requests
|
Apparently not working 100% yet, will work on it! |
| doc: | ||
| "Maximum number of retry attempts for failed batch requests." | ||
| ], | ||
| gzip: [ |
There was a problem hiding this comment.
Hmm, why make it configurable?
In general, the idea behind api_client_module option was to provide an avenue to override the whole HTTP client and therefore any option. Otherwise the top-level configuration is doomed to eventually include most options that usually come with HTTP: retries, timeouts, logging, telemetry, middleware, proxies.
If the only reason to configure gzip is to let cpu-sensitive folks disable it, I imagine they would be motivated enough to just define their own client!
There was a problem hiding this comment.
100% agree. This is me trying to get to 100% on this arbitrary set of "assumptions" we made and that all clients should fulfill. This was my first dumb way to implement it, but I think I'm tending towards just implementing a client (on the compliance adapter folder) that demonstrates that this can be supported by the implementation (by implementing your own client like one would in Elixir) rather than enabling it as a config.
I won't be merging this soon, I'll iterate on it slowly.
Question for you: how do you feel with gzip being turned on by default in something like Elixir?
There was a problem hiding this comment.
Cool, let me know if you want me to take a stab at any of this!
Answering your question, I don't know much about gzip to be honest. Are there any trade offs? Looking at Req code, it calls built-in :zlib module, which is probably a C NIF under the hood.
There was a problem hiding this comment.
I would never reject a PR from you, but I can also work on this eventually, don't worry.
For gzip, there's always the fear that some people already using the SDK could be running very close to their max CPU and turning gzip on could bring them to 100% which isn't really ideal - gzip is notoriously for using CPU. We do need to turn it on to make it compliant and improve our ingestion pipeline, so I probably will do it anyway even if in the end I reach the conclusion that it needs a v3.
| when attempt > max_retries, | ||
| do: :ok | ||
|
|
||
| defp send_with_retries(api_client, events, max_retries, attempt) do |
There was a problem hiding this comment.
Req itself comes with retries plugin and I think it's even enabled by default, although maybe not for POST requests. We should be able to just configure Req and change tracked client to be nice and not cut it off.
There was a problem hiding this comment.
Lovely, I'll do more research, thank you 😄
|
#79 solves most of this, so let's close this in favor of that one, and then close the remaining bit - automatic gzip compression - in a separate PR with a different approach (as mentioned above) |
This will hopefully bring us up to standard, meaning we pass all the compliance tests. It's also cool since it saves us some bytess (at the cost of CPU of course)