[SDK] Remove noexcept from the SDK Span constructor

CHANGELOG.md

@@ -23,6 +23,10 @@ Increment the:
 * [SDK] Fix disabled Tracer StartSpan noop behavior
   [#4710](https://github.com/open-telemetry/opentelemetry-cpp/pull/4710)
 
+* [SDK] Return a null recordable when MakeRecordable() fails to allocate,
+  instead of terminating the process from a noexcept function.
+  [#4700](https://github.com/open-telemetry/opentelemetry-cpp/pull/4700)
+
 * [SDK] Preserve the default unlimited timeout when batch span and log record
   processors shut down their exporters.
   [#4686](https://github.com/open-telemetry/opentelemetry-cpp/pull/4686)

exporters/memory/include/opentelemetry/exporters/memory/in_memory_span_exporter.h

@@ -45,7 +45,18 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
    */
   std::unique_ptr<sdk::trace::Recordable> MakeRecordable() noexcept override
   {
-    return std::unique_ptr<sdk::trace::Recordable>(new sdk::trace::SpanData());
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+    try
+    {
+#endif
+      return std::unique_ptr<sdk::trace::Recordable>(new sdk::trace::SpanData());
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+    }
+    catch (...)
+    {
+      return nullptr;
+    }
+#endif
   }
 
   /**

exporters/ostream/src/span_exporter.cc

@@ -68,7 +68,18 @@ OStreamSpanExporter::OStreamSpanExporter(std::ostream &sout) noexcept : sout_(so
 
 std::unique_ptr<trace_sdk::Recordable> OStreamSpanExporter::MakeRecordable() noexcept
 {
-  return std::unique_ptr<trace_sdk::Recordable>(new trace_sdk::SpanData);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    return std::unique_ptr<trace_sdk::Recordable>(new trace_sdk::SpanData);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)
+  {
+    return nullptr;
+  }
+#endif
 }
 
 sdk::common::ExportResult OStreamSpanExporter::Export(

exporters/otlp/src/otlp_file_exporter.cc

@@ -50,7 +50,18 @@ OtlpFileExporter::OtlpFileExporter(const OtlpFileExporterOptions &options,
 
 std::unique_ptr<opentelemetry::sdk::trace::Recordable> OtlpFileExporter::MakeRecordable() noexcept
 {
-  return std::make_unique<exporter::otlp::OtlpRecordable>();
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    return std::make_unique<exporter::otlp::OtlpRecordable>();
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)
+  {
+    return nullptr;
+  }
+#endif
 }
 
 opentelemetry::sdk::common::ExportResult OtlpFileExporter::Export(

exporters/otlp/src/otlp_grpc_exporter.cc

@@ -92,9 +92,20 @@ OtlpGrpcExporter::~OtlpGrpcExporter()
 
 std::unique_ptr<sdk::trace::Recordable> OtlpGrpcExporter::MakeRecordable() noexcept
 {
-  return std::make_unique<OtlpRecordable>(options_.max_attributes, options_.max_events,
-                                          options_.max_links, options_.max_attributes_per_event,
-                                          options_.max_attributes_per_link);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    return std::make_unique<OtlpRecordable>(options_.max_attributes, options_.max_events,
+                                            options_.max_links, options_.max_attributes_per_event,
+                                            options_.max_attributes_per_link);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)
+  {
+    return nullptr;
+  }
+#endif
 }
 
 sdk::common::ExportResult OtlpGrpcExporter::Export(

exporters/otlp/src/otlp_http_exporter.cc

@@ -301,9 +301,20 @@ OtlpHttpExporter::OtlpHttpExporter(std::unique_ptr<OtlpHttpClient> http_client)
 
 std::unique_ptr<opentelemetry::sdk::trace::Recordable> OtlpHttpExporter::MakeRecordable() noexcept
 {
-  return std::make_unique<exporter::otlp::OtlpRecordable>(
-      options_.max_attributes, options_.max_events, options_.max_links,
-      options_.max_attributes_per_event, options_.max_attributes_per_link);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    return std::make_unique<exporter::otlp::OtlpRecordable>(
+        options_.max_attributes, options_.max_events, options_.max_links,
+        options_.max_attributes_per_event, options_.max_attributes_per_link);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)
+  {
+    return nullptr;
+  }
+#endif
 }
 
 opentelemetry::sdk::common::ExportResult OtlpHttpExporter::Export(

exporters/zipkin/src/zipkin_exporter.cc

@@ -66,7 +66,18 @@ ZipkinExporter::ZipkinExporter(
 
 std::unique_ptr<sdk::trace::Recordable> ZipkinExporter::MakeRecordable() noexcept
 {
-  return std::unique_ptr<sdk::trace::Recordable>(new Recordable);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    return std::unique_ptr<sdk::trace::Recordable>(new Recordable);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)
+  {
+    return nullptr;
+  }
+#endif
 }
 
 sdk::common::ExportResult ZipkinExporter::Export(

sdk/include/opentelemetry/sdk/trace/multi_span_processor.h

@@ -6,6 +6,7 @@
 #include <chrono>
 #include <memory>
 #include <mutex>
+#include <utility>
 #include <vector>
 
 #include "opentelemetry/sdk/trace/multi_recordable.h"
@@ -65,16 +66,31 @@ class MultiSpanProcessor : public SpanProcessor
 
   std::unique_ptr<Recordable> MakeRecordable() noexcept override
   {
-    auto recordable       = std::unique_ptr<Recordable>(new MultiRecordable);
-    auto multi_recordable = static_cast<MultiRecordable *>(recordable.get());
-    ProcessorNode *node   = head_;
-    while (node != nullptr)
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+    try
     {
-      auto processor = node->value_.get();
-      multi_recordable->AddRecordable(*processor, processor->MakeRecordable());
-      node = node->next_;
+#endif
+      auto recordable       = std::unique_ptr<Recordable>(new MultiRecordable);
+      auto multi_recordable = static_cast<MultiRecordable *>(recordable.get());
+      ProcessorNode *node   = head_;
+      while (node != nullptr)
+      {
+        auto processor      = node->value_.get();
+        auto sub_recordable = processor->MakeRecordable();
+        if (sub_recordable != nullptr)
+        {
+          multi_recordable->AddRecordable(*processor, std::move(sub_recordable));
+        }
+        node = node->next_;
+      }
+      return recordable;
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+    }
+    catch (...)
+    {
+      return nullptr;
     }
-    return recordable;
+#endif
   }
 
   void OnStart(Recordable &span,

sdk/src/trace/multi_recordable.cc

@@ -31,7 +31,18 @@ std::size_t MultiRecordable::MakeKey(const SpanProcessor &processor) noexcept
 void MultiRecordable::AddRecordable(const SpanProcessor &processor,
                                     std::unique_ptr<Recordable> recordable) noexcept
 {
-  recordables_[MakeKey(processor)] = std::move(recordable);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    recordables_[MakeKey(processor)] = std::move(recordable);
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)  // NOLINT(bugprone-empty-catch)
+  {
+    // Drop the recordable, the processor will skip this span.
+  }
+#endif
 }
 
 const std::unique_ptr<Recordable> &MultiRecordable::GetRecordable(

sdk/test/trace/multi_span_processor_test.cc

@@ -12,6 +12,7 @@
 #include "opentelemetry/sdk/trace/processor.h"
 #include "opentelemetry/sdk/trace/recordable.h"
 #include "opentelemetry/sdk/trace/span_data.h"
+#include "opentelemetry/trace/span_context.h"
 
 using namespace opentelemetry::sdk::trace;
 using opentelemetry::trace::SpanContext;
@@ -47,6 +48,28 @@ class FixedResultProcessor final : public SpanProcessor
   bool result_;
 };
 
+// A processor that fails to create a recordable and counts the spans it receives.
+class NullRecordableProcessor final : public SpanProcessor
+{
+public:
+  std::unique_ptr<Recordable> MakeRecordable() noexcept override { return nullptr; }
+
+  void OnStart(Recordable & /* span */, const SpanContext & /* parent_context */) noexcept override
+  {
+    ++started_;
+  }
+
+  // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved)
+  void OnEnd(std::unique_ptr<Recordable> && /* span */) noexcept override { ++ended_; }
+
+  bool ForceFlush(std::chrono::microseconds /* timeout */) noexcept override { return true; }
+
+  bool Shutdown(std::chrono::microseconds /* timeout */) noexcept override { return true; }
+
+  int started_ = 0;
+  int ended_   = 0;
+};
+
 std::unique_ptr<SpanProcessor> MakeMultiProcessor(bool first, bool second)
 {
   std::vector<std::unique_ptr<SpanProcessor>> processors;
@@ -71,4 +94,24 @@ TEST(MultiSpanProcessorTest, ShutdownFailsWhenAnyChildFails)
   EXPECT_FALSE(MakeMultiProcessor(false, true)->Shutdown());
 }
 
+TEST(MultiSpanProcessorTest, MakeRecordableSkipsChildWithoutRecordable)
+{
+  auto *null_processor = new NullRecordableProcessor();
+  std::vector<std::unique_ptr<SpanProcessor>> processors;
+  processors.emplace_back(null_processor);
+  processors.emplace_back(new FixedResultProcessor(true));
+  MultiSpanProcessor processor(std::move(processors));
+
+  auto recordable = processor.MakeRecordable();
+  ASSERT_NE(recordable, nullptr);
+
+  recordable->SetName("span");
+  recordable->SetAttribute("key", "value");
+  processor.OnStart(*recordable, SpanContext::GetInvalid());
+  processor.OnEnd(std::move(recordable));
+
+  EXPECT_EQ(null_processor->started_, 0);
+  EXPECT_EQ(null_processor->ended_, 0);
+}
+
 }  // namespace