Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Commit

Permalink
feat: pass request in ApiTracer (#1491)
Browse files Browse the repository at this point in the history 
* feat: pass request or response in ApiTracer

* add comments

* make change backward compatible

* revert attemptSucceeded changes

Co-authored-by: Igor Bernstein <igorbernstein@google.com>
  • Loading branch information
@mutianf @igorbernstein2
mutianf and igorbernstein2 authored Oct 27, 2021
1 parent d4222e7 commit 27bf265
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 27 deletions.
2 changes: 1 addition & 1 deletion gax/src/main/java/com/google/api/gax/rpc/AttemptCallable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public ResponseT call() {

callContext
.getTracer()
.attemptStarted(externalFuture.getAttemptSettings().getOverallAttemptCount());
.attemptStarted(request, externalFuture.getAttemptSettings().getOverallAttemptCount());

ApiFuture<ResponseT> internalFuture = callable.futureCall(request, callContext);
externalFuture.setAttemptFuture(internalFuture);
Expand Down
2 changes: 1 addition & 1 deletion gax/src/main/java/com/google/api/gax/rpc/ServerStreamingAttemptCallable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public Void call() {

attemptContext
.getTracer()
.attemptStarted(outerRetryingFuture.getAttemptSettings().getOverallAttemptCount());
.attemptStarted(request, outerRetryingFuture.getAttemptSettings().getOverallAttemptCount());

innerCallable.call(
request,
Expand Down
12 changes: 12 additions & 0 deletions gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,21 @@ public interface ApiTracer {
* start of the operation. The attemptNumber is zero based. So the initial attempt will be 0.
*
* @param attemptNumber the zero based sequential attempt number.
* @deprecated Please use {@link #attemptStarted(Object, int)} instead.
*/
@Deprecated
void attemptStarted(int attemptNumber);

/**
* Adds an annotation that an attempt is about to start with additional information from the
* request. In general this should occur at the very start of the operation. The attemptNumber is
* zero based. So the initial attempt will be 0.
*
* @param attemptNumber the zero based sequential attempt number.
* @param request request of this attempt.
*/
void attemptStarted(Object request, int attemptNumber);

/** Adds an annotation that the attempt succeeded. */
void attemptSucceeded();

Expand Down
5 changes: 5 additions & 0 deletions gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ public void attemptStarted(int attemptNumber) {
// noop
}

@Override
public void attemptStarted(Object request, int attemptNumber) {
attemptStarted(attemptNumber);
}

@Override
public void attemptSucceeded() {
// noop
Expand Down
6 changes: 6 additions & 0 deletions gax/src/main/java/com/google/api/gax/tracing/OpencensusTracer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,12 @@ public void attemptStarted(int attemptNumber) {
// This simply is used for state management.
}

/** {@inheritDoc} */
@Override
public void attemptStarted(Object request, int attemptNumber) {
attemptStarted(attemptNumber);
}

/** {@inheritDoc} */
@Override
public void attemptSucceeded() {
Expand Down
33 changes: 17 additions & 16 deletions gax/src/test/java/com/google/api/gax/retrying/AbstractRetryingExecutorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -115,7 +116,7 @@ private RetrySettings getDefaultRetrySettings() {

@Test
public void testSuccess() throws Exception {
FailingCallable callable = new FailingCallable(0, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(0, "request", "SUCCESS", tracer);
RetryingExecutorWithContext<String> executor =
getExecutor(getAlgorithm(getDefaultRetrySettings(), 0, null));
RetryingFuture<String> future = executor.createFuture(callable, retryingContext);
Expand All @@ -124,14 +125,14 @@ public void testSuccess() throws Exception {
assertFutureSuccess(future);
assertEquals(0, future.getAttemptSettings().getAttemptCount());

verify(tracer, times(1)).attemptStarted(0);
verify(tracer, times(1)).attemptStarted(eq("request"), eq(0));
verify(tracer, times(1)).attemptSucceeded();
verifyNoMoreInteractions(tracer);
}

@Test
public void testSuccessWithFailures() throws Exception {
FailingCallable callable = new FailingCallable(5, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(5, "request", "SUCCESS", tracer);
RetryingExecutorWithContext<String> executor =
getExecutor(getAlgorithm(getDefaultRetrySettings(), 0, null));
RetryingFuture<String> future = executor.createFuture(callable, retryingContext);
Expand All @@ -140,15 +141,15 @@ public void testSuccessWithFailures() throws Exception {
assertFutureSuccess(future);
assertEquals(5, future.getAttemptSettings().getAttemptCount());

verify(tracer, times(6)).attemptStarted(anyInt());
verify(tracer, times(6)).attemptStarted(eq("request"), anyInt());
verify(tracer, times(5)).attemptFailed(any(Throwable.class), any(Duration.class));
verify(tracer, times(1)).attemptSucceeded();
verifyNoMoreInteractions(tracer);
}

@Test
public void testSuccessWithFailuresPeekGetAttempt() throws Exception {
FailingCallable callable = new FailingCallable(5, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(5, "request", "SUCCESS", tracer);
RetryingExecutorWithContext<String> executor =
getExecutor(getAlgorithm(getDefaultRetrySettings(), 0, null));
RetryingFuture<String> future = executor.createFuture(callable, retryingContext);
Expand All @@ -174,7 +175,7 @@ public void testSuccessWithFailuresPeekGetAttempt() throws Exception {

@Test
public void testMaxRetriesExceeded() throws Exception {
FailingCallable callable = new FailingCallable(6, "FAILURE", tracer);
FailingCallable callable = new FailingCallable(6, "request", "FAILURE", tracer);
RetryingExecutorWithContext<String> executor =
getExecutor(getAlgorithm(getDefaultRetrySettings(), 0, null));
RetryingFuture<String> future = executor.createFuture(callable, retryingContext);
Expand All @@ -183,7 +184,7 @@ public void testMaxRetriesExceeded() throws Exception {
assertFutureFail(future, CustomException.class);
assertEquals(5, future.getAttemptSettings().getAttemptCount());

verify(tracer, times(6)).attemptStarted(anyInt());
verify(tracer, times(6)).attemptStarted(eq("request"), anyInt());
verify(tracer, times(5)).attemptFailed(any(Throwable.class), any(Duration.class));
verify(tracer, times(1)).attemptFailedRetriesExhausted(any(Throwable.class));
verifyNoMoreInteractions(tracer);
Expand All @@ -202,7 +203,7 @@ public void testTotalTimeoutExceeded() throws Exception {
getExecutor(
getAlgorithm(
useContextRetrySettings ? getDefaultRetrySettings() : retrySettings, 0, null));
FailingCallable callable = new FailingCallable(6, "FAILURE", tracer);
FailingCallable callable = new FailingCallable(6, "request", "FAILURE", tracer);
RetryingContext context;
if (useContextRetrySettings) {
context = FakeCallContext.createDefault().withTracer(tracer).withRetrySettings(retrySettings);
Expand All @@ -215,14 +216,14 @@ public void testTotalTimeoutExceeded() throws Exception {
assertFutureFail(future, CustomException.class);
assertTrue(future.getAttemptSettings().getAttemptCount() < 4);

verify(tracer, times(1)).attemptStarted(anyInt());
verify(tracer, times(1)).attemptStarted(eq("request"), anyInt());
verify(tracer, times(1)).attemptFailedRetriesExhausted(any(Throwable.class));
verifyNoMoreInteractions(tracer);
}

@Test
public void testCancelOuterFutureBeforeStart() throws Exception {
FailingCallable callable = new FailingCallable(4, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(4, "request", "SUCCESS", tracer);

RetrySettings retrySettings =
FAST_RETRY_SETTINGS
Expand All @@ -248,7 +249,7 @@ public void testCancelOuterFutureBeforeStart() throws Exception {

@Test
public void testCancelByRetryingAlgorithm() throws Exception {
FailingCallable callable = new FailingCallable(6, "FAILURE", tracer);
FailingCallable callable = new FailingCallable(6, "request", "FAILURE", tracer);
RetryingExecutorWithContext<String> executor =
getExecutor(getAlgorithm(getDefaultRetrySettings(), 5, new CancellationException()));
RetryingFuture<String> future = executor.createFuture(callable, retryingContext);
Expand All @@ -257,7 +258,7 @@ public void testCancelByRetryingAlgorithm() throws Exception {
assertFutureCancel(future);
assertEquals(4, future.getAttemptSettings().getAttemptCount());

verify(tracer, times(5)).attemptStarted(anyInt());
verify(tracer, times(5)).attemptStarted(eq("request"), anyInt());
// Pre-apocalypse failures
verify(tracer, times(4)).attemptFailed(any(Throwable.class), any(Duration.class));
// Apocalypse failure
Expand All @@ -267,7 +268,7 @@ public void testCancelByRetryingAlgorithm() throws Exception {

@Test
public void testUnexpectedExceptionFromRetryAlgorithm() throws Exception {
FailingCallable callable = new FailingCallable(6, "FAILURE", tracer);
FailingCallable callable = new FailingCallable(6, "request", "FAILURE", tracer);
RetryingExecutorWithContext<String> executor =
getExecutor(getAlgorithm(getDefaultRetrySettings(), 5, new RuntimeException()));
RetryingFuture<String> future = executor.createFuture(callable, retryingContext);
Expand All @@ -276,7 +277,7 @@ public void testUnexpectedExceptionFromRetryAlgorithm() throws Exception {
assertFutureFail(future, RuntimeException.class);
assertEquals(4, future.getAttemptSettings().getAttemptCount());

verify(tracer, times(5)).attemptStarted(anyInt());
verify(tracer, times(5)).attemptStarted(eq("request"), anyInt());
// Pre-apocalypse failures
verify(tracer, times(4)).attemptFailed(any(Throwable.class), any(Duration.class));
// Apocalypse failure
Expand All @@ -302,7 +303,7 @@ public void testPollExceptionByPollAlgorithm() throws Exception {
NanoClock.getDefaultClock()));

RetryingExecutorWithContext<String> executor = getExecutor(retryAlgorithm);
FailingCallable callable = new FailingCallable(6, "FAILURE", tracer);
FailingCallable callable = new FailingCallable(6, "request", "FAILURE", tracer);
RetryingContext context;
if (useContextRetrySettings) {
context = FakeCallContext.createDefault().withTracer(tracer).withRetrySettings(retrySettings);
Expand All @@ -315,7 +316,7 @@ public void testPollExceptionByPollAlgorithm() throws Exception {
assertFutureFail(future, PollException.class);
assertTrue(future.getAttemptSettings().getAttemptCount() < 4);

verify(tracer, times(1)).attemptStarted(anyInt());
verify(tracer, times(1)).attemptStarted(eq("request"), anyInt());
verify(tracer, times(1)).attemptPermanentFailure(any(PollException.class));
verifyNoMoreInteractions(tracer);
}
Expand Down
6 changes: 4 additions & 2 deletions gax/src/test/java/com/google/api/gax/retrying/FailingCallable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,12 @@ class FailingCallable implements Callable<String> {
private AtomicInteger attemptsCount = new AtomicInteger(0);
private final ApiTracer tracer;
private final int expectedFailuresCount;
private final String request;
private final String result;
private final CountDownLatch firstAttemptFinished = new CountDownLatch(1);

FailingCallable(int expectedFailuresCount, String result, ApiTracer tracer) {
FailingCallable(int expectedFailuresCount, String request, String result, ApiTracer tracer) {
this.request = request;
this.tracer = tracer;
this.expectedFailuresCount = expectedFailuresCount;
this.result = result;
Expand All @@ -80,7 +82,7 @@ public String call() throws Exception {
try {
int attemptNumber = attemptsCount.getAndIncrement();

tracer.attemptStarted(attemptNumber);
tracer.attemptStarted(request, attemptNumber);

if (attemptNumber < expectedFailuresCount) {
throw new CustomException();
Expand Down
12 changes: 6 additions & 6 deletions gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void testSuccessWithFailuresPeekAttempt() throws Exception {
final int maxRetries = 100;

ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
FailingCallable callable = new FailingCallable(15, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(15, "request", "SUCCESS", tracer);

RetrySettings retrySettings =
FAST_RETRY_SETTINGS
Expand Down Expand Up @@ -139,7 +139,7 @@ public void testSuccessWithFailuresGetAttempt() throws Exception {
final int maxRetries = 100;

ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
FailingCallable callable = new FailingCallable(15, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(15, "request", "SUCCESS", tracer);
RetrySettings retrySettings =
FAST_RETRY_SETTINGS
.toBuilder()
Expand Down Expand Up @@ -192,7 +192,7 @@ public void testCancelGetAttempt() throws Exception {
ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
final int maxRetries = 100;

FailingCallable callable = new FailingCallable(maxRetries - 1, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(maxRetries - 1, "request", "SUCCESS", tracer);
RetrySettings retrySettings =
FAST_RETRY_SETTINGS
.toBuilder()
Expand Down Expand Up @@ -249,7 +249,7 @@ public void testCancelGetAttempt() throws Exception {
public void testCancelOuterFutureAfterStart() throws Exception {
for (int executionsCount = 0; executionsCount < EXECUTIONS_COUNT; executionsCount++) {
ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
FailingCallable callable = new FailingCallable(4, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(4, "requset", "SUCCESS", tracer);
RetrySettings retrySettings =
FAST_RETRY_SETTINGS
.toBuilder()
Expand All @@ -276,7 +276,7 @@ public void testCancelOuterFutureAfterStart() throws Exception {
@Test
public void testCancelIsTraced() throws Exception {
ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
FailingCallable callable = new FailingCallable(4, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(4, "request", "SUCCESS", tracer);
RetrySettings retrySettings =
FAST_RETRY_SETTINGS
.toBuilder()
Expand Down Expand Up @@ -305,7 +305,7 @@ public void testCancelProxiedFutureAfterStart() throws Exception {
// this is a heavy test, which takes a lot of time, so only few executions.
for (int executionsCount = 0; executionsCount < 2; executionsCount++) {
ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor();
FailingCallable callable = new FailingCallable(5, "SUCCESS", tracer);
FailingCallable callable = new FailingCallable(5, "request", "SUCCESS", tracer);
RetrySettings retrySettings =
FAST_RETRY_SETTINGS
.toBuilder()
Expand Down
2 changes: 1 addition & 1 deletion gax/src/test/java/com/google/api/gax/tracing/TracedCallableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void testNonRetriedCallable() throws Exception {
ApiFuture<String> future = callable.futureCall("Is your refrigerator running?", callContext);

verify(tracerFactory, times(1)).newTracer(parentTracer, SPAN_NAME, OperationType.Unary);
verify(tracer, times(1)).attemptStarted(anyInt());
verify(tracer, times(1)).attemptStarted(anyString(), anyInt());
verify(tracer, times(1)).attemptSucceeded();
verify(tracer, times(1)).operationSucceeded();
verifyNoMoreInteractions(tracer);
Expand Down

0 comments on commit 27bf265

Please sign in to comment.