[EXPORTER] General cleanup for is_shutdown_ flags in exporters. (#2663)

open-telemetry · May 8, 2024 · ce14bf6 · ce14bf6 · github-actions · May 8, 2024
1 parent 6de4ccd
commit ce14bf6
Show file tree

Hide file tree

Showing 19 changed files with 31 additions and 54 deletions.
diff --git a/...ters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h b/...ters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h
@@ -4,7 +4,6 @@
 #pragma once
 
 #include "nlohmann/json.hpp"
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/ext/http/client/http_client_factory.h"
 #include "opentelemetry/nostd/shared_ptr.h"
 #include "opentelemetry/sdk/logs/exporter.h"
@@ -110,14 +109,13 @@ class ElasticsearchLogRecordExporter final : public opentelemetry::sdk::logs::Lo
 
 private:
   // Stores if this exporter had its Shutdown() method called
-  bool is_shutdown_ = false;
+  std::atomic<bool> is_shutdown_{false};
 
   // Configuration options for the exporter
   ElasticsearchExporterOptions options_;
 
   // Object that stores the HTTP sessions that have been created
   std::shared_ptr<ext::http::client::HttpClient> http_client_;
-  mutable opentelemetry::common::SpinLockMutex lock_;
   bool isShutdown() const noexcept;
 
 #ifdef ENABLE_ASYNC_EXPORT

diff --git a/exporters/elasticsearch/src/es_log_record_exporter.cc b/exporters/elasticsearch/src/es_log_record_exporter.cc
@@ -460,7 +460,6 @@ bool ElasticsearchLogRecordExporter::ForceFlush(
 
 bool ElasticsearchLogRecordExporter::Shutdown(std::chrono::microseconds /* timeout */) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
 
   // Shutdown the session manager
@@ -472,7 +471,6 @@ bool ElasticsearchLogRecordExporter::Shutdown(std::chrono::microseconds /* timeo
 
 bool ElasticsearchLogRecordExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
 }  // namespace logs

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
@@ -2,12 +2,15 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #pragma once
+
+#include <atomic>
 #include <mutex>
-#include "opentelemetry/common/spin_lock_mutex.h"
+
 #include "opentelemetry/exporters/memory/in_memory_span_data.h"
 #include "opentelemetry/sdk/trace/exporter.h"
 #include "opentelemetry/sdk/trace/span_data.h"
 #include "opentelemetry/sdk_config.h"
+#include "opentelemetry/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace exporter
@@ -72,7 +75,6 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
    */
   bool Shutdown(std::chrono::microseconds /* timeout */) noexcept override
   {
-    const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
     is_shutdown_ = true;
     return true;
   }
@@ -84,13 +86,8 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
 
 private:
   std::shared_ptr<InMemorySpanData> data_;
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
-  bool isShutdown() const noexcept
-  {
-    const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
-    return is_shutdown_;
-  }
+  std::atomic<bool> is_shutdown_{false};
+  bool isShutdown() const noexcept { return is_shutdown_; }
 };
 }  // namespace memory
 }  // namespace exporter

diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/log_record_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/log_record_exporter.h
@@ -4,13 +4,12 @@
 #pragma once
 
 #include "opentelemetry/common/attribute_value.h"
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/nostd/span.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/sdk/logs/exporter.h"
-
 #include "opentelemetry/version.h"
 
+#include <atomic>
 #include <iostream>
 #include <sstream>
 #include <unordered_map>
