diff --git a/docs/cpp-metrics-sdk-design.md b/docs/cpp-metrics-sdk-design.md index 586372fc2b..5ad4ad80aa 100644 --- 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 index 864a7d9c9b..0e7c8726e3 100644 --- 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 index 81e90c2c0d..bf1ce497e6 100644 --- 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 index 0139ddff65..8580c53b6b 100644 --- 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> &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 index c041de633a..31cd029ba5 100644 --- 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 index 693a3557ba..7da55b1643 100644 --- 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 index bf2ad4a5d4..e5a0fb8116 100644 --- 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 index c96461db74..3291a573cc 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -71,12 +71,14 @@ 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. @@ -84,9 +86,13 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor * 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 index e0cd4f96ba..65acddd807 100644 --- 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 index 1f445686d1..b410ee16af 100644 --- 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 index d7eb6f532a..f050c5f179 100644 --- 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 index ebd37f6218..060f4c7ebb 100644 --- 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 index 38b7a1e7f7..aec0d51cc6 100644 --- 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 index 2f7e49d454..6e4c7b81ab 100644 --- 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 processor_; diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index cf0a342081..5ab41bf633 100644 --- 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 &&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,7 +175,7 @@ void BatchSpanProcessor::DrainQueue() } } -void BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept +bool BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept { is_shutdown_.store(true); @@ -181,8 +183,10 @@ void BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept 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 index 157b1b0449..4040ea0226 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -35,9 +35,9 @@ std::shared_ptr 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 index 8e9e43c735..94c3dc99df 100644 --- 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 index 7b5db6e1eb..cdf544e79c 100644 --- 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 index dbf7f5a14f..6226a16d3c 100644 --- 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()); }