feat: [SDK-5065] add bounded retry/backoff to remote log export

[abdulraqeeb33]

What OTel did by default, and what we lost

The remote-log exporter used to be OtlpHttpLogRecordExporter, which came with retry
turned on without anyone writing a config line for it — which is exactly why nobody
noticed when it went away. In the pinned 1.55.0:

• io/opentelemetry/exporter/internal/RetryUtil hardcodes the retryable HTTP statuses
  429, 502, 503, 504.
• opentelemetry-exporter-sender-okhttp ships a RetryInterceptor that applies
  exponential backoff with jitter across roughly 5 attempts.

The hand-rolled replacement does one POST and throws the batch away:
LogBatchProcessor caught the export failure and deliberately dropped
("best-effort drop (no retry)"), and LogTelemetryRemoteImpl.exportBatch discarded
post()'s return value, so a 503 was indistinguishable from a 200.

Consequence: one transient backend blip permanently lost up to 100 remote log records
(maxQueueSize / maxExportBatchSize are both 100), and a 429 was ignored rather than
backed off, so a rate-limited client kept posting at the same cadence.

Retry policy

ExportRetrier (kmp/src/commonMain/.../internal/LogExportRetry.kt) sits in the shared
batch/export path, so Android and iOS both inherit it. Defaults mirror OTel's:

Bound	Value
Max attempts	5 (1 initial + 4 retries)
Initial backoff	1s
Multiplier	1.6x
Max backoff	5s
Jitter	±20%
Max total backoff	15s

What is and is not bounded. The attempt count is the real bound, and it holds no
matter how slow each attempt is. The 15s is charged against sleeping only, and at 5
attempts the schedule already sums to ~9.3s, so it binds only if maxAttempts is raised;
it is there so raising it cannot silently produce an unbounded sleep. The final backoff is
clamped to whatever budget is left.

There is deliberately no wall-clock ceiling. This retrier cannot cancel a send once it
is running — ILogHttpSender takes no deadline and neither platform implementation accepts
one — so a wall-clock ceiling could only be checked between attempts, which bounds nothing:
an attempt starting just inside the budget still runs to the sender's own 10s timeout. It
also under-delivers exactly where retry matters most; against connect timeouts a 15s
ceiling with 10s timeouts yields ~2 attempts, not 5. So the honest statement of the worst
case for one export is maxAttempts × senderTimeout + total backoff — ~59s with today's
defaults — on a background export coroutine with no caller blocked on it.