@@ -59,8 +58,7 @@ class OStreamLogRecordExporter final : public opentelemetry::sdk::logs::LogRecor
   // The OStream to send the logs to
   std::ostream &sout_;
   // Whether this exporter has been shut down
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
+  std::atomic<bool> is_shutdown_{false};
   bool isShutdown() const noexcept;
   void printAttributes(
       const std::unordered_map<std::string, opentelemetry::sdk::common::OwnedAttributeValue> &map,

diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h
@@ -3,10 +3,11 @@
 
 #pragma once
 
+#include <atomic>
 #include <iostream>
+#include <mutex>
 #include <string>
 
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/sdk/metrics/data/metric_data.h"
 #include "opentelemetry/sdk/metrics/export/metric_producer.h"
 #include "opentelemetry/sdk/metrics/instruments.h"
@@ -72,8 +73,8 @@ class OStreamMetricExporter final : public opentelemetry::sdk::metrics::PushMetr
 
 private:
   std::ostream &sout_;
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
+  std::atomic<bool> is_shutdown_{false};
+  std::mutex serialize_lock_;
   sdk::metrics::AggregationTemporality aggregation_temporality_;
   bool isShutdown() const noexcept;
   void printInstrumentationInfoMetricData(const sdk::metrics::ScopeMetrics &info_metrics,

diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h
@@ -3,12 +3,12 @@
 
 #pragma once
 
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/nostd/span.h"
 #include "opentelemetry/sdk/trace/exporter.h"
 #include "opentelemetry/sdk/trace/span_data.h"
 #include "opentelemetry/version.h"
 
+#include <atomic>
 #include <iostream>
 #include <map>
 #include <sstream>
@@ -51,8 +51,7 @@ class OStreamSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
 
 private:
   std::ostream &sout_;
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
+  std::atomic<bool> is_shutdown_{false};
   bool isShutdown() const noexcept;
 
   // Mapping status number to the string from api/include/opentelemetry/trace/span_metadata.h

diff --git a/exporters/ostream/src/log_record_exporter.cc b/exporters/ostream/src/log_record_exporter.cc
@@ -126,14 +126,12 @@ bool OStreamLogRecordExporter::ForceFlush(std::chrono::microseconds /* timeout *
 
 bool OStreamLogRecordExporter::Shutdown(std::chrono::microseconds) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
   return true;
 }
 
 bool OStreamLogRecordExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
 

diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc
@@ -129,7 +129,7 @@ void OStreamMetricExporter::printInstrumentationInfoMetricData(
     const sdk::metrics::ResourceMetrics &data)
 {
   // sout_ is shared
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
+  const std::lock_guard<std::mutex> serialize(serialize_lock_);
   sout_ << "{";
   sout_ << "\n  scope name\t: " << info_metric.scope_->GetName()
         << "\n  schema url\t: " << info_metric.scope_->GetSchemaURL()
@@ -246,20 +246,19 @@ void OStreamMetricExporter::printPointAttributes(
 
 bool OStreamMetricExporter::ForceFlush(std::chrono::microseconds /* timeout */) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
+  const std::lock_guard<std::mutex> serialize(serialize_lock_);
+  sout_.flush();
   return true;
 }
 
 bool OStreamMetricExporter::Shutdown(std::chrono::microseconds /* timeout */) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
   return true;
 }
 
 bool OStreamMetricExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
 

diff --git a/exporters/ostream/src/span_exporter.cc b/exporters/ostream/src/span_exporter.cc
@@ -106,16 +106,15 @@ bool OStreamSpanExporter::ForceFlush(std::chrono::microseconds /* timeout */) no
 
 bool OStreamSpanExporter::Shutdown(std::chrono::microseconds /* timeout */) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
   return true;
 }
 
 bool OStreamSpanExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
+
 void OStreamSpanExporter::printAttributes(
     const std::unordered_map<std::string, sdkcommon::OwnedAttributeValue> &map,
     const std::string prefix)

diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_exporter.h
@@ -3,11 +3,11 @@
 
 #pragma once
 
+#include <atomic>
 #include <chrono>
 
 #include "opentelemetry/exporters/otlp/protobuf_include_prefix.h"
 
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h"
 
 #include "opentelemetry/exporters/otlp/protobuf_include_suffix.h"
@@ -92,8 +92,7 @@ class OtlpGrpcExporter final : public opentelemetry::sdk::trace::SpanExporter
    * @param stub the service stub to be used for exporting
    */
   OtlpGrpcExporter(std::unique_ptr<proto::collector::trace::v1::TraceService::StubInterface> stub);
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
+  std::atomic<bool> is_shutdown_{false};
   bool isShutdown() const noexcept;
 };
 }  // namespace otlp

diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_log_record_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_log_record_exporter.h
@@ -7,7 +7,6 @@
 
 #include "opentelemetry/exporters/otlp/protobuf_include_prefix.h"
 #include "opentelemetry/proto/collector/logs/v1/logs_service.grpc.pb.h"
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/exporters/otlp/protobuf_include_suffix.h"
 
 // clang-format on
@@ -16,6 +15,8 @@
 #include "opentelemetry/exporters/otlp/otlp_grpc_log_record_exporter_options.h"
 #include "opentelemetry/sdk/logs/exporter.h"
 
+#include <atomic>
+
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace exporter
 {
@@ -92,8 +93,7 @@ class OtlpGrpcLogRecordExporter : public opentelemetry::sdk::logs::LogRecordExpo
    */
   OtlpGrpcLogRecordExporter(
       std::unique_ptr<proto::collector::logs::v1::LogsService::StubInterface> stub);
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
+  std::atomic<bool> is_shutdown_{false};
   bool isShutdown() const noexcept;
 };
 

diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h
@@ -7,7 +7,6 @@
 
 #include "opentelemetry/exporters/otlp/protobuf_include_prefix.h"
 #include "opentelemetry/proto/collector/metrics/v1/metrics_service.grpc.pb.h"
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/exporters/otlp/protobuf_include_suffix.h"
 
 // clang-format on
@@ -16,6 +15,8 @@
 #include "opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h"
 #include "opentelemetry/sdk/metrics/push_metric_exporter.h"
 
