open-telemetry · lalitb · Dec 11, 2020 · Dec 3, 2020 · Dec 4, 2020 · Dec 4, 2020
diff --git a/docs/cpp-metrics-sdk-design.md b/docs/cpp-metrics-sdk-design.md
@@ -656,7 +656,7 @@ class StdoutExporter: public exporter {
     * Shuts down the channel and cleans up resources as required.
     *
     */
-    void Shutdown();
+    bool Shutdown();
 };
 ```
 

diff --git a/docs/cpp-ostream-exporter-design.md b/docs/cpp-ostream-exporter-design.md
@@ -111,9 +111,10 @@ public:
         return sdktrace::ExportResult::kSuccess;
     }
 
-    void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept
+    bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept
     {
         isShutdown = true;
+        return true;
     }
 
 };

diff --git a/exporters/memory/include/opentelemetry/exporters/memory/in_memory_span_exporter.h b/exporters/memory/include/opentelemetry/exporters/memory/in_memory_span_exporter.h
@@ -55,9 +55,13 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
   /**
    * @param timeout an optional value containing the timeout of the exporter
    * note: passing custom timeout values is not currently supported for this exporter
+   * @return Returns the status of the operation
    */
-  void Shutdown(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override{};
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
+  {
+    return true;
+  };
 
   /**
    * @return Returns a shared pointer to this exporters InMemorySpanData

diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h
@@ -36,7 +36,8 @@ class OStreamSpanExporter final : public sdktrace::SpanExporter
   sdktrace::ExportResult Export(
       const nostd::span<std::unique_ptr<sdktrace::Recordable>> &spans) noexcept override;
 
-  void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override;
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override;
 
 private:
   std::ostream &sout_;

diff --git a/exporters/ostream/src/span_exporter.cc b/exporters/ostream/src/span_exporter.cc
@@ -79,9 +79,10 @@ sdktrace::ExportResult OStreamSpanExporter::Export(
   return sdktrace::ExportResult::kSuccess;
 }
 
-void OStreamSpanExporter::Shutdown(std::chrono::microseconds timeout) noexcept
+bool OStreamSpanExporter::Shutdown(std::chrono::microseconds timeout) noexcept
 {
   isShutdown_ = true;
+  return true;
 }
 
 }  // namespace trace

diff --git a/exporters/ostream/test/ostream_span_test.cc b/exporters/ostream/test/ostream_span_test.cc
@@ -36,7 +36,7 @@ TEST(OStreamSpanExporter, Shutdown)
   // Redirect cout to our stringstream buffer
   std::cout.rdbuf(stdoutOutput.rdbuf());
 
-  processor->Shutdown();
+  EXPECT_TRUE(processor->Shutdown());
   processor->OnEnd(std::move(recordable));
 
   std::cout.rdbuf(sbuf);

diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_exporter.h
@@ -50,9 +50,13 @@ class OtlpExporter final : public opentelemetry::sdk::trace::SpanExporter
    * Shut down the exporter.
    * @param timeout an optional timeout, the default timeout of 0 means that no
    * timeout is applied.
+   * @return return the status of this operation
    */
-  void Shutdown(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override{};
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
+  {
+    return true;
+  }
 
 private:
   // The configuration options associated with this exporter.

diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h
@@ -71,22 +71,28 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor
   /*
    * For now, does nothing. In the future, it
    * may send all ended spans that have not yet been sent to the aggregator.
-   * @param timeout an optional timeout, the default timeout of 0 means that no
-   * timeout is applied. Currently, timeout does nothing.
+   * @param timeout an optional timeout. Currently, timeout does nothing.
+   * @return return the status of the operation.
    */
-  void ForceFlush(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
-  {}
+  bool ForceFlush(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
+  {
+    return true;
+  }
 
   /*
    * Shut down the processor and do any cleanup required, which is none.
    * After the call to Shutdown, subsequent calls to OnStart, OnEnd, ForceFlush
    * or Shutdown will return immediately without doing anything.
    * @param timeout an optional timeout, the default timeout of 0 means that no
    * timeout is applied. Currently, timeout does nothing.
+   * @return return the status of the operation.
    */
-  void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
-  {}
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
+  {
+    return true;
+  }
 
 private:
   mutable std::mutex mtx_;

diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc
@@ -489,8 +489,8 @@ TEST_F(TracezProcessor, FlushShutdown)
   auto pre_running_sz   = running.size();
   auto pre_completed_sz = completed.size();
 
-  processor->ForceFlush();
-  processor->Shutdown();
+  EXPECT_TRUE(processor->ForceFlush());
+  EXPECT_TRUE(processor->Shutdown());
 
   UpdateSpans(processor, completed, running);
 

diff --git a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h
@@ -67,8 +67,8 @@ class BatchSpanProcessor : public SpanProcessor
    *
    * NOTE: Timeout functionality not supported yet.
    */
-  void ForceFlush(
-      std::chrono::microseconds timeout = std::chrono::milliseconds(0)) noexcept override;
+  bool ForceFlush(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override;
 
   /**
    * Shuts down the processor and does any cleanup required. Completely drains the buffer/queue of
@@ -77,7 +77,8 @@ class BatchSpanProcessor : public SpanProcessor
    *
    * NOTE: Timeout functionality not supported yet.
    */
-  void Shutdown(std::chrono::microseconds timeout = std::chrono::milliseconds(0)) noexcept override;
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override;
 
   /**
    * Class destructor which invokes the Shutdown() method. The Shutdown() method is supposed to be

diff --git a/sdk/include/opentelemetry/sdk/trace/exporter.h b/sdk/include/opentelemetry/sdk/trace/exporter.h
@@ -55,11 +55,11 @@ class SpanExporter
 
   /**
    * Shut down the exporter.
-   * @param timeout an optional timeout, the default timeout of 0 means that no
-   * timeout is applied.
+   * @param timeout an optional timeout.
+   * @return return the status of the operation.
    */
-  virtual void Shutdown(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0;
+  virtual bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept = 0;
 };
 }  // namespace trace
 }  // namespace sdk

diff --git a/sdk/include/opentelemetry/sdk/trace/processor.h b/sdk/include/opentelemetry/sdk/trace/processor.h
@@ -46,8 +46,8 @@ class SpanProcessor
    * @param timeout an optional timeout, the default timeout of 0 means that no
    * timeout is applied.
    */
-  virtual void ForceFlush(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0;
+  virtual bool ForceFlush(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept = 0;
 
   /**
    * Shut down the processor and do any cleanup required. Ended spans are
@@ -57,8 +57,8 @@ class SpanProcessor
    * @param timeout an optional timeout, the default timeout of 0 means that no
    * timeout is applied.
    */
-  virtual void Shutdown(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept = 0;
+  virtual bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept = 0;
 };
 }  // namespace trace
 }  // namespace sdk

diff --git a/sdk/include/opentelemetry/sdk/trace/simple_processor.h b/sdk/include/opentelemetry/sdk/trace/simple_processor.h
@@ -50,17 +50,21 @@ class SimpleSpanProcessor : public SpanProcessor
     }
   }
 
-  void ForceFlush(
-      std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
-  {}
+  bool ForceFlush(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
+  {
+    return true;
+  }
 
-  void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
   {
     // We only call shutdown ONCE.
     if (exporter_ != nullptr && !shutdown_latch_.test_and_set(std::memory_order_acquire))
     {
-      exporter_->Shutdown(timeout);
+      return exporter_->Shutdown(timeout);
     }
+    return true;
   }
 
   ~SimpleSpanProcessor() { Shutdown(); }

diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h
@@ -55,7 +55,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
   /**
    * Shutdown the span processor associated with this tracer provider.
    */
-  void Shutdown() noexcept;
+  bool Shutdown() noexcept;
 
 private:
   opentelemetry::sdk::AtomicSharedPtr<SpanProcessor> processor_;

diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc
@@ -53,11 +53,11 @@ void BatchSpanProcessor::OnEnd(std::unique_ptr<Recordable> &&span) noexcept
   }
 }
 
-void BatchSpanProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept
+bool BatchSpanProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept
 {
   if (is_shutdown_.load() == true)
   {
-    return;
+    return false;
   }
 
   is_force_flush_ = true;
@@ -77,6 +77,8 @@ void BatchSpanProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept
 
   // Notify the worker thread
   is_force_flush_notified_ = false;
+
+  return true;
 }
 
 void BatchSpanProcessor::DoBackgroundWork()
@@ -173,16 +175,18 @@ void BatchSpanProcessor::DrainQueue()
   }
 }
 
-void BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept
+bool BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept
 {
   is_shutdown_.store(true);
 
   cv_.notify_one();
   worker_thread_.join();
   if (exporter_ != nullptr)
   {
-    exporter_->Shutdown();
+    return exporter_->Shutdown();
   }
+
+  return true;
 }
 
 BatchSpanProcessor::~BatchSpanProcessor()

diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc
@@ -35,9 +35,9 @@ std::shared_ptr<Sampler> TracerProvider::GetSampler() const noexcept
   return sampler_;
 }
 
-void TracerProvider::Shutdown() noexcept
+bool TracerProvider::Shutdown() noexcept
 {
-  GetProcessor()->Shutdown();
+  return GetProcessor()->Shutdown();
 }
 }  // namespace trace
 }  // namespace sdk

diff --git a/sdk/test/trace/batch_span_processor_test.cc b/sdk/test/trace/batch_span_processor_test.cc
@@ -52,9 +52,11 @@ class MockSpanExporter final : public sdk::trace::SpanExporter
     return sdk::trace::ExportResult::kSuccess;
   }
 
-  void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
   {
     *is_shutdown_ = true;
+    return true;
   }
 
   bool IsExportCompleted() { return is_export_completed_->load(); }
@@ -135,7 +137,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestShutdown)
     batch_processor->OnEnd(std::move(test_spans->at(i)));
   }
 
-  batch_processor->Shutdown();
+  EXPECT_TRUE(batch_processor->Shutdown());
 
   EXPECT_EQ(num_spans, spans_received->size());
   for (int i = 0; i < num_spans; ++i)
@@ -165,7 +167,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestForceFlush)
   // Give some time to export
   std::this_thread::sleep_for(std::chrono::milliseconds(50));
 
-  batch_processor->ForceFlush();
+  EXPECT_TRUE(batch_processor->ForceFlush());
 
   EXPECT_EQ(num_spans, spans_received->size());
   for (int i = 0; i < num_spans; ++i)
@@ -183,7 +185,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestForceFlush)
   // Give some time to export the spans
   std::this_thread::sleep_for(std::chrono::milliseconds(50));
 
-  batch_processor->ForceFlush();
+  EXPECT_TRUE(batch_processor->ForceFlush());
 
   EXPECT_EQ(num_spans * 2, spans_received->size());
   for (int i = 0; i < num_spans; ++i)
@@ -215,7 +217,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestManySpansLoss)
   // Give some time to export the spans
   std::this_thread::sleep_for(std::chrono::milliseconds(700));
 
-  batch_processor->ForceFlush();
+  EXPECT_TRUE(batch_processor->ForceFlush());
 
   // Span should be exported by now
   EXPECT_GE(max_queue_size, spans_received->size());
@@ -243,7 +245,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestManySpansLossLess)
   // Give some time to export the spans
   std::this_thread::sleep_for(std::chrono::milliseconds(50));
 
-  batch_processor->ForceFlush();
+  EXPECT_TRUE(batch_processor->ForceFlush());
 
   EXPECT_EQ(num_spans, spans_received->size());
   for (int i = 0; i < num_spans; ++i)

diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc
@@ -26,7 +26,7 @@ TEST(SimpleProcessor, ToInMemorySpanExporter)
 
   ASSERT_EQ(1, span_data->GetSpans().size());
 
-  processor.Shutdown();
+  EXPECT_TRUE(processor.Shutdown());
 }
 
 // An exporter that does nothing but record (and give back ) the # of times Shutdown was called.
@@ -46,9 +46,11 @@ class RecordShutdownExporter final : public SpanExporter
     return ExportResult::kSuccess;
   }
 
-  void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override
+  bool Shutdown(
+      std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override
   {
     *shutdown_counter_ += 1;
+    return true;
   }
 
 private:

diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc
@@ -66,8 +66,5 @@ TEST(TracerProvider, Shutdown)
 
   TracerProvider tp1(processor1);
 
-  tp1.Shutdown();
-
-  // Verify Shutdown returns.
-  ASSERT_TRUE(true);
+  EXPECT_TRUE(tp1.Shutdown());
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -656,7 +656,7 @@ class StdoutExporter: public exporter { @@
         * Shuts down the channel and cleans up resources as required.
         *
         */
-        void Shutdown();
+        bool Shutdown();
     };
     ```
@@ Expand Down @@