Retryable vs permanent. classifyStatus treats 429/502/503/504 as retryable, plus
transport-level failures — a thrown sender, or statusCode == -1, the sentinel both
platform senders already return when there is no HTTP response (DNS, timeout, reset).
Everything else is permanent: 4xx, the -2 "remote logging disabled" sentinel, and the
-3 "could not build the request" sentinel iOS returns
(OneSignal-iOS-SDK#1727).

No contract change. ILogHttpSender / LogHttpResponse are untouched — retryability
is derived entirely from the statusCode both senders already report, so there is no
follow-up needed in the Android or iOS repos.

Retry-After is not honored — deliberately. Neither platform sender surfaces response
headers, so plumbing it through means adding a field to LogHttpResponse. Kotlin default
arguments are not exported to Objective-C, so a new constructor parameter would break the
Swift build until the iOS repo was updated in lockstep. That is not cheap, so it is left
out rather than half-done. Jittered exponential backoff still de-escalates against a 429.

Teardown

Retrying happens inside onExport, which the batch processor runs under exportMutex —
the same mutex forceFlush and shutdown need. A teardown landing mid-retry would
therefore queue behind a cycle whose backoffs alone (~9s) outlast the 5s shutdown flush
budget: the caller blocks for the full 5s and the batch is dropped anyway. On Android that
caller is a lifecycle thread — a log-level change routes through LoggerLifecycleManager
under runBlocking — so a config change during a backend degradation would deterministically
stall it.

shutdown() completes an abort signal before asking for the flush. That wakes an in-flight
backoff immediately and stops further attempts, so the wait collapses to at most the single
attempt already in flight. Raising the flush timeout instead would just block that lifecycle
thread longer. The aborted batch is not requeued: it is mid-retry precisely because the
backend is rejecting it, so a final attempt would only cost another round trip during
teardown.

enqueue only takes the separate buffer mutex, so new records keep flowing while a retry is
in flight; they fill the existing bounded queue and are dropped past maxQueueSize as
before. Only the batch under retry is held, so memory is capped at two batches. Waiting is
suspension, never a blocking sleep, and CancellationException propagates.

Happy path is unchanged: one POST, no added latency, no delay ever entered.

Out of scope

The crash-upload path is untouched. LogCrashUploader.sendReports already retries
correctly — it deletes a record only after success and breaks on first failure, so
failures retry on the next launch. exportEncoded, which it calls, still uses the
original single-shot post, and two tests pin that split.

Tests

kmp/src/commonTest/kotlin/com/onesignal/logger/LogExportRetryTest.kt, 18 tests, using the
existing FakeHttpSender (which gained an exceptions queue so a send can throw). Timing
is asserted in virtual time via the runTest scheduler, so there are no wall-clock sleeps.

• happyPathPostsExactlyOnce
• retryableStatusIsRetriedUntilSuccess (503 → 429 → 200, same body each attempt)
• transportFailureIsRetried (thrown sender, then -1, then 200)
• permanentStatusIsNotRetried (400)
• attemptCapIsHonoredWhenBackendKeepsFailing
• exportEncodedIsNotRetriedOnARetryableStatus / ...WhenTheSenderThrows (crash path stays single-shot)
• backoffFollowsTheAdvertisedSchedule (1000/1600/2560/4096/5000/5000 — the table above)
• jitterScalesEachDelayByTheConfiguredFactor (±20%)
• theBackoffBudgetClampsTheLastDelayAndThenStopsRetrying
• theAttemptCapHoldsNoMatterHowSlowEachAttemptIs (10s attempts still get all 5)
• anAbortSignalWakesAnInFlightBackoffImmediately
• anAlreadyAbortedSignalStillLetsTheAttemptInFlightFinish
• shutdownFlushesOnceWithoutEnteringARetryCycle
• aCancelledSenderIsNotReclassifiedAsATransientFailure
• cancellationDuringBackoffDelayPropagates
• aRequestThatCouldNotBeBuiltIsPermanent, classifiesStatusCodes

Every test was checked against a deliberately broken implementation and observed to fail:
dropping the per-delay cap and the jitter breaks the three schedule tests; wrapping the loop
in a wall-clock withTimeout drops theAttemptCapHolds... to 2 attempts; replacing the
abort-aware wait with a plain delay breaks both abort tests; and removing the
shutdownSignal.complete(...) line makes shutdownFlushesOnce... post 3 times before the
5s budget cancels it.

./gradlew :kmp:allTests spotlessCheck passes. Confirmed from the result XML that all 18
ran with 0 failures on both iosSimulatorArm64Test and testDebugUnitTest.

[fadi-george]

Retry path looks right for fast 429/503. A few things the claims and tests don't currently pin down:

1. Elapsed cap vs in-flight HTTP. maxElapsedMillis is checked after attempt(), then delay is clamped and another send() always starts. In-flight work is not cancelled, so stall can be 15s plus one sender timeout, not 15s. A 10s connect timeout (the -1 case) only gets about 2 attempts, not 5. Either charge the ceiling against backoff only, raise it above attempts × timeout, or drop the "consumer stalls at most 15s" wording.

2. Shutdown is 5s, retry delays alone are ~9s. Retry holds exportMutex for the whole cycle, so disable/teardown that overlaps a 503 waits 5s, cancels, and drops the drained batch. Align the budgets or cancel the in-flight export instead of waiting on that mutex.

3. Tests. elapsedCapStopsRetriesBeforeAttemptCap advances the clock on every nowMillis() read, including start, and only asserts attempts < 10. Nothing asserts backoff duration. Also missing: exportEncoded + 503 still posts once.

Happy to ignore the rest (500 not retried, no Retry-After) as intentional.

[abdulraqeeb33]

Thanks — all three land. Partly addressed as of 30b5031, and I'd rather give you the honest split than claim it's all done.

3. Tests — mostly done. elapsedCapStopsRetriesBeforeAttemptCap no longer advances the clock on every read (the fake sender advances it explicitly now) and asserts an exact attempt count rather than < 10 — the old assertion would have passed at 9, i.e. against a nearly-broken ceiling. exportEncoded + 503 is covered by exportEncodedIsNotRetriedOnARetryableStatus, plus a companion for a throwing sender, since that bypass is the thing keeping crash records off the in-process retry.

You're right that nothing asserts backoff duration — that's still missing. With nextRandom pinned the schedule is deterministic, so there's no excuse for the advertised numbers being untested. Adding it.

1. Elapsed cap — you're right, and the second half is the part I'd underweighted. I'd added a budget re-check after the backoff, which stops a new attempt starting once the budget is spent but does nothing about an attempt that starts just inside it and then sits on a 10s sender timeout. So the ceiling still isn't a wall-clock bound.

The observation that a 15s ceiling against 10s timeouts yields ~2 attempts rather than 5 is the sharper problem: the policy under-delivers precisely on the slow-failure case retry exists for, and no wording change fixes that. Leaning toward charging the ceiling against cumulative backoff only — it makes the attempt count predictable and honest, at the cost of a real worst case of attempts × timeout that the KDoc then has to state plainly rather than claiming a 15s stall bound. Will make code and docs agree either way.

2. Shutdown budget — not addressed yet, and it's worse than the numbers suggest. Agreed on the 5s-vs-~9s mismatch. The part that makes it urgent is where that path runs from: on Android shutdown() is reachable from a config HYDRATE via LoggerLifecycleManager under runBlocking, so a log-level change during a backend degradation deterministically blocks a lifecycle thread for the full 5s and discards the drained batch anyway — worst of both.

Planning to cancel the in-flight export rather than align the budgets. Raising the timeout just means blocking a lifecycle thread longer for a batch we're likely to drop regardless.

Agreed on leaving 500-not-retried and Retry-After as intentional. On Retry-After specifically: honoring it needs a new field on LogHttpResponse, and since Kotlin default arguments don't cross into Objective-C that's a breaking change for the Swift senders — not worth coupling three repos for a marginal gain over jittered backoff.

Will follow up here when 1 and 2 are pushed.

[abdulraqeeb33]

Following up as promised — 1 and 2 are pushed in 331d6df, and the first one changed the design rather than the wording.

1. Elapsed cap. Took your first option: the ceiling is charged against backoff only. maxElapsedMillis is now maxTotalBackoffMillis, accumulated from the delays actually slept, and the injected clock is gone.

Your second sentence is what decided it. A wall-clock ceiling isn't enforceable here at all — ILogHttpSender takes no deadline and neither platform sender accepts one, so the retrier can never cancel a send in progress and any ceiling is only ever a between-attempts check. It also fails asymmetrically: it binds hardest against slow failures, which is the case retry exists for. So it was costing real attempts against connect timeouts while bounding nothing.

The cost, now stated plainly in the ExportRetrier KDoc instead of implied: worst case for one export is maxAttempts × senderTimeout + backoff, roughly 59s at defaults with a 10s sender timeout — a background coroutine with no caller blocked on it. The "elapsed-time ceiling OTel did not have" line is gone, as are the PR's "stalls at most 15s" and "we never sleep past the ceiling" claims. The 15s survives as a sleeping-only guard rail, and the KDoc notes it doesn't bind until someone raises maxAttempts, since the schedule sums to ~9.3s at five.

theAttemptCapHoldsNoMatterHowSlowEachAttemptIs keeps it honest — put a wall-clock ceiling back and it drops to 2 attempts, your arithmetic exactly.

2. Shutdown. Cancelling rather than aligning budgets, for the reason you gave. shutdown() completes an abort signal before requesting the flush; the retrier waits on withTimeoutOrNull(wait) { abort.await() } instead of a bare delay, so an in-flight backoff wakes immediately and no further attempt starts. The wait collapses from the full ~9s cycle to at most the single attempt already in flight, so the 5s budget only ever has to cover one send — SHUTDOWN_FLUSH_TIMEOUT_MILLIS documents that now rather than being a number that happened to be smaller than the retry cycle.

Abort stops retrying without cancelling work, so the attempt in flight finishes and shutdown's flush still gets one honest post. I did not requeue the aborted batch — it's mid-retry precisely because the backend is rejecting it, so a final attempt just buys another round trip during teardown. Happy to change that if you'd rather keep it.

3. Backoff duration. backoffFollowsTheAdvertisedSchedule pins [1000, 1600, 2560, 4096, 5000, 5000] in virtual time — initial, multiplier, and the per-delay cap. Plus a ±20% jitter test and a budget-clamp test that asserts the clamped 400ms rather than just the attempt count, so "return early instead of clamping" can't slip through.

Every new test was run against a deliberately broken implementation first and observed to fail, one perturbation at a time. The one I'd single out is shutdownFlushesOnceWithoutEnteringARetryCycle: without the abort signal it posts three times before the 5s budget cancels it, which is the regression this whole item is about.

18 tests, 0 failures on both iosSimulatorArm64Test and testDebugUnitTest. 500-not-retried and Retry-After left alone as agreed.

[fadi-george]

nits:

• forceFlush() can still wait out a full retry cycle. Fine for a suspend API; shutdown was the one that mattered.
• No integration test of shutdown() while a telemetry retry is parked in backoff. The retrier abort test covers the mechanism.
• classifyStatus’s comment still only mentions -2. -3 has its own test.

[abdulraqeeb33]

All three fair. 0bbeb4c.

classifyStatus comment. Fixed — both sentinels are now named at the branch that decides them, so -3 doesn't look like it falls through by accident.

forceFlush() waiting out a retry cycle. Agreed, leaving it. It's a suspend API with no lifecycle thread behind it, and the caller opted into waiting.

Integration test of shutdown() during a parked backoff. Worth reporting what happened here, because the answer changed twice.

The coverage does exist, just not where the name suggests: shutdownFlushesOnceWithoutEnteringARetryCycle runs through the real telemetry, and dropping abortSignal = shutdownSignal from exportBatch makes it post three times instead of once. I verified that by actually removing the wiring rather than assuming.

What it doesn't do is park in a backoff first, so I wrote the variant you're describing. It passed against that same broken build — asserting nothing. The reason is structural: shutdown() is deliberately non-suspend and uses runBlocking, so it doesn't interleave with runTest's virtual clock, and the parked job's completion can't be attributed to the abort. I dropped it rather than ship a green test that proves nothing, and left a comment recording the constraint so the next person doesn't spend the same hour on it. anAbortSignalWakesAnInFlightBackoffImmediately covers the wake-from-backoff behaviour directly on the retrier, which is the part virtual time can actually observe.

If you'd rather have that case covered end to end, the way in is making shutdown() suspend — but that changes the API for every caller including the Android lifecycle path, so it seemed like the wrong trade for test ergonomics.

18 tests, 0 failures on iosSimulatorArm64Test and testDebugUnitTest.