googleapis / googleapis/google-cloud-java

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

Offen
#12,657 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
priority: p3 type: feature request
Vorherrschende Sprache
Java
Sterne
2.1k
Forks
1.2k
Ø Merge
1 T. 23 Std.
Gemergte PRs (30 T.)
154

Beschreibung

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

```java
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`:

```java
ApiFuture 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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.