From 55fef547e6063129f9cf64234081e0e2172fe2b6 Mon Sep 17 00:00:00 2001 From: bajajnehaa Date: Wed, 29 Jul 2026 07:01:51 +0000 Subject: [PATCH 1/2] feat(storage): add resource span attributes for ACO ( App Centric Observability ) for rewrite API in async client --- .../internal/async/connection_tracing.cc | 5 +- .../internal/async/connection_tracing_test.cc | 53 +++++++++++++++++++ .../async/rewriter_connection_tracing.cc | 7 +++ .../async/rewriter_connection_tracing.h | 6 +++ 4 files changed, 70 insertions(+), 1 deletion(-) 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..111f92a1dfd97 100644 --- a/google/cloud/storage/internal/async/connection_tracing_test.cc +++ b/google/cloud/storage/internal/async/connection_tracing_test.cc @@ -539,6 +539,59 @@ TEST(ConnectionTracing, RewriteObject) { SpanHasEvents(EventNamed("gl-cpp.storage.rewrite.iterate"))))); } +TEST(ConnectionTracing, RewriteObjectSpanEnrichment) { + auto span_catcher = InstallSpanCatcher(); + PromiseWithOTelContext> p; + + auto options = TracingEnabled().set< + google::cloud::storage_experimental::OTelSpanEnrichmentOption>(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..a53164adc0dbd 100644 --- a/google/cloud/storage/internal/async/rewriter_connection_tracing.cc +++ b/google/cloud/storage/internal/async/rewriter_connection_tracing.cc @@ -68,6 +68,13 @@ MakeTracingAsyncRewriterConnection( std::shared_ptr impl, bool enabled) { if (!enabled) return impl; auto span = internal::MakeSpan("storage::AsyncConnection::RewriteObject"); + return MakeTracingAsyncRewriterConnection(std::move(impl), std::move(span)); +} + +std::shared_ptr +MakeTracingAsyncRewriterConnection( + std::shared_ptr impl, + opentelemetry::nostd::shared_ptr span) { 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..f3c3be3a52724 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 @@ -29,6 +30,11 @@ std::shared_ptr MakeTracingAsyncRewriterConnection( std::shared_ptr impl, bool enabled); +std::shared_ptr +MakeTracingAsyncRewriterConnection( + std::shared_ptr impl, + opentelemetry::nostd::shared_ptr span); + GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage_internal } // namespace cloud From ae615ff55e4d2f65b4fbf9d8735df14e191b617f Mon Sep 17 00:00:00 2001 From: bajajnehaa Date: Wed, 29 Jul 2026 10:45:32 +0000 Subject: [PATCH 2/2] refactor code --- .../storage/internal/async/connection_tracing_test.cc | 6 ++++-- .../internal/async/rewriter_connection_tracing.cc | 9 +-------- .../storage/internal/async/rewriter_connection_tracing.h | 4 ---- .../internal/async/rewriter_connection_tracing_test.cc | 8 +++++--- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/google/cloud/storage/internal/async/connection_tracing_test.cc b/google/cloud/storage/internal/async/connection_tracing_test.cc index 111f92a1dfd97..e6d3c1fec4440 100644 --- a/google/cloud/storage/internal/async/connection_tracing_test.cc +++ b/google/cloud/storage/internal/async/connection_tracing_test.cc @@ -543,8 +543,10 @@ TEST(ConnectionTracing, RewriteObjectSpanEnrichment) { auto span_catcher = InstallSpanCatcher(); PromiseWithOTelContext> p; - auto options = TracingEnabled().set< - google::cloud::storage_experimental::OTelSpanEnrichmentOption>(true); + auto options = + TracingEnabled() + .set( + true); auto mock = std::make_unique(); EXPECT_CALL(*mock, options).WillRepeatedly(Return(options)); diff --git a/google/cloud/storage/internal/async/rewriter_connection_tracing.cc b/google/cloud/storage/internal/async/rewriter_connection_tracing.cc index a53164adc0dbd..15c42d171ed6d 100644 --- a/google/cloud/storage/internal/async/rewriter_connection_tracing.cc +++ b/google/cloud/storage/internal/async/rewriter_connection_tracing.cc @@ -63,18 +63,11 @@ class AsyncRewriterTracingConnection : public storage::AsyncRewriterConnection { } // namespace -std::shared_ptr -MakeTracingAsyncRewriterConnection( - std::shared_ptr impl, bool enabled) { - if (!enabled) return impl; - auto span = internal::MakeSpan("storage::AsyncConnection::RewriteObject"); - return MakeTracingAsyncRewriterConnection(std::move(impl), std::move(span)); -} - std::shared_ptr MakeTracingAsyncRewriterConnection( 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 f3c3be3a52724..11d76b9cbeb99 100644 --- a/google/cloud/storage/internal/async/rewriter_connection_tracing.h +++ b/google/cloud/storage/internal/async/rewriter_connection_tracing.h @@ -26,10 +26,6 @@ namespace cloud { namespace storage_internal { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN -std::shared_ptr -MakeTracingAsyncRewriterConnection( - std::shared_ptr impl, bool enabled); - std::shared_ptr MakeTracingAsyncRewriterConnection( std::shared_ptr impl, 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); }