From 2999332d64c68181bb7196b576b01cf7fd5d1a9a Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 9 Feb 2024 09:25:20 -0800 Subject: [PATCH] [CSM] De-experimentalize CsmObservability API (#35836) Closes #35836 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35836 from yashykt:DeexperimentalizeCsmObs 1711e6d9b08199f92c45e0b5fdd69372051eb632 PiperOrigin-RevId: 605650394 --- examples/cpp/csm/csm_greeter_client.cc | 4 ++-- examples/cpp/csm/csm_greeter_server.cc | 2 +- include/grpcpp/ext/csm_observability.h | 10 ++++++++-- src/cpp/ext/csm/csm_observability.cc | 3 --- test/cpp/ext/csm/csm_observability_test.cc | 18 +++++++++--------- test/cpp/interop/xds_interop_client.cc | 6 +++--- test/cpp/interop/xds_interop_server.cc | 6 +++--- 7 files changed, 26 insertions(+), 23 deletions(-) diff --git a/examples/cpp/csm/csm_greeter_client.cc b/examples/cpp/csm/csm_greeter_client.cc index 3aba7058562..a689fdfa0d2 100644 --- a/examples/cpp/csm/csm_greeter_client.cc +++ b/examples/cpp/csm/csm_greeter_client.cc @@ -158,7 +158,7 @@ class GreeterClient { absl::optional cookie_from_server_; }; -absl::StatusOr InitializeObservability() { +absl::StatusOr InitializeObservability() { opentelemetry::exporter::metrics::PrometheusExporterOptions opts; // default was "localhost:9464" which causes connection issue across GKE pods opts.url = "0.0.0.0:9464"; @@ -167,7 +167,7 @@ absl::StatusOr InitializeObservability() { auto meter_provider = std::make_shared(); meter_provider->AddMetricReader(std::move(prometheus_exporter)); - return grpc::experimental::CsmObservabilityBuilder() + return grpc::CsmObservabilityBuilder() .SetMeterProvider(std::move(meter_provider)) .BuildAndRegister(); } diff --git a/examples/cpp/csm/csm_greeter_server.cc b/examples/cpp/csm/csm_greeter_server.cc index 0e6d24bfd15..c5d7857e7fa 100644 --- a/examples/cpp/csm/csm_greeter_server.cc +++ b/examples/cpp/csm/csm_greeter_server.cc @@ -108,7 +108,7 @@ int main(int argc, char** argv) { auto meter_provider = std::make_shared(); meter_provider->AddMetricReader(std::move(prometheus_exporter)); - auto observability = grpc::experimental::CsmObservabilityBuilder() + auto observability = grpc::CsmObservabilityBuilder() .SetMeterProvider(std::move(meter_provider)) .BuildAndRegister(); if (!observability.ok()) { diff --git a/include/grpcpp/ext/csm_observability.h b/include/grpcpp/ext/csm_observability.h index 86463acf5f7..dcb7bd8c29c 100644 --- a/include/grpcpp/ext/csm_observability.h +++ b/include/grpcpp/ext/csm_observability.h @@ -36,8 +36,6 @@ namespace internal { class OpenTelemetryPluginBuilderImpl; } // namespace internal -namespace experimental { - // This object maintains state around the registered CsmObservability plugin. // The application is responsible for retaining this object until it has closed // all channels and servers that are recording metrics. @@ -102,7 +100,15 @@ class CsmObservabilityBuilder { std::unique_ptr builder_; }; +namespace experimental { +// TODO(yashykt): Remove this once no longer needed. +using CsmObservability GRPC_DEPRECATED("Use grpc::CsmObservability instead.") = + grpc::CsmObservability; +using CsmObservabilityBuilder GRPC_DEPRECATED( + "Use grpc::CsmObservabilityBuilder instead.") = + grpc::CsmObservabilityBuilder; } // namespace experimental + } // namespace grpc #endif // GRPCPP_EXT_CSM_OBSERVABILITY_H diff --git a/src/cpp/ext/csm/csm_observability.cc b/src/cpp/ext/csm/csm_observability.cc index 0b503455efe..f400a919229 100644 --- a/src/cpp/ext/csm/csm_observability.cc +++ b/src/cpp/ext/csm/csm_observability.cc @@ -100,8 +100,6 @@ class CsmOpenTelemetryPluginOption } // namespace internal -namespace experimental { - // // CsmObservability // @@ -164,5 +162,4 @@ absl::StatusOr CsmObservabilityBuilder::BuildAndRegister() { return CsmObservability(); } -} // namespace experimental } // namespace grpc diff --git a/test/cpp/ext/csm/csm_observability_test.cc b/test/cpp/ext/csm/csm_observability_test.cc index 34af9edd58b..63c2af641c8 100644 --- a/test/cpp/ext/csm/csm_observability_test.cc +++ b/test/cpp/ext/csm/csm_observability_test.cc @@ -32,7 +32,7 @@ namespace testing { namespace { TEST(CsmObservabilityBuilderTest, Basic) { - EXPECT_EQ(experimental::CsmObservabilityBuilder().BuildAndRegister().status(), + EXPECT_EQ(CsmObservabilityBuilder().BuildAndRegister().status(), absl::OkStatus()); } @@ -41,7 +41,7 @@ TEST(GsmDependencyTest, GoogleCloudOpenTelemetryDependency) { } TEST(CsmChannelTargetSelectorTest, NonXdsTargets) { - auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + auto obs = CsmObservabilityBuilder().BuildAndRegister(); EXPECT_FALSE(internal::CsmChannelTargetSelector("foo.bar.google.com")); EXPECT_FALSE(internal::CsmChannelTargetSelector("dns:///foo.bar.google.com")); EXPECT_FALSE( @@ -51,24 +51,24 @@ TEST(CsmChannelTargetSelectorTest, NonXdsTargets) { } TEST(CsmChannelTargetSelectorTest, XdsTargets) { - auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + auto obs = CsmObservabilityBuilder().BuildAndRegister(); EXPECT_TRUE(internal::CsmChannelTargetSelector("xds:///foo")); EXPECT_TRUE(internal::CsmChannelTargetSelector("xds:///foo.bar")); } TEST(CsmChannelTargetSelectorTest, XdsTargetsWithNonTDAuthority) { - auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + auto obs = CsmObservabilityBuilder().BuildAndRegister(); EXPECT_FALSE(internal::CsmChannelTargetSelector("xds://authority/foo")); } TEST(CsmChannelTargetSelectorTest, XdsTargetsWithTDAuthority) { - auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + auto obs = CsmObservabilityBuilder().BuildAndRegister(); EXPECT_TRUE(internal::CsmChannelTargetSelector( "xds://traffic-director-global.xds.googleapis.com/foo")); } TEST(CsmChannelTargetSelectorTest, CsmObservabilityOutOfScope) { - { auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); } + { auto obs = CsmObservabilityBuilder().BuildAndRegister(); } // When CsmObservability goes out of scope, the target selector should return // false as well. EXPECT_FALSE(internal::CsmChannelTargetSelector("foo.bar.google.com")); @@ -78,18 +78,18 @@ TEST(CsmChannelTargetSelectorTest, CsmObservabilityOutOfScope) { } TEST(CsmServerSelectorTest, ChannelArgsWithoutXdsServerArg) { - auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + auto obs = CsmObservabilityBuilder().BuildAndRegister(); EXPECT_FALSE(internal::CsmServerSelector(grpc_core::ChannelArgs())); } TEST(CsmServerSelectorTest, ChannelArgsWithXdsServerArg) { - auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + auto obs = CsmObservabilityBuilder().BuildAndRegister(); EXPECT_TRUE(internal::CsmServerSelector( grpc_core::ChannelArgs().Set(GRPC_ARG_XDS_ENABLED_SERVER, true))); } TEST(CsmServerSelectorTest, CsmObservabilityOutOfScope) { - { auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); } + { auto obs = CsmObservabilityBuilder().BuildAndRegister(); } // When CsmObservability goes out of scope, the server selector should return // false as well. EXPECT_FALSE(internal::CsmServerSelector(grpc_core::ChannelArgs())); diff --git a/test/cpp/interop/xds_interop_client.cc b/test/cpp/interop/xds_interop_client.cc index 76dab581fa3..d5c97c2ddce 100644 --- a/test/cpp/interop/xds_interop_client.cc +++ b/test/cpp/interop/xds_interop_client.cc @@ -427,7 +427,7 @@ void RunTestLoop(std::chrono::duration duration_per_query, GPR_UNREACHABLE_CODE(thread.join()); } -grpc::experimental::CsmObservability EnableCsmObservability() { +grpc::CsmObservability EnableCsmObservability() { gpr_log(GPR_DEBUG, "Registering Prometheus exporter"); opentelemetry::exporter::metrics::PrometheusExporterOptions opts; // default was "localhost:9464" which causes connection issue across GKE @@ -438,7 +438,7 @@ grpc::experimental::CsmObservability EnableCsmObservability() { auto meter_provider = std::make_shared(); meter_provider->AddMetricReader(std::move(prometheus_exporter)); - auto observability = grpc::experimental::CsmObservabilityBuilder() + auto observability = grpc::CsmObservabilityBuilder() .SetMeterProvider(std::move(meter_provider)) .BuildAndRegister(); assert(observability.ok()); @@ -553,7 +553,7 @@ int main(int argc, char** argv) { } BuildRpcConfigsFromFlags(&rpc_config_queue); - grpc::experimental::CsmObservability observability; + grpc::CsmObservability observability; if (absl::GetFlag(FLAGS_enable_csm_observability)) { observability = EnableCsmObservability(); } diff --git a/test/cpp/interop/xds_interop_server.cc b/test/cpp/interop/xds_interop_server.cc index 80637dfa050..6de9c18042a 100644 --- a/test/cpp/interop/xds_interop_server.cc +++ b/test/cpp/interop/xds_interop_server.cc @@ -43,7 +43,7 @@ ABSL_FLAG(bool, secure_mode, false, ABSL_FLAG(bool, enable_csm_observability, false, "Whether to enable CSM Observability"); -grpc::experimental::CsmObservability EnableCsmObservability() { +grpc::CsmObservability EnableCsmObservability() { gpr_log(GPR_DEBUG, "Registering Prometheus exporter"); opentelemetry::exporter::metrics::PrometheusExporterOptions opts; // default was "localhost:9464" which causes connection issue across GKE @@ -54,7 +54,7 @@ grpc::experimental::CsmObservability EnableCsmObservability() { auto meter_provider = std::make_shared(); meter_provider->AddMetricReader(std::move(prometheus_exporter)); - auto observability = grpc::experimental::CsmObservabilityBuilder() + auto observability = grpc::CsmObservabilityBuilder() .SetMeterProvider(std::move(meter_provider)) .BuildAndRegister(); assert(observability.ok()); @@ -81,7 +81,7 @@ int main(int argc, char** argv) { } grpc::EnableDefaultHealthCheckService(false); bool enable_csm_observability = absl::GetFlag(FLAGS_enable_csm_observability); - grpc::experimental::CsmObservability observability; + grpc::CsmObservability observability; if (enable_csm_observability) { observability = EnableCsmObservability(); }