[otel] Backport: Return absl::Status as a return from BuildAndRegiste…

…rGlobal (#35659) (#35672) Just to be future-proof, I'm amending the `void` return status of `BuildAndRegisterGlobal` in `OpenTelemetryPluginBuilder` to absl::Status.
grpc · Jan 26, 2024 · dfa07e7 · dfa07e7
1 parent 972adc5
commit dfa07e7
Show file tree

Hide file tree

Showing 9 changed files with 33 additions and 22 deletions.
diff --git a/include/grpcpp/ext/otel_plugin.h b/include/grpcpp/ext/otel_plugin.h
@@ -27,6 +27,7 @@
 #include <memory>
 
 #include "absl/functional/any_invocable.h"
+#include "absl/status/status.h"
 #include "absl/strings/string_view.h"
 #include "opentelemetry/metrics/meter_provider.h"
 
@@ -107,7 +108,7 @@ class OpenTelemetryPluginBuilder {
       std::unique_ptr<OpenTelemetryPluginOption> option);
   /// Registers a global plugin that acts on all channels and servers running on
   /// the process.
-  void BuildAndRegisterGlobal();
+  absl::Status BuildAndRegisterGlobal();
 
  private:
   std::unique_ptr<internal::OpenTelemetryPluginBuilderImpl> impl_;

diff --git a/src/cpp/ext/csm/csm_observability.cc b/src/cpp/ext/csm/csm_observability.cc
@@ -137,7 +137,10 @@ absl::StatusOr<CsmObservability> CsmObservabilityBuilder::BuildAndRegister() {
           google::cloud::otel::MakeResourceDetector()
               ->Detect()
               .GetAttributes()));
-  builder_->BuildAndRegisterGlobal();
+  auto status = builder_->BuildAndRegisterGlobal();
+  if (!status.ok()) {
+    return status;
+  }
   return CsmObservability();
 }
 

diff --git a/src/cpp/ext/otel/otel_plugin.cc b/src/cpp/ext/otel/otel_plugin.cc
@@ -163,13 +163,13 @@ OpenTelemetryPluginBuilderImpl& OpenTelemetryPluginBuilderImpl::AddPluginOption(
   return *this;
 }
 
-void OpenTelemetryPluginBuilderImpl::BuildAndRegisterGlobal() {
+absl::Status OpenTelemetryPluginBuilderImpl::BuildAndRegisterGlobal() {
   opentelemetry::nostd::shared_ptr<opentelemetry::metrics::MeterProvider>
       meter_provider = meter_provider_;
   delete g_otel_plugin_state_;
   g_otel_plugin_state_ = new struct OpenTelemetryPluginState;
   if (meter_provider == nullptr) {
-    return;
+    return absl::OkStatus();
   }
   auto meter = meter_provider->GetMeter("grpc-c++", GRPC_CPP_VERSION_STRING);
   if (metrics_.contains(grpc::OpenTelemetryPluginBuilder::
@@ -269,6 +269,7 @@ void OpenTelemetryPluginBuilderImpl::BuildAndRegisterGlobal() {
                          args.GetString(GRPC_ARG_SERVER_URI).value_or(""));
             });
       });
+  return absl::OkStatus();
 }
 
 }  // namespace internal
@@ -331,8 +332,8 @@ OpenTelemetryPluginBuilder& OpenTelemetryPluginBuilder::AddPluginOption(
   return *this;
 }
 
-void OpenTelemetryPluginBuilder::BuildAndRegisterGlobal() {
-  impl_->BuildAndRegisterGlobal();
+absl::Status OpenTelemetryPluginBuilder::BuildAndRegisterGlobal() {
+  return impl_->BuildAndRegisterGlobal();
 }
 
 }  // namespace grpc
diff --git a/src/cpp/ext/otel/otel_plugin.h b/src/cpp/ext/otel/otel_plugin.h
@@ -206,7 +206,7 @@ class OpenTelemetryPluginBuilderImpl {
           generic_method_attribute_filter);
   OpenTelemetryPluginBuilderImpl& AddPluginOption(
       std::unique_ptr<InternalOpenTelemetryPluginOption> option);
-  void BuildAndRegisterGlobal();
+  absl::Status BuildAndRegisterGlobal();
 
  private:
   std::shared_ptr<opentelemetry::metrics::MeterProvider> meter_provider_;

