diff --git a/google/cloud/storage/internal/async/connection_tracing.cc b/google/cloud/storage/internal/async/connection_tracing.cc index d4a2f40b5cc00..e21ce48e4a032 100644 --- a/google/cloud/storage/internal/async/connection_tracing.cc +++ b/google/cloud/storage/internal/async/connection_tracing.cc @@ -227,8 +227,11 @@ class AsyncConnectionTracing : public storage::AsyncConnection { std::shared_ptr RewriteObject( RewriteObjectParams p) override { auto const enabled = internal::TracingEnabled(p.options); + if (!enabled) return impl_->RewriteObject(std::move(p)); + auto span = internal::MakeSpan("storage::AsyncConnection::RewriteObject"); + EnrichSpan(*span, p.options, p.request.destination_bucket()); return MakeTracingAsyncRewriterConnection( - impl_->RewriteObject(std::move(p)), enabled); + impl_->RewriteObject(std::move(p)), std::move(span)); } future> GetBucket( diff --git a/google/cloud/storage/internal/async/connection_tracing_test.cc b/google/cloud/storage/internal/async/connection_tracing_test.cc index f793243f42c05..e6d3c1fec4440 100644 --- a/google/cloud/storage/internal/async/connection_tracing_test.cc +++ b/google/cloud/storage/internal/async/connection_tracing_test.cc @@ -539,6 +539,61 @@ TEST(ConnectionTracing, RewriteObject) { SpanHasEvents(EventNamed("gl-cpp.storage.rewrite.iterate"))))); } +TEST(ConnectionTracing, RewriteObjectSpanEnrichment) { + auto span_catcher = InstallSpanCatcher(); + PromiseWithOTelContext> p; + + auto options = + TracingEnabled() + .set( + true); + auto mock = std::make_unique(); + EXPECT_CALL(*mock, options).WillRepeatedly(Return(options)); + + EXPECT_CALL(*mock, GetBucket).WillOnce(expect_context(p)); + EXPECT_CALL(*mock, RewriteObject).WillOnce([] { + auto rewriter = std::make_shared(); + EXPECT_CALL(*rewriter, Iterate).WillOnce([] { + return make_ready_future(make_status_or(MakeRewriteResponse())); + }); + return rewriter; + }); + + auto connection = MakeTracingAsyncConnection(std::move(mock)); + + // 1st call populates the cache + google::storage::v2::GetBucketRequest req; + req.set_name("projects/_/buckets/test-bucket"); + auto res1 = connection->GetBucket({req, options}).then(expect_no_context); + google::storage::v2::Bucket bucket_meta; + bucket_meta.set_project("projects/123456"); + bucket_meta.set_location("us-east1"); + bucket_meta.set_location_type("regional"); + p.set_value(make_status_or(std::move(bucket_meta))); + ASSERT_STATUS_OK(res1.get()); + + (void)span_catcher->GetSpans(); + + // 2nd call: RewriteObject uses cached bucket metadata for span enrichment + google::storage::v2::RewriteObjectRequest rewrite_req; + rewrite_req.set_destination_bucket("projects/_/buckets/test-bucket"); + auto rewriter = connection->RewriteObject({rewrite_req, options}); + auto r1 = rewriter->Iterate().get(); + ASSERT_STATUS_OK(r1); + + auto spans = span_catcher->GetSpans(); + EXPECT_THAT( + spans, + ElementsAre(AllOf( + SpanNamed("storage::AsyncConnection::RewriteObject"), + SpanWithStatus(opentelemetry::trace::StatusCode::kOk), + SpanHasAttributes( + OTelAttribute("gcp.resource.destination.id", + "projects/123456/buckets/test-bucket"), + OTelAttribute("gcp.resource.destination.location", + "us-east1"))))); +} + TEST(ConnectionTracing, OpenError) { auto span_catcher = InstallSpanCatcher(); PromiseWithOTelContext< diff --git a/google/cloud/storage/internal/async/rewriter_connection_tracing.cc b/google/cloud/storage/internal/async/rewriter_connection_tracing.cc index be91d0038961d..15c42d171ed6d 100644 --- a/google/cloud/storage/internal/async/rewriter_connection_tracing.cc +++ b/google/cloud/storage/internal/async/rewriter_connection_tracing.cc @@ -65,9 +65,9 @@ class AsyncRewriterTracingConnection : public storage::AsyncRewriterConnection { std::shared_ptr MakeTracingAsyncRewriterConnection( - std::shared_ptr impl, bool enabled) { - if (!enabled) return impl; - auto span = internal::MakeSpan("storage::AsyncConnection::RewriteObject"); + std::shared_ptr impl, + opentelemetry::nostd::shared_ptr span) { + if (!span) return impl; return std::make_shared(std::move(impl), std::move(span)); } diff --git a/google/cloud/storage/internal/async/rewriter_connection_tracing.h b/google/cloud/storage/internal/async/rewriter_connection_tracing.h index a81a76601e843..11d76b9cbeb99 100644 --- a/google/cloud/storage/internal/async/rewriter_connection_tracing.h +++ b/google/cloud/storage/internal/async/rewriter_connection_tracing.h @@ -16,6 +16,7 @@ #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_REWRITER_CONNECTION_TRACING_H #include "google/cloud/storage/async/rewriter_connection.h" +#include "google/cloud/internal/opentelemetry.h" #include "google/cloud/options.h" #include "google/cloud/version.h" #include @@ -27,7 +28,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN std::shared_ptr MakeTracingAsyncRewriterConnection( - std::shared_ptr impl, bool enabled); + std::shared_ptr impl, + opentelemetry::nostd::shared_ptr span); GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage_internal diff --git a/google/cloud/storage/internal/async/rewriter_connection_tracing_test.cc b/google/cloud/storage/internal/async/rewriter_connection_tracing_test.cc index 417eabf621e71..3d9c71fdadd8a 100644 --- a/google/cloud/storage/internal/async/rewriter_connection_tracing_test.cc +++ b/google/cloud/storage/internal/async/rewriter_connection_tracing_test.cc @@ -93,8 +93,9 @@ TEST(RewriterTracingConnection, Basic) { }); }); + auto span = internal::MakeSpan("storage::AsyncConnection::RewriteObject"); auto actual = - MakeTracingAsyncRewriterConnection(std::move(mock), /*enabled=*/true); + MakeTracingAsyncRewriterConnection(std::move(mock), std::move(span)); auto r1 = actual->Iterate(); sequencer.PopFront().set_value(); EXPECT_THAT(r1.get(), StatusIs(PermanentError().status().code())); @@ -145,8 +146,9 @@ TEST(RewriterTracingConnection, Disabled) { auto mock = std::make_unique(); auto* const expected = mock.get(); - auto actual = - MakeTracingAsyncRewriterConnection(std::move(mock), /*enabled=*/false); + auto actual = MakeTracingAsyncRewriterConnection( + std::move(mock), + opentelemetry::nostd::shared_ptr{}); EXPECT_EQ(actual.get(), expected); }