fix(storage): add App Hub storage.googleapis.com prefix to destination.id

google/cloud/storage/internal/async/connection_tracing.cc

@@ -267,7 +267,6 @@ class AsyncConnectionTracing : public storage::AsyncConnection {
 
  private:
   static constexpr char kProjectBucketPrefix[] = "projects/_/buckets/";
-  static constexpr char kGlobalLocation[] = "global";
 
   BucketMetadataCache& cache() const { return *cache_; }
 
@@ -291,17 +290,14 @@ class AsyncConnectionTracing : public storage::AsyncConnection {
           StatusOr<google::storage::v2::Bucket> metadata = f.get();
           if (metadata.ok()) {
             BucketCacheEntry entry = BucketCacheEntry::FromLocation(
-                metadata->project() + "/buckets/" +
-                    BucketMetadataCache::NormalizeBucketName(bucket_name),
+                BucketCacheEntry::ResourceName(metadata->project(),
+                                               bucket_name),
                 metadata->location(), metadata->location_type());
             cache->Put(bucket_name, std::move(entry));
           } else if (metadata.status().code() ==
                      StatusCode::kPermissionDenied) {
-            BucketCacheEntry entry{
-                std::string(kProjectBucketPrefix) +
-                    BucketMetadataCache::NormalizeBucketName(bucket_name),
-                kGlobalLocation};
-            cache->Put(bucket_name, std::move(entry));
+            cache->Put(bucket_name,
+                       BucketCacheEntry::FromUnknownProject(bucket_name));
           }
         });
   }
