[BUG] Keep the curl retry deadline stable within an attempt

[shixi-li]

Fixes #4403

NextRetryTime() drew a new jitter sample on every call, so polling the same retryable request moved its deadline and biased the realized delay toward the lower end of the backoff band.

Changes

• calculate and store one retry deadline when each curl attempt completes
• make the stored deadline visible before the synchronous Response event and reuse it for scheduling
• preserve valid Retry-After deadlines and use one stable jittered backoff when the header is missing or invalid
• cover repeated reads, response-event visibility, invalid-header fallback, and later attempts

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed (no public API surface is changed)

Tests

• focused curl retry deadline and exponential-backoff tests
• full curl_http_test suite
• ./ci/do_ci.sh cmake.maintainer.sync.test
• ./ci/do_ci.sh cmake.test
• ./ci/do_ci.sh cmake.exporter.otprotocol.test
• ./ci/do_ci.sh format
• markdownlint .

Coverage notes

The focused cases exercise the shared completion path through synchronous Send(). The existing full client suites cover async operation, but this change does not add an end-to-end timing assertion for the pending retry queue. Existing HTTP-date and over-max-backoff Retry-After behavior is unchanged. Local runtime validation was on Linux/amd64.

[codecov]

Codecov Report

❌ Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.85%. Comparing base (3603e13) to head (b699ff2).

Files with missing lines	Patch %	Lines
ext/src/http/client/curl/http_operation_curl.cc	86.37%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4452      +/-   ##
==========================================
+ Coverage   82.63%   82.85%   +0.23%     
==========================================
  Files         512      512              
  Lines       20138    20145       +7     
==========================================
+ Hits        16639    16690      +51     
+ Misses       3499     3455      -44     

Files with missing lines	Coverage Δ
...lemetry/ext/http/client/curl/http_operation_curl.h	91.31% <ø> (ø)
ext/src/http/client/curl/http_operation_curl.cc	62.99% <86.37%> (+2.40%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mateenali66]

the defect is real. doRetrySessions calls operation->NextRetryTime() on every poll (http_client_curl.cc:860) and every call redraws dis(gen).

one correction to the description. "biased toward the lower end" is not the mechanism, and at the first attempt with defaults it is not the direction either. simulating the release rule at the default 256ms scheduled_delay_milliseconds_ (:278), 20k runs, against drawing once:

backoff	redraw mean, band	stable mean, band
1s	1136 [1024-1280]	998 [800-1200]
5s	4805 [4096-6144]	5002 [4000-6000]
30s	26126 [24064-31232]	29981 [24001-36000]

at 1s the mean goes up, because poll quantisation dominates when the backoff is only four polls wide. the low bias appears once the backoff grows past that, and sharpens as the poll interval shrinks relative to it.

the band narrowing is the part worth citing. NextRetryTime's own comment says the jitter is there "to avoid hammering servers at the same time from a large number of clients", and a redrawn deadline pulls every client onto the same edge of the band.

the rename is contained, retry_after_time_point_ has no readers outside this file and NextRetryTime has one production caller. CI is green but the branch is BEHIND and has not moved since 19 Aug.

[mateenali66]

reviewed the retry path against main. the deadline caching looks right and the tests cover what the issue described.

the one inline note is about the NextRetryTime() fallback. the reason I am raising it rather than letting it go is a second change this PR makes that the description does not mention.

IsRetryable() moves from after the DispatchEvent sequence to before it. previously the value computed inside PerformCurlMessage and the one the scheduler computes at http_client_curl.cc:539 sat on the same side of the
event handlers. now three synchronous dispatches run between them. i cannot find anything in tree that breaks the agreement between those two calls, so this is not a bug report. it is that the invariant the fallback quietly relies on is
weaker after this change than before it, which is why i would rather the fallback were a deliberate choice than a leftover.

one question on coverage: with ENABLE_OTLP_RETRY_PREVIEW off, IsRetryable() returns false unconditionally and the whole reordered block is a no op. are the new tests running under a job that defines it, or only locally?

[mateenali66]

this fallback is the behaviour #4403 describes, recomputing a fresh jittered value on every call.

i think it is unreachable as written. http_client_curl.cc:870 is the only non test caller and it only walks pending_to_retry_sessions_, which is filled at http_client_curl.cc:539 behind operation->IsRetryable(). whenever is_retryable is true this PR always stores a deadline, either the parsed
Retry-After or CalculateNextRetryTime() at line 1606. so nothing reaches the fallback and the old behaviour survives in a branch nothing exercises.

would you rather drop it so a missing deadline surfaces as a visible bug, or keep it with an assert naming the invariant? see the summary for why the invariant is worth naming.