+#include <atomic>
+
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace exporter
 {
@@ -82,8 +83,7 @@ class OtlpGrpcMetricExporter : public opentelemetry::sdk::metrics::PushMetricExp
    */
   OtlpGrpcMetricExporter(
       std::unique_ptr<proto::collector::metrics::v1::MetricsService::StubInterface> stub);
-  bool is_shutdown_ = false;
-  mutable opentelemetry::common::SpinLockMutex lock_;
+  std::atomic<bool> is_shutdown_{false};
   bool isShutdown() const noexcept;
 };
 }  // namespace otlp

diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h
@@ -3,7 +3,6 @@
 
 #pragma once
 
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/ext/http/client/http_client.h"
 #include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/exporter_utils.h"

diff --git a/exporters/otlp/src/otlp_grpc_exporter.cc b/exporters/otlp/src/otlp_grpc_exporter.cc
@@ -135,7 +135,6 @@ bool OtlpGrpcExporter::ForceFlush(
 bool OtlpGrpcExporter::Shutdown(
     OPENTELEMETRY_MAYBE_UNUSED std::chrono::microseconds timeout) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
 #ifdef ENABLE_ASYNC_EXPORT
   return client_->Shutdown(timeout);
@@ -146,7 +145,6 @@ bool OtlpGrpcExporter::Shutdown(
 
 bool OtlpGrpcExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
 

diff --git a/exporters/otlp/src/otlp_grpc_log_record_exporter.cc b/exporters/otlp/src/otlp_grpc_log_record_exporter.cc
@@ -138,7 +138,6 @@ opentelemetry::sdk::common::ExportResult OtlpGrpcLogRecordExporter::Export(
 bool OtlpGrpcLogRecordExporter::Shutdown(
     OPENTELEMETRY_MAYBE_UNUSED std::chrono::microseconds timeout) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
 #ifdef ENABLE_ASYNC_EXPORT
   return client_->Shutdown(timeout);
@@ -159,7 +158,6 @@ bool OtlpGrpcLogRecordExporter::ForceFlush(
 
 bool OtlpGrpcLogRecordExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
 

diff --git a/exporters/otlp/src/otlp_grpc_metric_exporter.cc b/exporters/otlp/src/otlp_grpc_metric_exporter.cc
@@ -141,7 +141,6 @@ bool OtlpGrpcMetricExporter::ForceFlush(
 bool OtlpGrpcMetricExporter::Shutdown(
     OPENTELEMETRY_MAYBE_UNUSED std::chrono::microseconds timeout) noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;
 #ifdef ENABLE_ASYNC_EXPORT
   return client_->Shutdown(timeout);
@@ -152,7 +151,6 @@ bool OtlpGrpcMetricExporter::Shutdown(
 
 bool OtlpGrpcMetricExporter::isShutdown() const noexcept
 {
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return is_shutdown_;
 }
 

diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h
@@ -8,7 +8,7 @@
 #include <vector>
 
 #include <prometheus/exposer.h>
-#include "opentelemetry/common/spin_lock_mutex.h"
+
 #include "opentelemetry/exporters/prometheus/collector.h"
 #include "opentelemetry/exporters/prometheus/exporter_options.h"
 #include "opentelemetry/nostd/span.h"

diff --git a/exporters/zipkin/include/opentelemetry/exporters/zipkin/zipkin_exporter.h b/exporters/zipkin/include/opentelemetry/exporters/zipkin/zipkin_exporter.h
@@ -3,7 +3,6 @@
 
 #pragma once
 
-#include "opentelemetry/common/spin_lock_mutex.h"
 #include "opentelemetry/exporters/zipkin/zipkin_exporter_options.h"
 #include "opentelemetry/ext/http/client/http_client_factory.h"
 #include "opentelemetry/ext/http/common/url_parser.h"
@@ -13,6 +12,8 @@
 
 #include "nlohmann/json.hpp"
 
+#include <atomic>
+
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace exporter
 {
@@ -69,7 +70,7 @@ class ZipkinExporter final : public opentelemetry::sdk::trace::SpanExporter
 
 private:
   // The configuration options associated with this exporter.
-  bool is_shutdown_ = false;
+  std::atomic<bool> is_shutdown_{false};
   ZipkinExporterOptions options_;
   std::shared_ptr<opentelemetry::ext::http::client::HttpClientSync> http_client_;
   opentelemetry::ext::http::common::UrlParser url_parser_;
@@ -84,7 +85,6 @@ class ZipkinExporter final : public opentelemetry::sdk::trace::SpanExporter
    */
   ZipkinExporter(std::shared_ptr<opentelemetry::ext::http::client::HttpClientSync> http_client);
 
-  mutable opentelemetry::common::SpinLockMutex lock_;
   bool isShutdown() const noexcept;
 };
 }  // namespace zipkin
Benchmark suite	Current: `ce14bf6`	Previous: `6de4ccd`	Ratio
`BM_SpinLockThrashing/4/process_time/real_time`	`4.080026149749756` ms/iter	`0.7228291034698486` ms/iter	`5.64`
`BM_ProcYieldSpinLockThrashing/1/process_time/real_time`	`10.663231213887533` ms/iter	`0.09473901206312088` ms/iter	`112.55`