diff --git a/test/cpp/ext/csm/csm_observability_test.cc b/test/cpp/ext/csm/csm_observability_test.cc
@@ -32,8 +32,8 @@ namespace testing {
 namespace {
 
 TEST(CsmObservabilityBuilderTest, Basic) {
-  EXPECT_TRUE(
-      experimental::CsmObservabilityBuilder().BuildAndRegister().status().ok());
+  EXPECT_EQ(experimental::CsmObservabilityBuilder().BuildAndRegister().status(),
+            absl::OkStatus());
 }
 
 TEST(GsmDependencyTest, GoogleCloudOpenTelemetryDependency) {
@@ -64,9 +64,11 @@ TEST(CsmChannelTargetSelectorTest, XdsTargetsWithTDAuthority) {
 }
 
 TEST(CsmPluginOptionTest, Basic) {
-  OpenTelemetryPluginBuilder()
-      .AddPluginOption(experimental::MakeCsmOpenTelemetryPluginOption())
-      .BuildAndRegisterGlobal();
+  EXPECT_EQ(
+      OpenTelemetryPluginBuilder()
+          .AddPluginOption(experimental::MakeCsmOpenTelemetryPluginOption())
+          .BuildAndRegisterGlobal(),
+      absl::OkStatus());
 }
 
 }  // namespace

diff --git a/test/cpp/ext/otel/otel_test_library.cc b/test/cpp/ext/otel/otel_test_library.cc
@@ -131,7 +131,7 @@ void OpenTelemetryPluginEnd2EndTest::Init(
   for (auto& option : plugin_options) {
     ot_builder.AddPluginOption(std::move(option));
   }
-  ot_builder.BuildAndRegisterGlobal();
+  ASSERT_EQ(ot_builder.BuildAndRegisterGlobal(), absl::OkStatus());
   ChannelArguments channel_args;
   if (!labels_to_inject.empty()) {
     labels_to_inject_ = labels_to_inject;

diff --git a/test/cpp/interop/observability_client.cc b/test/cpp/interop/observability_client.cc
@@ -238,7 +238,7 @@ int main(int argc, char** argv) {
     meter_provider->AddMetricReader(std::move(prometheus_exporter));
     grpc::OpenTelemetryPluginBuilder otel_builder;
     otel_builder.SetMeterProvider(std::move(meter_provider));
-    otel_builder.BuildAndRegisterGlobal();
+    assert(otel_builder.BuildAndRegisterGlobal().ok());
   }
 
   grpc::testing::ChannelCreationFunc channel_creation_func;

diff --git a/test/cpp/interop/xds_interop_client.cc b/test/cpp/interop/xds_interop_client.cc
@@ -403,10 +403,12 @@ void EnableCsmObservability() {
   auto meter_provider =
       std::make_shared<opentelemetry::sdk::metrics::MeterProvider>();
   meter_provider->AddMetricReader(std::move(prometheus_exporter));
-  grpc::OpenTelemetryPluginBuilder()
-      .AddPluginOption(grpc::experimental::MakeCsmOpenTelemetryPluginOption())
-      .SetMeterProvider(std::move(meter_provider))
-      .BuildAndRegisterGlobal();
+  assert(grpc::OpenTelemetryPluginBuilder()
+             .AddPluginOption(
+                 grpc::experimental::MakeCsmOpenTelemetryPluginOption())
+             .SetMeterProvider(std::move(meter_provider))
+             .BuildAndRegisterGlobal()
+             .ok());
 }
 
 void RunServer(const int port, StatsWatchers* stats_watchers,

diff --git a/test/cpp/interop/xds_interop_server.cc b/test/cpp/interop/xds_interop_server.cc
@@ -54,10 +54,12 @@ void EnableCsmObservability() {
   auto meter_provider =
       std::make_shared<opentelemetry::sdk::metrics::MeterProvider>();
   meter_provider->AddMetricReader(std::move(prometheus_exporter));
-  grpc::OpenTelemetryPluginBuilder()
-      .AddPluginOption(grpc::experimental::MakeCsmOpenTelemetryPluginOption())
-      .SetMeterProvider(std::move(meter_provider))
-      .BuildAndRegisterGlobal();
+  assert(grpc::OpenTelemetryPluginBuilder()
+             .AddPluginOption(
+                 grpc::experimental::MakeCsmOpenTelemetryPluginOption())
+             .SetMeterProvider(std::move(meter_provider))
+             .BuildAndRegisterGlobal()
+             .ok());
 }
 
 int main(int argc, char** argv) {