[sdk-platform-java] Race condition in TracedUnaryCallable causes flaky integration tests

Open
  • Is something wrong?
#12,657 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
grpc, java
Domain
observability, testing

Research direction

Start in TracedUnaryCallable and TracedBatchingCallable, then inspect the ApiFutures callback usage described in the issue. Run ITOtelGoldenMetrics to reproduce the empty-metrics failure. Done means the returned future completes only after TraceFinisher records success or failure, and the integration test no longer flakes.

Written by the indexing model from the issue text.

Description

priority: p3 type: feature request

Description

In gax-java, observability tracers (such as TracedUnaryCallable and TracedBatchingCallable) use TraceFinisher attached via ApiFutures.addCallback to record metrics and spans upon completion.

ApiFutures.addCallback(future, finisher, MoreExecutors.directExecutor());

This pattern creates a race condition in synchronous environments, such as integration tests (ITOtelGoldenMetrics), where the test thread calls future.get() and immediately asserts state (e.g., checks OpenTelemetry metrics).

Root Cause

When the underlying RPC future completes, the Guava/ApiFuture infrastructure notifies listeners. addCallback attaches a listener that runs independently of the future's resolution state for the blocking caller.

In ITOtelGoldenMetrics:

  1. client.echo() blocks the test thread.
  2. The gRPC network thread completes the RPC.
  3. The test thread unblocks and calls metricReader.collectAllMetrics().
  4. Concurrently (or slightly after), the TraceFinisher runs to record the metric in the OTel meter.

If the test thread executes before the TraceFinisher completes its work, the test fails with "empty metrics".

Proposed Solution

Convert the detached addCallback into a chained future transformation. This guarantees that the returned future only resolves after the tracer has finished recording metrics/spans.

Example for TracedUnaryCallable:

ApiFuture<ResponseT> onSuccessFuture =
    ApiFutures.transform(
        future,
        response -> {
            finisher.onSuccess(response);
            return response;
        },
        MoreExecutors.directExecutor());

return ApiFutures.catchingAsync(
    onSuccessFuture,
    Throwable.class,
    t -> {
        finisher.onFailure(t);
        return ApiFutures.immediateFailedFuture(t);
    },
    MoreExecutors.directExecutor());

This approach ensures structural LIFO ordering: the metric is recorded before any downstream consumer can yield from a blocking .get() on the returned future.

Dominant language
Java
Stars
2.1k
Forks
1.2k
Avg merge
2d 14h
Merged PRs (30d)
192

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from googleapis/google-cloud-java

All issues in googleapis/google-cloud-java

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.