@@ -321,8 +317,7 @@ class AsyncConnectionTracing : public storage::AsyncConnection {
         google::cloud::storage_experimental::OTelSpanEnrichmentOption>();
     if (!enabled) return;
     auto entry = BucketCacheEntry::FromLocation(
-        bucket.project() + "/buckets/" +
-            BucketMetadataCache::NormalizeBucketName(bucket_name),
+        BucketCacheEntry::ResourceName(bucket.project(), bucket_name),
         bucket.location(), bucket.location_type());
     EnrichSpan(span, entry);
     cache.Put(bucket_name, std::move(entry));

google/cloud/storage/internal/async/connection_tracing_test.cc

@@ -590,7 +590,8 @@ TEST(ConnectionTracing, RewriteObjectSpanEnrichment) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -890,7 +891,8 @@ TEST(ConnectionTracing, GetBucketSpanEnrichment) {
           SpanHasInstrumentationScope(), SpanKindIsClient(),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -953,7 +955,8 @@ TEST(ConnectionTracing, BucketMetadataCacheSuccess) {
           SpanHasInstrumentationScope(), SpanKindIsClient(),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -1054,7 +1057,8 @@ TEST(ConnectionTracing, DeleteObjectNoEvictOnError) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }

google/cloud/storage/internal/bucket_metadata_cache.cc

@@ -24,6 +24,13 @@ namespace cloud {
 namespace storage_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
+std::string BucketCacheEntry::ResourceName(std::string const& project,
+                                           std::string const& bucket) {
+  return std::string(kStorageResourceNamePrefix) +
+         (project.empty() ? std::string("projects/_") : project) + "/buckets/" +
+         BucketMetadataCache::NormalizeBucketName(bucket);
+}
+
 BucketCacheEntry BucketCacheEntry::FromLocation(
     std::string id, std::string location, std::string const& location_type) {
   if (location_type == "multi-region" || location_type == "dual-region") {
@@ -34,9 +41,16 @@ BucketCacheEntry BucketCacheEntry::FromLocation(
 
 BucketCacheEntry BucketCacheEntry::FromMetadata(
     storage::BucketMetadata const& m) {
-  return FromLocation(
-      "projects/" + std::to_string(m.project_number()) + "/buckets/" + m.name(),
-      m.location(), m.location_type());
+  auto project = m.project_number() == 0
+                     ? std::string{}
+                     : "projects/" + std::to_string(m.project_number());
+  return FromLocation(ResourceName(project, m.name()), m.location(),
+                      m.location_type());
+}
+
+BucketCacheEntry BucketCacheEntry::FromUnknownProject(
+    std::string const& bucket) {
+  return {ResourceName(std::string{}, bucket), "global"};
 }
 
 std::string BucketMetadataCache::NormalizeBucketName(

google/cloud/storage/internal/bucket_metadata_cache.h

@@ -38,13 +38,33 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 namespace storage_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
+// The App Hub / Cloud Asset Inventory full resource name prefix for Cloud
+// Storage resources.
+inline constexpr char kStorageResourceNamePrefix[] =
+    "//storage.googleapis.com/";
+
 struct BucketCacheEntry {
   std::string id;
   std::string location;
 
+  /**
+   * Returns the full resource name for a bucket, i.e.
+   * `//storage.googleapis.com/{project}/buckets/{bucket}`.
+   *
+   * @param project the project in `projects/{project-id-or-number}` format. If
+   *     empty, `projects/_` is used.
+   * @param bucket the bucket name, optionally in `projects/_/buckets/{bucket}`
+   *     format.
+   */
+  static std::string ResourceName(std::string const& project,
+                                  std::string const& bucket);
+
   static BucketCacheEntry FromLocation(std::string id, std::string location,
                                        std::string const& location_type);
   static BucketCacheEntry FromMetadata(storage::BucketMetadata const& m);
+  // Creates an entry for a bucket whose project and location are unknown,
+  // e.g. when fetching its metadata fails with `kPermissionDenied`.
+  static BucketCacheEntry FromUnknownProject(std::string const& bucket);
 };
 
 class BucketMetadataCache {

google/cloud/storage/internal/bucket_metadata_cache_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/storage/internal/bucket_metadata_cache.h"
+#include "google/cloud/storage/bucket_metadata.h"
 #include <gmock/gmock.h>
 
 namespace google {
@@ -39,36 +40,71 @@ TEST(BucketMetadataCacheTest, NormalizeBucketName) {
               Eq("test-bucket"));
 }
 
+TEST(BucketCacheEntryTest, ResourceName) {
+  EXPECT_THAT(BucketCacheEntry::ResourceName("projects/123", "test-bucket"),
+              Eq("//storage.googleapis.com/projects/123/buckets/test-bucket"));
+  EXPECT_THAT(BucketCacheEntry::ResourceName("projects/123",
+                                             "projects/_/buckets/test-bucket"),
+              Eq("//storage.googleapis.com/projects/123/buckets/test-bucket"));
+  EXPECT_THAT(BucketCacheEntry::ResourceName("", "test-bucket"),
+              Eq("//storage.googleapis.com/projects/_/buckets/test-bucket"));
+}
+
+TEST(BucketCacheEntryTest, FromMetadata) {
+  auto entry = BucketCacheEntry::FromMetadata(storage::BucketMetadata()
+                                                  .set_name("test-bucket")
+                                                  .set_location("US-EAST1"));
+  // project_number() defaults to 0, which means "unknown".
+  EXPECT_THAT(entry.id,
+              Eq("//storage.googleapis.com/projects/_/buckets/test-bucket"));
+}
+
+TEST(BucketCacheEntryTest, FromUnknownProject) {
+  auto entry = BucketCacheEntry::FromUnknownProject("test-bucket");
+  EXPECT_THAT(entry.id,
+              Eq("//storage.googleapis.com/projects/_/buckets/test-bucket"));
+  EXPECT_THAT(entry.location, Eq("global"));
+}
+
 TEST(BucketMetadataCacheTest, HitAndMiss) {
   BucketMetadataCache cache(10);
   EXPECT_FALSE(cache.Get("test-bucket").has_value());
 
-  BucketCacheEntry entry{"projects/123/buckets/test-bucket", "us-central1"};
+  BucketCacheEntry entry{
+      "//storage.googleapis.com/projects/123/buckets/test-bucket",
+      "us-central1"};
   cache.Put("test-bucket", entry);
 
   auto res = cache.Get("test-bucket");
   ASSERT_TRUE(res.has_value());
-  EXPECT_THAT(res->id, Eq("projects/123/buckets/test-bucket"));
+  EXPECT_THAT(res->id,
+              Eq("//storage.googleapis.com/projects/123/buckets/test-bucket"));
   EXPECT_THAT(res->location, Eq("us-central1"));
 }
 
 TEST(BucketMetadataCacheTest, PutUpdatesExisting) {
   BucketMetadataCache cache(10);
-  BucketCacheEntry entry1{"projects/123/buckets/test-bucket", "us-central1"};
+  BucketCacheEntry entry1{
+      "//storage.googleapis.com/projects/123/buckets/test-bucket",
+      "us-central1"};
   cache.Put("test-bucket", entry1);
 
-  BucketCacheEntry entry2{"projects/456/buckets/test-bucket", "global"};
+  BucketCacheEntry entry2{
+      "//storage.googleapis.com/projects/456/buckets/test-bucket", "global"};
   cache.Put("test-bucket", entry2);
 
   auto res = cache.Get("test-bucket");
   ASSERT_TRUE(res.has_value());
-  EXPECT_THAT(res->id, Eq("projects/456/buckets/test-bucket"));
+  EXPECT_THAT(res->id,
+              Eq("//storage.googleapis.com/projects/456/buckets/test-bucket"));
   EXPECT_THAT(res->location, Eq("global"));
 }
 
 TEST(BucketMetadataCacheTest, InvalidateAndClear) {
   BucketMetadataCache cache(10);
-  BucketCacheEntry entry{"projects/123/buckets/test-bucket", "us-central1"};
+  BucketCacheEntry entry{
+      "//storage.googleapis.com/projects/123/buckets/test-bucket",
+      "us-central1"};
   cache.Put("test-bucket", entry);
   EXPECT_TRUE(cache.Get("test-bucket").has_value());
 

google/cloud/storage/internal/tracing_connection.cc

@@ -69,18 +69,19 @@ void TracingConnection::MaybeTriggerBackgroundFetch(
 
   auto guard = ScopedFetch(cache_, bucket_name);
   auto current_options = google::cloud::internal::SaveCurrentOptions();
-  runner()([impl = impl_, cache = cache_, bucket_name, current_options,
-            guard]() {
-    google::cloud::internal::OptionsSpan span(current_options);
-    storage::internal::GetBucketMetadataRequest request(bucket_name);
-    auto result = impl->GetBucketMetadata(request);
-
-    if (result.ok()) {
-      cache->Put(bucket_name, BucketCacheEntry::FromMetadata(*result));
-    } else if (result.status().code() == StatusCode::kPermissionDenied) {
-      cache->Put(bucket_name, {"projects/_/buckets/" + bucket_name, "global"});
-    }
-  });
+  runner()(
+      [impl = impl_, cache = cache_, bucket_name, current_options, guard]() {
+        google::cloud::internal::OptionsSpan span(current_options);
+        storage::internal::GetBucketMetadataRequest request(bucket_name);
+        auto result = impl->GetBucketMetadata(request);
+
+        if (result.ok()) {
+          cache->Put(bucket_name, BucketCacheEntry::FromMetadata(*result));
+        } else if (result.status().code() == StatusCode::kPermissionDenied) {
+          cache->Put(bucket_name,
+                     BucketCacheEntry::FromUnknownProject(bucket_name));
+        }
+      });
 }
 
 void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,

google/cloud/storage/internal/tracing_connection_test.cc

@@ -158,7 +158,8 @@ TEST(TracingClientTest, CreateBucketSuccess) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -210,7 +211,8 @@ TEST(TracingClientTest, GetBucketMetadataSuccess) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -263,7 +265,8 @@ TEST(TracingClientTest, BucketMetadataCacheSuccess) {
           SpanNamed("storage::Client::DeleteObject"),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -338,7 +341,8 @@ TEST(TracingClientTest, UpdateBucketSuccess) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -388,7 +392,8 @@ TEST(TracingClientTest, PatchBucketSuccess) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }
@@ -511,7 +516,8 @@ TEST(TracingClientTest, LockBucketRetentionPolicySuccess) {
           SpanWithStatus(opentelemetry::trace::StatusCode::kOk),
           SpanHasAttributes(
               OTelAttribute<std::string>("gcp.resource.destination.id",
-                                         "projects/123456/buckets/test-bucket"),
+                                         "//storage.googleapis.com/projects/"
+                                         "123456/buckets/test-bucket"),
               OTelAttribute<std::string>("gcp.resource.destination.location",
                                          "us-east1")))));
 }