diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml index 2b30445b41..0471e3e423 100644 --- a/.github/workflows/cppcheck.yml +++ b/.github/workflows/cppcheck.yml @@ -1,11 +1,10 @@ - name: cppcheck on: push: - branches: [ main ] + branches: [main] pull_request: - branches: [ main ] + branches: [main] permissions: contents: read @@ -66,12 +65,7 @@ jobs: set +e readonly WARNING_COUNT=`grep -c -E "\[.+\]" cppcheck.log` echo "cppcheck reported ${WARNING_COUNT} warning(s)" - # Acceptable limit, to decrease over time down to 0 - readonly WARNING_LIMIT=10 - # FAIL the build if WARNING_COUNT > WARNING_LIMIT - if [ $WARNING_COUNT -gt $WARNING_LIMIT ] ; then - exit 1 - # WARN in annotations if WARNING_COUNT > 0 - elif [ $WARNING_COUNT -gt 0 ] ; then + if [ $WARNING_COUNT -gt 0 ] ; then echo "::warning::cppcheck reported ${WARNING_COUNT} warning(s)" + exit 1 fi diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h index 77bd6bf89c..1dc4e6a905 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h @@ -323,6 +323,8 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient void SetMaxSessionsPerConnection(std::size_t max_requests_per_connection) noexcept override; + bool InternalCancelAllSessions() noexcept; + inline uint64_t GetMaxSessionsPerConnection() const noexcept { return max_sessions_per_connection_; diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index 816fc112a9..77105398d3 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -302,7 +302,7 @@ HttpClient::~HttpClient() } // Force to abort all sessions - CancelAllSessions(); + InternalCancelAllSessions(); if (!background_thread) { @@ -342,27 +342,7 @@ std::shared_ptr HttpClient::CreateSes bool HttpClient::CancelAllSessions() noexcept { - // CancelSession may change sessions_, we can not change a container while iterating it. - while (true) - { - std::unordered_map> sessions; - { - // We can only cleanup session and curl handles in the IO thread. - std::lock_guard lock_guard{sessions_m_}; - sessions = sessions_; - } - - if (sessions.empty()) - { - break; - } - - for (auto &session : sessions) - { - session.second->CancelSession(); - } - } - return true; + return InternalCancelAllSessions(); } bool HttpClient::FinishAllSessions() noexcept @@ -435,6 +415,31 @@ void HttpClient::CleanupSession(uint64_t session_id) } } +bool HttpClient::InternalCancelAllSessions() noexcept +{ + // CancelSession may change sessions_, we can not change a container while iterating it. + while (true) + { + std::unordered_map> sessions; + { + // We can only cleanup session and curl handles in the IO thread. + std::lock_guard lock_guard{sessions_m_}; + sessions = sessions_; + } + + if (sessions.empty()) + { + break; + } + + for (auto &session : sessions) + { + session.second->CancelSession(); + } + } + return true; +} + bool HttpClient::MaybeSpawnBackgroundThread() { std::lock_guard lock_guard{background_thread_m_}; diff --git a/sdk/include/opentelemetry/sdk/common/circular_buffer.h b/sdk/include/opentelemetry/sdk/common/circular_buffer.h index a35498ecea..943b250b1c 100644 --- a/sdk/include/opentelemetry/sdk/common/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/common/circular_buffer.h @@ -181,7 +181,7 @@ class CircularBuffer { return {}; } - auto data = data_.get(); + AtomicUniquePtr *data = data_.get(); if (tail_index < head_index) { return CircularBufferRange>{nostd::span>{ diff --git a/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h b/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h index 9f36b18fca..49398a1454 100644 --- a/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h @@ -93,10 +93,11 @@ class BatchLogRecordProcessor : public LogRecordProcessor /** * Shuts down the processor and does any cleanup required. Completely drains the buffer/queue of - * all its logs and passes them to the exporter. Any subsequent calls to - * ForceFlush or Shutdown will return immediately without doing anything. + * all its logs and passes them to the exporter. * - * NOTE: Timeout functionality not supported yet. + * @param timeout minimum amount of microseconds to wait for shutdown before giving up and + * returning failure. + * @return true if the shutdown succeeded, false otherwise */ bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; @@ -107,6 +108,13 @@ class BatchLogRecordProcessor : public LogRecordProcessor ~BatchLogRecordProcessor() override; protected: + /** + * Shuts down the processor and does any cleanup required. Completely drains the buffer/queue of + * all its logs and passes them to the exporter. + */ + bool InternalShutdown( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; + /** * The background routine performed by the worker thread. */ diff --git a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h index 46432ef22e..4db2b3497a 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h @@ -49,13 +49,32 @@ class MultiLogRecordProcessor : public LogRecordProcessor /** * Shuts down the processor and does any cleanup required. * ShutDown should only be called once for each processor. - * @param timeout minimum amount of microseconds to wait for - * shutdown before giving up and returning failure. + * @param timeout minimum amount of microseconds to wait for shutdown before giving up and + * returning failure. * @return true if the shutdown succeeded, false otherwise */ bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; +protected: + /** + * Exports all log records that have not yet been exported to the configured Exporter. + * @param timeout that the forceflush is required to finish within. + * @return a result code indicating whether it succeeded, failed or timed out + */ + bool InternalForceFlush( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; + + /** + * Shuts down the processor and does any cleanup required. + * ShutDown should only be called once for each processor. + * @param timeout minimum amount of microseconds to wait for shutdown before giving up and + * returning failure. + * @return true if the shutdown succeeded, false otherwise + */ + bool InternalShutdown( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; + private: std::vector> processors_; }; diff --git a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h index 3a78d6f33c..333c01df8a 100644 --- a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h @@ -91,10 +91,11 @@ class BatchSpanProcessor : public SpanProcessor /** * Shuts down the processor and does any cleanup required. Completely drains the buffer/queue of - * all its ended spans and passes them to the exporter. Any subsequent calls to OnStart, OnEnd, - * ForceFlush or Shutdown will return immediately without doing anything. + * all its ended spans and passes them to the exporter. * - * NOTE: Timeout functionality not supported yet. + * @param timeout minimum amount of microseconds to wait for shutdown before giving up and + * returning failure. + * @return true if the shutdown succeeded, false otherwise */ bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; @@ -108,6 +109,17 @@ class BatchSpanProcessor : public SpanProcessor ~BatchSpanProcessor() override; protected: + /** + * Shuts down the processor and does any cleanup required. Completely drains the buffer/queue of + * all its ended spans and passes them to the exporter. + * + * @param timeout minimum amount of microseconds to wait for shutdown before giving up and + * returning failure. + * @return true if the shutdown succeeded, false otherwise + */ + bool InternalShutdown( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; + /** * The background routine performed by the worker thread. */ diff --git a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h index 094c5b3e24..4f732df5a4 100644 --- a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h @@ -123,6 +123,19 @@ class MultiSpanProcessor : public SpanProcessor bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override + { + return InternalShutdown(timeout); + } + + ~MultiSpanProcessor() override + { + InternalShutdown(); + Cleanup(); + } + +protected: + bool InternalShutdown( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept { bool result = true; ProcessorNode *node = head_; @@ -135,12 +148,6 @@ class MultiSpanProcessor : public SpanProcessor return result; } - ~MultiSpanProcessor() override - { - Shutdown(); - Cleanup(); - } - private: struct ProcessorNode { diff --git a/sdk/include/opentelemetry/sdk/trace/simple_processor.h b/sdk/include/opentelemetry/sdk/trace/simple_processor.h index ce6d378b79..6a562a8f0a 100644 --- a/sdk/include/opentelemetry/sdk/trace/simple_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/simple_processor.h @@ -76,6 +76,15 @@ class SimpleSpanProcessor : public SpanProcessor bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override + { + return InternalShutdown(timeout); + } + + ~SimpleSpanProcessor() override { InternalShutdown(); } + +protected: + bool InternalShutdown( + std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept { // We only call shutdown ONCE. if (exporter_ != nullptr && !shutdown_latch_.test_and_set(std::memory_order_acquire)) @@ -85,8 +94,6 @@ class SimpleSpanProcessor : public SpanProcessor return true; } - ~SimpleSpanProcessor() override { Shutdown(); } - private: std::unique_ptr exporter_; opentelemetry::common::SpinLockMutex lock_; diff --git a/sdk/src/logs/batch_log_record_processor.cc b/sdk/src/logs/batch_log_record_processor.cc index 14c88eeed9..a656ef1f64 100644 --- a/sdk/src/logs/batch_log_record_processor.cc +++ b/sdk/src/logs/batch_log_record_processor.cc @@ -368,6 +368,19 @@ void BatchLogRecordProcessor::GetWaitAdjustedTime( } bool BatchLogRecordProcessor::Shutdown(std::chrono::microseconds timeout) noexcept +{ + return InternalShutdown(timeout); +} + +BatchLogRecordProcessor::~BatchLogRecordProcessor() +{ + if (synchronization_data_->is_shutdown.load() == false) + { + InternalShutdown(); + } +} + +bool BatchLogRecordProcessor::InternalShutdown(std::chrono::microseconds timeout) noexcept { auto start_time = std::chrono::system_clock::now(); @@ -391,14 +404,6 @@ bool BatchLogRecordProcessor::Shutdown(std::chrono::microseconds timeout) noexce return true; } -BatchLogRecordProcessor::~BatchLogRecordProcessor() -{ - if (synchronization_data_->is_shutdown.load() == false) - { - Shutdown(); - } -} - } // namespace logs } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/logs/multi_log_record_processor.cc b/sdk/src/logs/multi_log_record_processor.cc index 8fdbf08328..d7dc1127d3 100644 --- a/sdk/src/logs/multi_log_record_processor.cc +++ b/sdk/src/logs/multi_log_record_processor.cc @@ -30,8 +30,8 @@ MultiLogRecordProcessor::MultiLogRecordProcessor( MultiLogRecordProcessor::~MultiLogRecordProcessor() { - ForceFlush(); - Shutdown(); + InternalForceFlush(); + InternalShutdown(); } void MultiLogRecordProcessor::AddProcessor(std::unique_ptr &&processor) @@ -74,6 +74,16 @@ void MultiLogRecordProcessor::OnEmit(std::unique_ptr &&record) noexc } bool MultiLogRecordProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept +{ + return InternalForceFlush(timeout); +} + +bool MultiLogRecordProcessor::Shutdown(std::chrono::microseconds timeout) noexcept +{ + return InternalShutdown(timeout); +} + +bool MultiLogRecordProcessor::InternalForceFlush(std::chrono::microseconds timeout) noexcept { bool result = true; auto start_time = std::chrono::system_clock::now(); @@ -108,7 +118,7 @@ bool MultiLogRecordProcessor::ForceFlush(std::chrono::microseconds timeout) noex return result; } -bool MultiLogRecordProcessor::Shutdown(std::chrono::microseconds timeout) noexcept +bool MultiLogRecordProcessor::InternalShutdown(std::chrono::microseconds timeout) noexcept { bool result = true; auto start_time = std::chrono::system_clock::now(); diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index 5ec2376656..ad93eae7ae 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -358,6 +358,19 @@ void BatchSpanProcessor::GetWaitAdjustedTime( } bool BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept +{ + return InternalShutdown(timeout); +} + +BatchSpanProcessor::~BatchSpanProcessor() +{ + if (synchronization_data_->is_shutdown.load() == false) + { + InternalShutdown(); + } +} + +bool BatchSpanProcessor::InternalShutdown(std::chrono::microseconds timeout) noexcept { auto start_time = std::chrono::system_clock::now(); std::lock_guard shutdown_guard{synchronization_data_->shutdown_m}; @@ -380,14 +393,6 @@ bool BatchSpanProcessor::Shutdown(std::chrono::microseconds timeout) noexcept return true; } -BatchSpanProcessor::~BatchSpanProcessor() -{ - if (synchronization_data_->is_shutdown.load() == false) - { - Shutdown(); - } -} - } // namespace trace } // namespace sdk OPENTELEMETRY_END_NAMESPACE