diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_options.cc b/google/cloud/storage/internal/grpc/metrics_exporter_options.cc index f0fed9bfce07..8bebcebe6a05 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_options.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_options.cc @@ -41,25 +41,8 @@ auto ByName(opentelemetry::sdk::resource::ResourceAttributes const& attributes, Options MetricsExporterOptions( Project const& project, - opentelemetry::sdk::resource::Resource const& resource, - Options const& options) { + opentelemetry::sdk::resource::Resource const& resource) { namespace sc = ::opentelemetry::sdk::resource::SemanticConventions; - auto result = - Options{} - .set(true) - .set("storage.googleapis.com/"); - - // We need a UUID because there may be multiple monitored resources within the - // same process, and we need these monitored resources to be globally unique - // or GCM may complain about the publication rate. There is no information - // available that can make this unique enough, all the clients in a service - // may be using the same project and bucket (not that buckets are available). - // - // This is fairly expensive, it requires initializing a new PRNG, and fetching - // entropy from the OS. Outside tests, this function will be called a handful - // of times, so the performance is not that important. - auto uuid = - google::cloud::internal::InvocationIdGenerator().MakeInvocationId(); auto const& attributes = resource.GetAttributes(); auto monitored_resource = google::api::MonitoredResource{}; @@ -71,12 +54,29 @@ Options MetricsExporterOptions( labels["cloud_platform"] = ByName(attributes, sc::kCloudPlatform, "unknown"); labels["host_id"] = ByName(attributes, "faas.id", ByName(attributes, sc::kHostId, "unknown")); - labels["instance_id"] = std::move(uuid); + + // We need a UUID because there may be multiple monitored resources within the + // same process, and we need these monitored resources to be globally unique + // or GCM may complain about the publication rate. There is no information + // available that can make this unique enough, all the clients in a service + // may be using the same project and bucket (not that buckets are available). + // + // This is fairly expensive, it requires initializing a new PRNG, and fetching + // entropy from the OS. Outside tests, this function will be called a handful + // of times, so the performance is not that important. + labels["instance_id"] = + google::cloud::internal::InvocationIdGenerator().MakeInvocationId(); labels["api"] = "GRPC"; - result.set( - std::move(monitored_resource)); + return Options{} + .set(true) + .set("storage.googleapis.com/") + .set( + std::move(monitored_resource)); +} +Options MetricsExporterConnectionOptions(Options const& options) { + auto result = Options{}; auto ep_canonical = std::string{"storage.googleapis.com"}; auto ep_private = std::string{"private.googleapis.com"}; auto ep_restricted = std::string{"restricted.googleapis.com"}; @@ -95,7 +95,6 @@ Options MetricsExporterOptions( if (matches(ep_canonical)) return result; if (matches(ep_private)) result.set(ep_private); if (matches(ep_restricted)) result.set(ep_restricted); - return result; } diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_options.h b/google/cloud/storage/internal/grpc/metrics_exporter_options.h index 46a0fe45e133..739da8ea04ce 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_options.h +++ b/google/cloud/storage/internal/grpc/metrics_exporter_options.h @@ -27,14 +27,14 @@ namespace cloud { namespace storage_internal { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN -/** - * Returns the monitoring options given the (fully populated) options for - * Storage. - */ +/// Returns the monitoring exporter options given the project and resource. Options MetricsExporterOptions( Project const& project, - opentelemetry::sdk::resource::Resource const& resource, - Options const& options); + opentelemetry::sdk::resource::Resource const& resource); + +/// Returns the monitoring exporter connection options given the fully populated +/// storage options. +Options MetricsExporterConnectionOptions(Options const& options); #endif // GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_options_test.cc b/google/cloud/storage/internal/grpc/metrics_exporter_options_test.cc index 2ec6c66dc656..212e2fe3dd1e 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_options_test.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_options_test.cc @@ -40,11 +40,7 @@ using ::google::protobuf::TextFormat; using ::testing::Contains; using ::testing::Pair; -auto TestResource() { - return opentelemetry::sdk::resource::Resource::Create({}); -} - -TEST(MetricsExporterOptions, DefaultEndpoint) { +TEST(MetricsExporterConnectionOptions, DefaultEndpoint) { // In all these cases the default monitoring endpoint is either the best // choice, or the least bad choice. struct TestCase { @@ -83,74 +79,49 @@ TEST(MetricsExporterOptions, DefaultEndpoint) { expected_ud = t.universe_domain; } options = DefaultOptionsGrpc(std::move(options)); - auto const actual = - MetricsExporterOptions(Project("test-only"), TestResource(), options); + auto const actual = MetricsExporterConnectionOptions(options); EXPECT_FALSE(actual.has()); EXPECT_EQ(actual.get(), expected_ud); - EXPECT_TRUE(actual.has()); - EXPECT_TRUE(actual.get()); - EXPECT_EQ(actual.get(), - "storage.googleapis.com/"); } } -TEST(MetricsExporterOptions, PrivateDefaultUD) { +TEST(MetricsExporterConnectionOptions, PrivateDefaultUD) { for (std::string prefix : {"", "google-c2p:///"}) { SCOPED_TRACE("Testing with prefix = " + prefix); - auto actual = MetricsExporterOptions( - Project("test-only"), TestResource(), + auto actual = MetricsExporterConnectionOptions( Options{}.set(prefix + "private.googleapis.com")); EXPECT_THAT(actual.get(), "private.googleapis.com"); - EXPECT_TRUE(actual.has()); - EXPECT_TRUE(actual.get()); - EXPECT_EQ(actual.get(), - "storage.googleapis.com/"); } } -TEST(MetricsExporterOptions, PrivateUD) { +TEST(MetricsExporterConnectionOptions, PrivateUD) { for (std::string prefix : {"", "google-c2p:///"}) { SCOPED_TRACE("Testing with prefix = " + prefix); - auto actual = MetricsExporterOptions( - Project("test-only"), TestResource(), + auto actual = MetricsExporterConnectionOptions( Options{} .set(prefix + "private.ud.net") .set("ud.net")); EXPECT_THAT(actual.get(), "private.ud.net"); - EXPECT_TRUE(actual.has()); - EXPECT_TRUE(actual.get()); - EXPECT_EQ(actual.get(), - "storage.googleapis.com/"); } } -TEST(MetricsExporterOptions, RestrictedDefaultUD) { +TEST(MetricsExporterConnectionOptions, RestrictedDefaultUD) { for (std::string prefix : {"", "google-c2p:///"}) { SCOPED_TRACE("Testing with prefix = " + prefix); - auto actual = MetricsExporterOptions( - Project("test-only"), TestResource(), + auto actual = MetricsExporterConnectionOptions( Options{}.set(prefix + "restricted.googleapis.com")); EXPECT_THAT(actual.get(), "restricted.googleapis.com"); - EXPECT_TRUE(actual.has()); - EXPECT_TRUE(actual.get()); - EXPECT_EQ(actual.get(), - "storage.googleapis.com/"); } } -TEST(MetricsExporterOptions, RestrictedUD) { +TEST(MetricsExporterConnectionOptions, RestrictedUD) { for (std::string prefix : {"", "google-c2p:///"}) { SCOPED_TRACE("Testing with prefix = " + prefix); - auto actual = MetricsExporterOptions( - Project("test-only"), TestResource(), + auto actual = MetricsExporterConnectionOptions( Options{} .set(prefix + "restricted.ud.net") .set("ud.net")); EXPECT_THAT(actual.get(), "restricted.ud.net"); - EXPECT_TRUE(actual.has()); - EXPECT_TRUE(actual.get()); - EXPECT_EQ(actual.get(), - "storage.googleapis.com/"); } } @@ -160,15 +131,13 @@ MATCHER(MatchesInstanceId, "looks like an instance id") { } TEST(MetricsExporterOptions, MonitoredResource) { - auto actual = - MetricsExporterOptions(Project("test-project"), - opentelemetry::sdk::resource::Resource::Create({ - {"cloud.availability_zone", "us-central1-c"}, - {"cloud.region", "us-central1"}, - {"cloud.platform", "gcp"}, - {"host.id", "test-host-id"}, - }), - Options{}); + auto actual = MetricsExporterOptions( + Project("test-project"), opentelemetry::sdk::resource::Resource::Create({ + {"cloud.availability_zone", "us-central1-c"}, + {"cloud.region", "us-central1"}, + {"cloud.platform", "gcp"}, + {"host.id", "test-host-id"}, + })); EXPECT_TRUE(actual.has()); auto mr = actual.get(); @@ -190,6 +159,31 @@ TEST(MetricsExporterOptions, MonitoredResource) { EXPECT_THAT(mr, IsProtoEqual(expected)); } +TEST(MetricsExporterOptions, DefaultMonitoredResource) { + auto actual = MetricsExporterOptions( + Project("test-project"), + opentelemetry::sdk::resource::Resource::Create({})); + + EXPECT_TRUE(actual.has()); + auto mr = actual.get(); + auto labels = mr.labels(); + // The `instance_id` label has unpredictable values, + EXPECT_THAT(labels, Contains(Pair("instance_id", MatchesInstanceId()))); + mr.mutable_labels()->erase("instance_id"); + + auto constexpr kExpected = R"pb( + type: "storage_client" + labels { key: "project_id" value: "test-project" } + labels { key: "location" value: "global" } + labels { key: "cloud_platform" value: "unknown" } + labels { key: "host_id" value: "unknown" } + labels { key: "api" value: "GRPC" } + )pb"; + auto expected = google::api::MonitoredResource{}; + ASSERT_TRUE(TextFormat::ParseFromString(kExpected, &expected)); + EXPECT_THAT(mr, IsProtoEqual(expected)); +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage_internal