googleapis / googleapis/google-cloud-java

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

Abierto
#12,657 0 comentarios 0 reacciones 0 asignados Ver en GitHub
priority: p3 type: feature request
Lenguaje dominante
Java
Estrellas
2.1k
Forks
1.2k
Merge medio
1 d 23 h
PR fusionados (30 d)
154

Descripción

## 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.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.