impl(otel): traced async backoff in retry loop (#11051)

googleapis · Mar 16, 2023 · fcd6cab · fcd6cab
1 parent 4b5e31d
commit fcd6cab
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 16 deletions.
diff --git a/google/cloud/internal/async_rest_retry_loop.h b/google/cloud/internal/async_rest_retry_loop.h
@@ -20,6 +20,7 @@
 #include "google/cloud/future.h"
 #include "google/cloud/idempotency.h"
 #include "google/cloud/internal/call_context.h"
+#include "google/cloud/internal/grpc_opentelemetry.h"
 #include "google/cloud/internal/invoke_result.h"
 #include "google/cloud/internal/rest_context.h"
 #include "google/cloud/internal/retry_loop_helpers.h"
@@ -245,11 +246,11 @@ class AsyncRestRetryLoopImpl
     auto self = this->shared_from_this();
     auto state = StartOperation();
     if (state.cancelled) return;
-    SetPending(state.operation,
-               cq_.MakeRelativeTimer(backoff_policy_->OnCompletion())
-                   .then([self](future<TimerArgType> f) {
-                     self->OnBackoff(f.get());
-                   }));
+    SetPending(state.operation, internal::TracedAsyncBackoff(
+                                    cq_, backoff_policy_->OnCompletion())
+                                    .then([self](future<TimerArgType> f) {
+                                      self->OnBackoff(f.get());
+                                    }));
   }
 
   void OnAttempt(T result) {

diff --git a/google/cloud/internal/async_rest_retry_loop_test.cc b/google/cloud/internal/async_rest_retry_loop_test.cc
@@ -551,6 +551,33 @@ TEST_F(AsyncRestRetryLoopCancelTest, ShutdownDuringTimer) {
 
 #ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY
 
+using ::google::cloud::testing_util::IsActive;
+using ::google::cloud::testing_util::SpanNamed;
+using ::testing::AllOf;
+using ::testing::Each;
+using ::testing::SizeIs;
+
+TEST(AsyncRestRetryLoopTest, TracedBackoff) {
+  auto span_catcher = testing_util::InstallSpanCatcher();
+
+  internal::OptionsSpan o(
+      Options{}.set<internal::OpenTelemetryTracingOption>(true));
+  AutomaticallyCreatedRestBackgroundThreads background;
+  (void)AsyncRestRetryLoop(
+      TestRetryPolicy(), TestBackoffPolicy(), Idempotency::kIdempotent,
+      background.cq(),
+      [&](auto, auto, auto) {
+        return make_ready_future<StatusOr<int>>(
+            internal::UnavailableError("try again"));
+      },
+      42, "error message")
+      .get();
+
+  auto spans = span_catcher->GetSpans();
+  EXPECT_THAT(spans,
+              AllOf(SizeIs(kMaxRetries), Each(SpanNamed("Async Backoff"))));
+}
+
 TEST(AsyncRestRetryLoopTest, CallSpanActiveThroughout) {
   auto span_catcher = testing_util::InstallSpanCatcher();
 
@@ -565,7 +592,6 @@ TEST(AsyncRestRetryLoopTest, CallSpanActiveThroughout) {
       TestRetryPolicy(), TestBackoffPolicy(), Idempotency::kIdempotent,
       background.cq(),
       [&](auto, auto, auto) {
-        using testing_util::IsActive;
         EXPECT_THAT(span, IsActive());
         return sequencer.PushBack();
       },
@@ -585,10 +611,7 @@ TEST(AsyncRestRetryLoopTest, CallSpanActiveDuringCancel) {
   internal::OptionsSpan o(
       Options{}.set<internal::OpenTelemetryTracingOption>(true));
 
-  promise<StatusOr<int>> p([&] {
-    using testing_util::IsActive;
-    EXPECT_THAT(span, IsActive());
-  });
+  promise<StatusOr<int>> p([&] { EXPECT_THAT(span, IsActive()); });
 
   AutomaticallyCreatedRestBackgroundThreads background;
   future<StatusOr<int>> actual = AsyncRestRetryLoop(

diff --git a/google/cloud/internal/async_retry_loop.h b/google/cloud/internal/async_retry_loop.h
@@ -20,6 +20,7 @@
 #include "google/cloud/future.h"
 #include "google/cloud/grpc_options.h"
 #include "google/cloud/internal/call_context.h"
+#include "google/cloud/internal/grpc_opentelemetry.h"
 #include "google/cloud/internal/invoke_result.h"
 #include "google/cloud/internal/retry_loop_helpers.h"
 #include "google/cloud/internal/retry_policy.h"
@@ -244,7 +245,7 @@ class AsyncRetryLoopImpl
     auto state = StartOperation();
     if (state.cancelled) return;
     SetPending(state.operation,
-               cq_.MakeRelativeTimer(backoff_policy_->OnCompletion())
+               TracedAsyncBackoff(cq_, backoff_policy_->OnCompletion())
                    .then([self](future<TimerArgType> f) {
                      self->OnBackoff(f.get());
                    }));

diff --git a/google/cloud/internal/async_retry_loop_test.cc b/google/cloud/internal/async_retry_loop_test.cc
@@ -562,6 +562,31 @@ TEST_F(AsyncRetryLoopCancelTest, ShutdownDuringTimer) {
 
 #ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY
 
+using ::google::cloud::testing_util::IsActive;
+using ::google::cloud::testing_util::SpanNamed;
+using ::testing::AllOf;
+using ::testing::Each;
+using ::testing::SizeIs;
+
+TEST(AsyncRetryLoopTest, TracedBackoff) {
+  auto span_catcher = testing_util::InstallSpanCatcher();
+
+  OptionsSpan o(Options{}.set<OpenTelemetryTracingOption>(true));
+  AutomaticallyCreatedBackgroundThreads background;
+  (void)AsyncRetryLoop(
+      TestRetryPolicy(), TestBackoffPolicy(), Idempotency::kIdempotent,
+      background.cq(),
+      [&](auto, auto, auto) {
+        return make_ready_future<StatusOr<int>>(UnavailableError("try again"));
+      },
+      42, "error message")
+      .get();
+
+  auto spans = span_catcher->GetSpans();
+  EXPECT_THAT(spans,
+              AllOf(SizeIs(kMaxRetries), Each(SpanNamed("Async Backoff"))));
+}
+
 TEST(AsyncRetryLoopTest, CallSpanActiveThroughout) {
   auto span_catcher = testing_util::InstallSpanCatcher();
 
@@ -575,7 +600,6 @@ TEST(AsyncRetryLoopTest, CallSpanActiveThroughout) {
       TestRetryPolicy(), TestBackoffPolicy(), Idempotency::kIdempotent,
       background.cq(),
       [&](auto, auto, auto) {
-        using testing_util::IsActive;
         EXPECT_THAT(span, IsActive());
         return sequencer.PushBack();
       },
@@ -594,10 +618,7 @@ TEST(AsyncRetryLoopTest, CallSpanActiveDuringCancel) {
   auto scope = opentelemetry::trace::Scope(span);
   OptionsSpan o(Options{}.set<OpenTelemetryTracingOption>(true));
 
-  promise<StatusOr<int>> p([span] {
-    using testing_util::IsActive;
-    EXPECT_THAT(span, IsActive());
-  });
+  promise<StatusOr<int>> p([span] { EXPECT_THAT(span, IsActive()); });
 
   AutomaticallyCreatedBackgroundThreads background;
   future<StatusOr<int>> actual = AsyncRetryLoop(