diff --git a/include/grpcpp/ext/csm_observability.h b/include/grpcpp/ext/csm_observability.h index 706807032d7..86463acf5f7 100644 --- a/include/grpcpp/ext/csm_observability.h +++ b/include/grpcpp/ext/csm_observability.h @@ -38,16 +38,21 @@ class OpenTelemetryPluginBuilderImpl; namespace experimental { -// This is a no-op at present, but in the future, this object would be useful -// for performing cleanup. +// 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. class CsmObservability { public: CsmObservability() = default; + ~CsmObservability(); // Disable copy constructor and copy-assignment operator. CsmObservability(const CsmObservability&) = delete; CsmObservability& operator=(const CsmObservability&) = delete; - CsmObservability(CsmObservability&&) = default; - CsmObservability& operator=(CsmObservability&&) = default; + CsmObservability(CsmObservability&&) noexcept; + CsmObservability& operator=(CsmObservability&&) noexcept; + + private: + bool valid_ = true; }; // CsmObservabilityBuilder configures observability for all service mesh traffic diff --git a/src/cpp/ext/csm/csm_observability.cc b/src/cpp/ext/csm/csm_observability.cc index 0bb2590daaf..0b503455efe 100644 --- a/src/cpp/ext/csm/csm_observability.cc +++ b/src/cpp/ext/csm/csm_observability.cc @@ -45,11 +45,17 @@ namespace grpc { namespace internal { +namespace { +std::atomic g_csm_plugin_enabled(false); +} + bool CsmServerSelector(const grpc_core::ChannelArgs& args) { - return args.GetBool(GRPC_ARG_XDS_ENABLED_SERVER).value_or(false); + return g_csm_plugin_enabled && + args.GetBool(GRPC_ARG_XDS_ENABLED_SERVER).value_or(false); } bool CsmChannelTargetSelector(absl::string_view target) { + if (!g_csm_plugin_enabled) return false; auto uri = grpc_core::URI::Parse(target); if (!uri.ok()) { gpr_log(GPR_ERROR, "Failed to parse URI: %s", std::string(target).c_str()); @@ -96,6 +102,25 @@ class CsmOpenTelemetryPluginOption namespace experimental { +// +// CsmObservability +// + +CsmObservability::~CsmObservability() { + if (valid_) { + internal::g_csm_plugin_enabled = false; + } +} + +CsmObservability::CsmObservability(CsmObservability&& other) noexcept { + other.valid_ = false; +} +CsmObservability& CsmObservability::operator=( + CsmObservability&& other) noexcept { + other.valid_ = false; + return *this; +} + // // CsmObservabilityBuilder // @@ -132,6 +157,7 @@ absl::StatusOr CsmObservabilityBuilder::BuildAndRegister() { builder_->AddPluginOption( std::make_unique()); auto status = builder_->BuildAndRegisterGlobal(); + internal::g_csm_plugin_enabled = true; if (!status.ok()) { return status; } diff --git a/src/cpp/ext/csm/csm_observability.h b/src/cpp/ext/csm/csm_observability.h index d882f230177..1f908138f34 100644 --- a/src/cpp/ext/csm/csm_observability.h +++ b/src/cpp/ext/csm/csm_observability.h @@ -23,6 +23,8 @@ #include "absl/strings/string_view.h" +#include "src/core/lib/channel/channel_args.h" + namespace grpc { namespace internal { @@ -30,6 +32,10 @@ namespace internal { // Returns true if the channel is a CSM channel. bool CsmChannelTargetSelector(absl::string_view target); +// EXPOSED FOR TESTING PURPOSES ONLY +// Returns true if the server is a CSM server. +bool CsmServerSelector(const grpc_core::ChannelArgs& args); + } // namespace internal } // namespace grpc diff --git a/test/cpp/ext/csm/csm_observability_test.cc b/test/cpp/ext/csm/csm_observability_test.cc index 69073ff8f92..34af9edd58b 100644 --- a/test/cpp/ext/csm/csm_observability_test.cc +++ b/test/cpp/ext/csm/csm_observability_test.cc @@ -24,7 +24,7 @@ #include #include -#include "src/core/lib/gprpp/env.h" +#include "src/core/ext/xds/xds_enabled_server.h" #include "test/core/util/test_config.h" namespace grpc { @@ -41,6 +41,7 @@ TEST(GsmDependencyTest, GoogleCloudOpenTelemetryDependency) { } TEST(CsmChannelTargetSelectorTest, NonXdsTargets) { + auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); EXPECT_FALSE(internal::CsmChannelTargetSelector("foo.bar.google.com")); EXPECT_FALSE(internal::CsmChannelTargetSelector("dns:///foo.bar.google.com")); EXPECT_FALSE( @@ -50,19 +51,52 @@ TEST(CsmChannelTargetSelectorTest, NonXdsTargets) { } TEST(CsmChannelTargetSelectorTest, XdsTargets) { + auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); EXPECT_TRUE(internal::CsmChannelTargetSelector("xds:///foo")); EXPECT_TRUE(internal::CsmChannelTargetSelector("xds:///foo.bar")); } TEST(CsmChannelTargetSelectorTest, XdsTargetsWithNonTDAuthority) { + auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); EXPECT_FALSE(internal::CsmChannelTargetSelector("xds://authority/foo")); } TEST(CsmChannelTargetSelectorTest, XdsTargetsWithTDAuthority) { + auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); EXPECT_TRUE(internal::CsmChannelTargetSelector( "xds://traffic-director-global.xds.googleapis.com/foo")); } +TEST(CsmChannelTargetSelectorTest, CsmObservabilityOutOfScope) { + { auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); } + // When CsmObservability goes out of scope, the target selector should return + // false as well. + EXPECT_FALSE(internal::CsmChannelTargetSelector("foo.bar.google.com")); + EXPECT_FALSE(internal::CsmChannelTargetSelector("xds:///foo")); + EXPECT_FALSE(internal::CsmChannelTargetSelector( + "xds://traffic-director-global.xds.googleapis.com/foo")); +} + +TEST(CsmServerSelectorTest, ChannelArgsWithoutXdsServerArg) { + auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + EXPECT_FALSE(internal::CsmServerSelector(grpc_core::ChannelArgs())); +} + +TEST(CsmServerSelectorTest, ChannelArgsWithXdsServerArg) { + auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); + EXPECT_TRUE(internal::CsmServerSelector( + grpc_core::ChannelArgs().Set(GRPC_ARG_XDS_ENABLED_SERVER, true))); +} + +TEST(CsmServerSelectorTest, CsmObservabilityOutOfScope) { + { auto obs = experimental::CsmObservabilityBuilder().BuildAndRegister(); } + // When CsmObservability goes out of scope, the server selector should return + // false as well. + EXPECT_FALSE(internal::CsmServerSelector(grpc_core::ChannelArgs())); + EXPECT_FALSE(internal::CsmServerSelector( + grpc_core::ChannelArgs().Set(GRPC_ARG_XDS_ENABLED_SERVER, true))); +} + } // namespace } // namespace testing } // namespace grpc