googleapis / googleapis/google-cloud-java

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

オープン
#12,657 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
priority: p3 type: feature request
主要言語
Java
スター
2.1k
フォーク
1.2k
平均マージ
1日 23時間
マージ済み PR(30日)
154

説明

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

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。