Align OTLP HTTP retry behaviour with the specification - #5460
Conversation
Per the OTLP spec, only 429, 502, 503 and 504 are retryable, and a Retry-After header on such a response must be honoured, falling back to exponential backoff only when it is absent. The HTTP exporters did neither: 429 was dropped immediately while 408 and every 5xx code was retried, and the backoff was computed before the request was sent, so Retry-After could never influence it. Retry exactly the four spec-mandated codes and let a parseable Retry-After value (delay-seconds or HTTP-date) override the backoff for that attempt, matching the rules the experimental otlp-common client already implements.
ae2fe5c to
bf6016f
Compare
|
This is already being addressed in #5389 |
Thanks — I saw #5389. That's a larger refactor that fixes this incidentally via the shared otlp-common client. #5460 is a small, targeted fix for the stable otlp-proto-http package: today 429 is dropped instead of retried and Retry-After is ignored — a spec-compliance bug users hit right now, and it's easy to backport. |
I'd prefer we just wait for #5389 since the changes in this targeted fix would end up being removed anyways once its merged (because of the larger refactor) |
Description
Aligns the retry behaviour of the three OTLP HTTP exporters (traces, metrics, logs) in
opentelemetry-exporter-otlp-proto-httpwith the OTLP specification:_is_retryabletreated 429 as permanent (dropping the batch immediately) while retrying 408 and every 5xx code.Retry-Afterheader: the spec asks clients to honour aRetry-Afterheader on a retryable response, falling back to exponential backoff only when it is absent. Previously the backoff was computed before the request was sent, so the header could never influence the delay.A new
_extract_retry_afterhelper parses both RFC 7231 forms (delay-seconds and HTTP-date). An unparseable or non-finite value falls back to exponential backoff, and an already-elapsed delay retries immediately. ARetry-Afterlonger than the remaining export deadline still ends the export, preserving the existing timeout contract.The experimental
opentelemetry-exporter-otlp-commonshared client already implements these rules; this brings the stable HTTP exporters in line with it without sharing code between a stable and an experimental package.Type of change
How Has This Been Tested?
Ran the full OTLP HTTP exporter test suite (97 tests passed), including new tests added in this PR:
tests/test_common.py: unit tests for_extract_retry_after— delay-seconds, negative delay clamped to zero, non-finite and malformed values falling back to backoff, HTTP-date in the future/past, and naive HTTP-date treated as UTCtests/test_proto_span_exporter.py,tests/test_proto_log_exporter.py,tests/metrics/test_otlp_metrics_exporter.py: per-exporter tests that 429 is retried (test_too_many_requests_is_retried) and that aRetry-Afterheader overrides the exponential backoff (test_retry_after_header_overrides_backoff)