[BUG] Stop OTLP ForceFlush returning on the first notification and overrunning its deadline

[thc1006]

Towards #4339. @owent confirmed the inverted comparison and suggested bounding the
wait by the caller's remaining time, which is what this does. It does not close the
issue; see "What this leaves" below.

OTLP gRPC reported the whole flush done on one completion

OtlpGrpcClient::ForceFlush left its wait loop on any notification:

if (std::cv_status::timeout !=
    async_data_->session_waker.wait_for(lock, async_data_->export_timeout))
{
  break;
}

std::cv_status::timeout != status is true when the wait was notified, and the
loop condition is the only place finished_request_counter is read, so the break
skipped it. One finished request out of many ended the wait, and the function then
returned the leftover duration rather than the predicate.

Rather than inverting the test I dropped the break and let the loop condition
decide, which is what the == form reduces to once the wait is bounded. The bounded
wait_for is what covers a notification lost between the check and the wait; the
break was not doing that. The return is the counter predicate now.

The comment above it described running_sessions_ and gc session cleanup, neither of
which exists in the gRPC client. It is a verbatim copy of the OtlpHttpClient one,
down to the missing space in forever.We, so it is replaced rather than kept.

Both clients waited on the exporter's timeout, not the caller's

ForceFlush(1ms) could block for the full configured export timeout. The wait
interval is min(configured timeout, caller's remaining time) now, as suggested. No
notification path and no lock discipline changes.

OtlpHttpClient::Shutdown is where that shows up in practice:

// Wait util all sessions are canceled.
while (cleanupGCSessions())
{
  ForceFlush(std::chrono::milliseconds{1});
}

Each of those calls could take options_.timeout, so shutdown cost a multiple of the
export timeout rather than a multiple of 1 ms.

The result is the predicate, on both clients

Both loops used to end by returning what was left of the deadline. A completion that lands between
the last check and the last wait takes its notification with it, and the wait then consumes the rest
of the deadline and reports a timeout for work that had already finished. Each client now returns
the condition it was waiting on.

One thing I tried and backed out

OtlpGrpcClient increments finished_request_counter before it runs grpc_async_callback, which
is where the ExportResult reaches the caller, so a bounded wait can see the counter satisfied
while the result is still on its way. Moving the increment after the callback closes that, and I
had it that way for a while.

It is not worth it here. DelegateAsyncExport is public and takes the callback, so a caller whose
callback calls ForceFlush() would wait for an increment that cannot happen until the callback
returns, and the no-deadline form of that wait never ends. Trading a bounded early signal for an
unbounded self-wait is the wrong way round, and neither of the two bugs this PR fixes needs the
change. Where a request counts as finished belongs with the identity work #4339 still needs.

Test

In otlp_http_exporter_custom_client_test.cc,
ForceFlushReturnsWithinTheCallerDeadline holds a request open with the nosend HTTP
client and asserts ForceFlush(50ms) returns inside its deadline against a 30 second
client timeout. It drives OtlpHttpClient::Export directly with a non-zero request
budget, so it does not depend on ENABLE_ASYNC_EXPORT.

	result	elapsed
otlp_http_client.cc reverted to main, test kept	Expected: (elapsed) < (1000), actual: 30000 vs 1000	30000 ms
this branch	passed	50 ms

Built and run in two configurations, WITH_ASYNC_EXPORT_PREVIEW off and on, six of six
passing in both, whole file 51 ms.

The file carries the #ifndef OPENTELEMETRY_STL_VERSION guard its neighbours in
exporters/otlp/test already use, so like them it builds under WITH_STL=OFF and is
empty in the cmake.c++*.stl.test jobs.

The gRPC half is covered through the stub. OtlpGrpcClientAsyncData is defined in the
.cc, so no test can read finished_request_counter directly, but it does not have
to: OtlpMockTraceServiceStub was running the completion callback inline, so every
request was already finished by the time ForceFlush could be called and the loop
never had anything to wait for. Holding the callback instead leaves the request in
flight, which is all these two cases need.

In otlp_grpc_exporter_test.cc, OtlpGrpcExporterFlushTestPeer carries two cases.
ForceFlushReturnsWithinTheCallerDeadline asks for 50 ms against the 10 second export
timeout. ForceFlushKeepsWaitingWhenAnotherRequestCompletes leaves two requests in
flight and completes one while the flush is parked, which is the notification that used
to end the wait.

	result	elapsed
otlp_grpc_client.cc reverted to main, tests kept	Expected: (elapsed.count()) < (5000), actual: 10000 vs 5000	10000 ms
same, second case	Value of: flushed / Actual: true / Expected: false	100 ms
this branch	both passed	78 ms, 1007 ms

The second row is the bug in one line: the flush returned success at the exact moment a
different request completed, with the one it was waiting for still in flight.

56 of 56 with WITH_ASYNC_EXPORT_PREVIEW=ON. With it off the client tracks no
in-flight requests, so the two cases skip in the fixture rather than being compiled
away. gtest_add_tests registers from the source text, so an #if-ed out case is
still registered with CTest and then passes without running. CTest lists both, and the
binary reports 54 passed, 2 skipped.

What this leaves

ForceFlush still does not fully meet the contract in your point 1. Both clients
snapshot a monotonic total and wait for the finished total to reach it, so a request
that starts after the call can satisfy the snapshot while one that was pending
before it is still running. On entry started=10 finished=8; two new requests
start and finish; finished reaches 10 and the wait ends with the original two in
flight.

Reading both clients for that follow-up turned up four more places where the boundary
sits in the wrong spot, all of them on main and none of them touched here:

• a request is not registered until after the exporter has serialised it, so a flush
  during PopulateRequest sees nothing;
• the HTTP registration is two steps, the running_sessions_ insert under the lock and
  start_session_counter_ after it;
• the HTTP loop leaves on running_sessions_.empty(), which counts sessions started
  after the call, so a flush can also wait too long;
• Unbind runs the caller's callback before ReleaseSession, deliberately, because the
  response lives in an arena the session data owns.

All five want the same thing, a token issued at the exporter's Export entry rather
than a counting total, which also settles the wrap a monotonic total has on a 32 bit
target. That is a change to both hot paths, so I kept it out of one you had already
scoped and wrote the set up on #4339 instead. Happy to send it as a follow-up.

Behaviour change worth knowing

OtlpGrpcClient::Shutdown() defaults to a timeout of 0, documented as "no timeout is
applied". With the break gone that call waits for outstanding requests instead of
returning after the first completion, so an application that exported and exited
immediately will now see the export finish rather than be cancelled.

It does not wait indefinitely under a normal configuration: a call sets
context->set_deadline(now() + options.timeout) whenever options.timeout is
positive, so each one completes or fails within the export timeout and increments the
counter either way. A caller that wants the old return-early behaviour can pass an
explicit short timeout.

One thing I noticed but did not touch

OtlpGrpcClientOptions::timeout and OtlpHttpClientOptions::timeout both default to
a zero duration, and the exporters fill in a real value. If a client is left with zero,
set_deadline is skipped, and in ForceFlush the wait interval is zero as well, so
the loop spins instead of sleeping. main does the same thing there, since
wait_for(lock, 0) returns cv_status::timeout and the old != test then did not
break either, so this is not something the PR changes. I left it alone because the
sensible re-check interval for "no configured timeout" is a policy call, and falling
back to the caller's remaining time would reintroduce the overflow that
AdjustWaitForTimeout exists to avoid. A sketch of the policy is on #4339 with the
rest of the follow-up.

Checklist

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed: no signature changes, but ForceFlush returns
  false in cases where it used to return true, and a default Shutdown() now waits.
  Both are described above.

CI note

func_otlp_grpc has a pre-existing nondeterministic double free that aborts after the
tests report passing, and it reds gcc-14 and clang-18 on main as well. It is not
related to this change.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.89%. Comparing base (89729e1) to head (d0dc3c9).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4357      +/-   ##
==========================================
+ Coverage   80.86%   80.89%   +0.03%     
==========================================
  Files         450      450              
  Lines       19216    19224       +8     
==========================================
+ Hits        15537    15549      +12     
+ Misses       3679     3675       -4     

Files with missing lines	Coverage Δ
exporters/otlp/src/otlp_grpc_client.cc	71.06% <100.00%> (+0.43%)	⬆️
exporters/otlp/src/otlp_http_client.cc	71.35% <100.00%> (+0.60%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[thc1006]

The one red job is a Docker Hub timeout in the functional test setup, before anything
is compiled:

#2 [internal] load metadata for docker.io/otel/opentelemetry-collector:0.123.0@sha256:c8e3...
#2 ERROR: failed to do request: Head "https://registry-1.docker.io/v2/otel/opentelemetry-collector/manifests/sha256:...": dial tcp
ERROR: failed to build: failed to solve: DeadlineExceeded

The same job passed on the previous commit of this branch,
https://github.com/open-telemetry/opentelemetry-cpp/actions/runs/30941753951, and the
two commits differ by two lines of CHANGELOG.md and nothing else, so no part of the
diff reaches it.

It is also not the func_otlp_grpc double free in the CI note above. That one aborts
after the tests report passing; this one never got to the build.

I do not have re-run rights on the repository. Happy to force-push the same tree to
retrigger it if that is easier than a re-run.