PR comments

sdk/include/opentelemetry/sdk/trace/span_data.h

@@ -73,35 +73,38 @@ class SpanData final : public Recordable
    */
   std::chrono::nanoseconds GetDuration() const noexcept { return duration_; }
 
-  void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept { trace_id_ = trace_id; }
+  void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept override
+  {
+    trace_id_ = trace_id;
+  }
 
-  void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept { span_id_ = span_id; }
+  void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept override { span_id_ = span_id; }
 
-  void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept
+  void SetParentSpanId(opentelemetry::trace::SpanId parent_span_id) noexcept override
   {
     parent_span_id_ = parent_span_id;
   }
 
-  void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept
+  void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override
   {
     (void)name;
     (void)timestamp;
   }
 
-  void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept
+  void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept override
   {
     status_code_ = code;
     status_desc_ = std::string(description);
   }
 
-  void SetName(nostd::string_view name) noexcept { name_ = std::string(name); }
+  void SetName(nostd::string_view name) noexcept override { name_ = std::string(name); }
 
-  void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept
+  void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept override
   {
     start_time_ = start_time;
   }
 
-  void SetDuration(std::chrono::nanoseconds duration) noexcept { duration_ = duration; }
+  void SetDuration(std::chrono::nanoseconds duration) noexcept override { duration_ = duration; }
 
 private:
   opentelemetry::trace::TraceId trace_id_;

sdk/src/trace/simple_processor.h

@@ -12,24 +12,27 @@ namespace trace
  * The simple span processor passes finished recordables to the configured
  * SpanExporter, as soon as they are finished.
  *
- * OnEnd, ForceFlush and Shutdown are no-ops.
+ * OnEnd and ForceFlush are no-ops.
  */
-class SimpleSpanProcessor
+class SimpleSpanProcessor : public SpanProcessor
 {
 public:
   /**
    * Initialize a simple span processor.
    * @param exporter the exporter used by the span processor
    */
-  SimpleSpanProcessor(std::unique_ptr<SpanExporter> &&exporter) noexcept
+  explicit SimpleSpanProcessor(std::unique_ptr<SpanExporter> &&exporter) noexcept
       : exporter_(std::move(exporter))
   {}
 
-  std::unique_ptr<Recordable> MakeRecordable() noexcept { return exporter_->MakeRecordable(); }
+  std::unique_ptr<Recordable> MakeRecordable() noexcept override
+  {
+    return exporter_->MakeRecordable();
+  }
 
-  void OnStart(Recordable &span) noexcept {}
+  void OnStart(Recordable &span) noexcept override {}
 
-  void OnEnd(std::unique_ptr<Recordable> &&span) noexcept
+  void OnEnd(std::unique_ptr<Recordable> &&span) noexcept override
   {
     std::shared_ptr<Recordable> recordable{std::move(span)};
     nostd::span<std::shared_ptr<Recordable>> s(&recordable, 1);
@@ -40,9 +43,11 @@ class SimpleSpanProcessor
     }
   }
 
-  void ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept {}
+  void ForceFlush(
+      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
+  {}
 
-  void Shutdown() noexcept {}
+  void Shutdown() noexcept override { exporter_->Shutdown(); }
 
 private:
   std::unique_ptr<SpanExporter> exporter_;

sdk/test/trace/simple_processor_test.cc

@@ -12,7 +12,10 @@ using namespace opentelemetry::sdk::trace;
 class MockSpanExporter final : public SpanExporter
 {
 public:
-  MockSpanExporter(std::shared_ptr<bool> span_received) noexcept : span_received_(span_received) {}
+  MockSpanExporter(std::shared_ptr<bool> span_received,
+                   std::shared_ptr<bool> shutdown_called) noexcept
+      : span_received_(span_received), shutdown_called_(shutdown_called)
+  {}
 
   std::unique_ptr<Recordable> MakeRecordable() noexcept
   {
@@ -31,16 +34,18 @@ class MockSpanExporter final : public SpanExporter
     return ExportResult::rSuccess;
   }
 
-  void Shutdown() noexcept {}
+  void Shutdown() noexcept { *shutdown_called_ = true; }
 
 private:
   std::shared_ptr<bool> span_received_;
+  std::shared_ptr<bool> shutdown_called_;
 };
 
 TEST(SimpleSpanProcessor, ToMockSpanExporter)
 {
   std::shared_ptr<bool> span_received(new bool(false));
-  std::unique_ptr<SpanExporter> exporter(new MockSpanExporter(span_received));
+  std::shared_ptr<bool> shutdown_called(new bool(false));
+  std::unique_ptr<SpanExporter> exporter(new MockSpanExporter(span_received, shutdown_called));
   SimpleSpanProcessor processor(std::move(exporter));
 
   auto recordable = processor.MakeRecordable();
@@ -52,4 +57,5 @@ TEST(SimpleSpanProcessor, ToMockSpanExporter)
   ASSERT_EQ(*span_received, true);
 
   processor.Shutdown();
+  ASSERT_EQ(*shutdown_called, true);
 }