First pass

chusitoo · chusitoo · commit 9375819625a6 · 2024-11-17T17:01:47.000-05:00
diff --git a/api/include/opentelemetry/baggage/baggage_context.h b/api/include/opentelemetry/baggage/baggage_context.h
@@ -27,7 +27,7 @@ inline nostd::shared_ptr<Baggage> GetBaggage(const context::Context &context) no
 }
 
 inline context::Context SetBaggage(context::Context &context,
-                                   nostd::shared_ptr<Baggage> baggage) noexcept
+                                   const nostd::shared_ptr<Baggage>& baggage) noexcept
 {
   return context.SetValue(kBaggageHeader, baggage);
 }
diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h
@@ -29,16 +29,14 @@ class Context
   // hold a shared_ptr to the head of the DataList linked list
   template <class T>
   Context(const T &keys_and_values) noexcept
-  {
-    head_ = nostd::shared_ptr<DataList>{new DataList(keys_and_values)};
-  }
+      : head_{nostd::shared_ptr<DataList>{new DataList(keys_and_values)}}
+  {}
 
   // Creates a context object from a key and value, this will
   // hold a shared_ptr to the head of the DataList linked list
   Context(nostd::string_view key, ContextValue value) noexcept
-  {
-    head_ = nostd::shared_ptr<DataList>{new DataList(key, value)};
-  }
+      : head_{nostd::shared_ptr<DataList>{new DataList(key, value)}}
+  {}
 
   // Accepts a new iterable and then returns a new context that
   // contains the new key and value data. It attaches the
@@ -92,22 +90,21 @@ class Context
 
 private:
   // A linked list to contain the keys and values of this context node
-  class DataList
+  struct DataList
   {
-  public:
-    char *key_;
+    char *key_ = nullptr;
 
-    nostd::shared_ptr<DataList> next_;
+    size_t key_length_ = 0UL;
 
-    size_t key_length_;
+    nostd::shared_ptr<DataList> next_;
 
     ContextValue value_;
 
-    DataList() { next_ = nullptr; }
+    DataList() : next_{nullptr} {}
 
     // Builds a data list off of a key and value iterable and returns the head
     template <class T>
-    DataList(const T &keys_and_vals) : key_{nullptr}, next_(nostd::shared_ptr<DataList>{nullptr})
+    DataList(const T &keys_and_vals) : next_(nostd::shared_ptr<DataList>{nullptr})
     {
       bool first = true;
       auto *node = this;
@@ -133,8 +130,14 @@ class Context
       key_        = new char[key.size()];
       key_length_ = key.size();
       memcpy(key_, key.data(), key.size() * sizeof(char));
-      value_ = value;
       next_  = nostd::shared_ptr<DataList>{nullptr};
+      value_ = value;
+    }
+
+    DataList(const DataList &other)
+        : key_length_(other.key_length_), next_(other.next_), value_(other.value_)
+    {
+      memcpy(key_, other.key_, other.key_length_ * sizeof(char));
     }
 
     DataList &operator=(DataList &&other) noexcept
diff --git a/api/include/opentelemetry/context/propagation/global_propagator.h b/api/include/opentelemetry/context/propagation/global_propagator.h
@@ -32,7 +32,7 @@ class OPENTELEMETRY_EXPORT GlobalTextMapPropagator
     return nostd::shared_ptr<TextMapPropagator>(GetPropagator());
   }
 
-  static void SetGlobalPropagator(nostd::shared_ptr<TextMapPropagator> prop) noexcept
+  static void SetGlobalPropagator(const nostd::shared_ptr<TextMapPropagator>& prop) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetPropagator() = prop;
diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h
@@ -152,7 +152,7 @@ class OPENTELEMETRY_EXPORT RuntimeContext
    *
    * @param storage a custom runtime context storage
    */
-  static void SetRuntimeContextStorage(nostd::shared_ptr<RuntimeContextStorage> storage) noexcept
+  static void SetRuntimeContextStorage(const nostd::shared_ptr<RuntimeContextStorage>& storage) noexcept
   {
     GetStorage() = storage;
   }
diff --git a/api/include/opentelemetry/logs/event_id.h b/api/include/opentelemetry/logs/event_id.h
@@ -19,9 +19,9 @@ namespace logs
 class EventId
 {
 public:
-  EventId(int64_t id, nostd::string_view name) noexcept : id_{id}
+  EventId(int64_t id, nostd::string_view name) noexcept
+      : id_{id}, name_{nostd::unique_ptr<char[]>{new char[name.length() + 1]}}
   {
-    name_ = nostd::unique_ptr<char[]>{new char[name.length() + 1]};
     std::copy(name.begin(), name.end(), name_.get());
     name_.get()[name.length()] = 0;
   }
diff --git a/api/include/opentelemetry/logs/provider.h b/api/include/opentelemetry/logs/provider.h
@@ -39,7 +39,7 @@ class OPENTELEMETRY_EXPORT Provider
   /**
    * Changes the singleton LoggerProvider.
    */
-  static void SetLoggerProvider(nostd::shared_ptr<LoggerProvider> tp) noexcept
+  static void SetLoggerProvider(const nostd::shared_ptr<LoggerProvider>& tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetProvider() = tp;
@@ -60,7 +60,7 @@ class OPENTELEMETRY_EXPORT Provider
   /**
    * Changes the singleton EventLoggerProvider.
    */
-  static void SetEventLoggerProvider(nostd::shared_ptr<EventLoggerProvider> tp) noexcept
+  static void SetEventLoggerProvider(const nostd::shared_ptr<EventLoggerProvider>& tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetEventProvider() = tp;
diff --git a/api/include/opentelemetry/trace/context.h b/api/include/opentelemetry/trace/context.h
@@ -34,7 +34,8 @@ inline bool IsRootSpan(const context::Context &context) noexcept
 }
 
 // Set Span into explicit context
-inline context::Context SetSpan(context::Context &context, nostd::shared_ptr<Span> span) noexcept
+inline context::Context SetSpan(context::Context &context,
+                                const nostd::shared_ptr<Span> &span) noexcept
 {
   return context.SetValue(kSpanKey, span);
 }
diff --git a/api/include/opentelemetry/trace/default_span.h b/api/include/opentelemetry/trace/default_span.h
@@ -63,7 +63,7 @@ class DefaultSpan : public Span
 
   nostd::string_view ToString() const noexcept { return "DefaultSpan"; }
 
-  DefaultSpan(SpanContext span_context) noexcept : span_context_(span_context) {}
+  DefaultSpan(const SpanContext& span_context) noexcept : span_context_(span_context) {}
 
   // movable and copiable
   DefaultSpan(DefaultSpan &&spn) noexcept : Span(), span_context_(spn.GetContext()) {}
diff --git a/api/include/opentelemetry/trace/provider.h b/api/include/opentelemetry/trace/provider.h
@@ -36,7 +36,7 @@ class OPENTELEMETRY_EXPORT Provider
   /**
    * Changes the singleton TracerProvider.
    */
-  static void SetTracerProvider(nostd::shared_ptr<TracerProvider> tp) noexcept
+  static void SetTracerProvider(const nostd::shared_ptr<TracerProvider>& tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetProvider() = tp;
diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h
@@ -41,7 +41,7 @@ class SpanContext final
               SpanId span_id,
               TraceFlags trace_flags,
               bool is_remote,
-              nostd::shared_ptr<TraceState> trace_state = TraceState::GetDefault()) noexcept
+              const nostd::shared_ptr<TraceState>& trace_state = TraceState::GetDefault()) noexcept
       : trace_id_(trace_id),
         span_id_(span_id),
         trace_flags_(trace_flags),
@@ -64,7 +64,7 @@ class SpanContext final
   const trace::SpanId &span_id() const noexcept { return span_id_; }
 
   // @returns the trace_state associated with this span_context
-  const nostd::shared_ptr<trace::TraceState> trace_state() const noexcept { return trace_state_; }
+  const nostd::shared_ptr<trace::TraceState>& trace_state() const noexcept { return trace_state_; }
 
   /*
    * @param that SpanContext for comparing.
diff --git a/examples/grpc/tracer_common.h b/examples/grpc/tracer_common.h
@@ -44,7 +44,7 @@ class GrpcClientCarrier : public opentelemetry::context::propagation::TextMapCar
     context_->AddMetadata(std::string(key), std::string(value));
   }
 
-  ClientContext *context_;
+  ClientContext *context_ = nullptr;
 };
 
 class GrpcServerCarrier : public opentelemetry::context::propagation::TextMapCarrier
@@ -69,7 +69,7 @@ class GrpcServerCarrier : public opentelemetry::context::propagation::TextMapCar
     // Not required for server
   }
 
-  ServerContext *context_;
+  ServerContext *context_ = nullptr;
 };
 
 void InitTracer()
diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc
@@ -968,7 +968,6 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender
   explicit OtlpFileSystemBackend(const OtlpFileClientFileSystemOptions &options)
       : options_(options), is_initialized_{false}
   {
-    file_ = std::make_shared<FileStats>();
     file_->is_shutdown.store(false);
     file_->rotate_index            = 0;
     file_->written_size            = 0;
@@ -1536,9 +1535,9 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender
     std::mutex background_thread_waiter_lock;
     std::condition_variable background_thread_waiter_cv;
   };
-  std::shared_ptr<FileStats> file_;
+  std::shared_ptr<FileStats> file_ = std::make_shared<FileStats>();
 
-  std::atomic<bool> is_initialized_;
+  std::atomic<bool> is_initialized_{false};
   std::time_t check_file_path_interval_{0};
 };
 
diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc
@@ -753,13 +753,7 @@ sdk::common::ExportResult OtlpHttpClient::Export(
   auto session = createSession(message, std::move(result_callback));
   if (opentelemetry::nostd::holds_alternative<sdk::common::ExportResult>(session))
   {
-    sdk::common::ExportResult result =
-        opentelemetry::nostd::get<sdk::common::ExportResult>(session);
-    if (result_callback)
-    {
-      result_callback(result);
-    }
-    return result;
+    return opentelemetry::nostd::get<sdk::common::ExportResult>(session);
   }
 
   addSession(std::move(opentelemetry::nostd::get<HttpSessionData>(session)));
@@ -906,7 +900,9 @@ OtlpHttpClient::createSession(
       }
       OTEL_INTERNAL_LOG_ERROR(error_message.c_str());
 
-      return opentelemetry::sdk::common::ExportResult::kFailure;
+      const auto result = opentelemetry::sdk::common::ExportResult::kFailure;
+      result_callback(result);
+      return result;
     }
 
     if (!parse_url.path_.empty() && parse_url.path_[0] == '/')
@@ -938,7 +934,10 @@ OtlpHttpClient::createSession(
         OTEL_INTERNAL_LOG_DEBUG("[OTLP HTTP Client] Serialize body failed(Binary):"
                                 << message.InitializationErrorString());
       }
-      return opentelemetry::sdk::common::ExportResult::kFailure;
+
+      const auto result = opentelemetry::sdk::common::ExportResult::kFailure;
+      result_callback(result);
+      return result;
     }
     content_type = kHttpBinaryContentType;
   }
@@ -971,7 +970,9 @@ OtlpHttpClient::createSession(
     }
     OTEL_INTERNAL_LOG_ERROR(error_message);
 
-    return opentelemetry::sdk::common::ExportResult::kFailure;
+    const auto result = opentelemetry::sdk::common::ExportResult::kFailure;
+    result_callback(result);
+    return result;
   }
 
   auto session = http_client_->CreateSession(options_.url);
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
@@ -169,13 +169,11 @@ class Session : public opentelemetry::ext::http::client::Session,
 {
 public:
   Session(HttpClient &http_client,
-          std::string scheme      = "http",
-          const std::string &host = "",
-          uint16_t port           = 80)
-      : http_client_(http_client)
-  {
-    host_ = scheme + "://" + host + ":" + std::to_string(port) + "/";
-  }
+          const std::string &scheme = "http",
+          const std::string &host   = "",
+          uint16_t port             = 80)
+      : http_client_(http_client), host_{scheme + "://" + host + ":" + std::to_string(port) + "/"}
+  {}
 
   std::shared_ptr<opentelemetry::ext::http::client::Request> CreateRequest() noexcept override
   {
@@ -225,7 +223,7 @@ class Session : public opentelemetry::ext::http::client::Session,
   std::shared_ptr<Request> http_request_;
   std::string host_;
   std::unique_ptr<HttpOperation> curl_operation_;
-  uint64_t session_id_;
+  uint64_t session_id_ = 0UL;
   HttpClient &http_client_;
   std::atomic<bool> is_session_active_{false};
 };
diff --git a/opentracing-shim/include/opentelemetry/opentracingshim/span_context_shim.h b/opentracing-shim/include/opentelemetry/opentracingshim/span_context_shim.h
@@ -24,7 +24,7 @@ class SpanContextShim final : public opentracing::SpanContext
   {}
 
   inline const opentelemetry::trace::SpanContext &context() const { return context_; }
-  inline const BaggagePtr baggage() const { return baggage_; }
+  inline const BaggagePtr& baggage() const { return baggage_; }
   SpanContextShim newWithKeyValue(nostd::string_view key, nostd::string_view value) const noexcept;
   bool BaggageItem(nostd::string_view key, std::string &value) const noexcept;
 
diff --git a/opentracing-shim/src/shim_utils.cc b/opentracing-shim/src/shim_utils.cc
@@ -71,11 +71,7 @@ nostd::shared_ptr<opentelemetry::baggage::Baggage> makeBaggage(
     entry.second->ForeachBaggageItem(
         [&baggage_items](const std::string &key, const std::string &value) {
           // It is unspecified which Baggage value is used in the case of repeated keys.
-          if (baggage_items.find(key) == baggage_items.end())
-          {
-            baggage_items.emplace(key, value);  // Here, only insert if key not already present
-          }
-          return true;
+          return baggage_items.emplace(key, value).second;
         });
   }
   // If no such list of references is specified, the current Baggage
diff --git a/sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h b/sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h
@@ -54,8 +54,7 @@ class AtomicUniquePtr
                                                      std::memory_order_relaxed);
     if (was_successful)
     {
-      owner.release();
-      return true;
+      return owner.release() != nullptr;
     }
     return false;
   }
diff --git a/sdk/include/opentelemetry/sdk/common/global_log_handler.h b/sdk/include/opentelemetry/sdk/common/global_log_handler.h
@@ -109,7 +109,7 @@ class GlobalLogHandler
    * This should be called once at the start of application before creating any Provider
    * instance.
    */
-  static inline void SetLogHandler(nostd::shared_ptr<LogHandler> eh) noexcept
+  static inline void SetLogHandler(const nostd::shared_ptr<LogHandler>& eh) noexcept
   {
     GetHandlerAndLevel().first = eh;
   }
diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h
@@ -113,9 +113,10 @@ class DefaultAggregation
     }
   }
 
-  static std::unique_ptr<Aggregation> CloneAggregation(AggregationType aggregation_type,
-                                                       InstrumentDescriptor instrument_descriptor,
-                                                       const Aggregation &to_copy)
+  static std::unique_ptr<Aggregation> CloneAggregation(
+      AggregationType aggregation_type,
+      const InstrumentDescriptor &instrument_descriptor,
+      const Aggregation &to_copy)
   {
     const PointType point_data = to_copy.ToPoint();
     bool is_monotonic          = true;
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h
@@ -90,7 +90,7 @@ class NoopMetricStorage : public MetricStorage
                opentelemetry::common::SystemTimestamp /* collection_ts */,
                nostd::function_ref<bool(MetricData)> callback) noexcept override
   {
-    MetricData metric_data;
+    MetricData metric_data{};
     return callback(std::move(metric_data));
   }
 };
diff --git a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h
@@ -65,11 +65,15 @@ class DefaultAttributesProcessor : public AttributesProcessor
 class FilteringAttributesProcessor : public AttributesProcessor
 {
 public:
-  FilteringAttributesProcessor(
-      const std::unordered_map<std::string, bool> allowed_attribute_keys = {})
+  FilteringAttributesProcessor(std::unordered_map<std::string, bool> &&allowed_attribute_keys = {})
       : allowed_attribute_keys_(std::move(allowed_attribute_keys))
   {}
 
+  FilteringAttributesProcessor(
+      const std::unordered_map<std::string, bool> &allowed_attribute_keys = {})
+      : allowed_attribute_keys_(allowed_attribute_keys)
+  {}
+
   MetricAttributes process(
       const opentelemetry::common::KeyValueIterable &attributes) const noexcept override
   {

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ inline nostd::shared_ptr<Baggage> GetBaggage(const context::Context &context) no`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`inline context::Context SetBaggage(context::Context &context,`
`30`		`- nostd::shared_ptr<Baggage> baggage) noexcept`
	`30`	`+ const nostd::shared_ptr<Baggage>& baggage) noexcept`
`31`	`31`	`{`
`32`	`32`	`return context.SetValue(kBaggageHeader, baggage);`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ class OPENTELEMETRY_EXPORT GlobalTextMapPropagator`
`32`	`32`	`return nostd::shared_ptr<TextMapPropagator>(GetPropagator());`
`33`	`33`	`}`
`34`	`34`
`35`		`- static void SetGlobalPropagator(nostd::shared_ptr<TextMapPropagator> prop) noexcept`
	`35`	`+ static void SetGlobalPropagator(const nostd::shared_ptr<TextMapPropagator>& prop) noexcept`
`36`	`36`	`{`
`37`	`37`	`std::lock_guard<common::SpinLockMutex> guard(GetLock());`
`38`	`38`	`GetPropagator() = prop;`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ class OPENTELEMETRY_EXPORT RuntimeContext`
`152`	`152`	`*`
`153`	`153`	`* @param storage a custom runtime context storage`
`154`	`154`	`*/`
`155`		`- static void SetRuntimeContextStorage(nostd::shared_ptr<RuntimeContextStorage> storage) noexcept`
	`155`	`+ static void SetRuntimeContextStorage(const nostd::shared_ptr<RuntimeContextStorage>& storage) noexcept`
`156`	`156`	`{`
`157`	`157`	`GetStorage() = storage;`
`158`	`158`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,9 +19,9 @@ namespace logs`
`19`	`19`	`class EventId`
`20`	`20`	`{`
`21`	`21`	`public:`
`22`		`- EventId(int64_t id, nostd::string_view name) noexcept : id_{id}`
	`22`	`+ EventId(int64_t id, nostd::string_view name) noexcept`
	`23`	`+ : id_{id}, name_{nostd::unique_ptr<char[]>{new char[name.length() + 1]}}`
`23`	`24`	`{`
`24`		`- name_ = nostd::unique_ptr<char[]>{new char[name.length() + 1]};`
`25`	`25`	`std::copy(name.begin(), name.end(), name_.get());`
`26`	`26`	`name_.get()[name.length()] = 0;`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,8 @@ inline bool IsRootSpan(const context::Context &context) noexcept`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`// Set Span into explicit context`
`37`		`-inline context::Context SetSpan(context::Context &context, nostd::shared_ptr<Span> span) noexcept`
	`37`	`+inline context::Context SetSpan(context::Context &context,`
	`38`	`+ const nostd::shared_ptr<Span> &span) noexcept`
`38`	`39`	`{`
`39`	`40`	`return context.SetValue(kSpanKey, span);`
`40`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ class OPENTELEMETRY_EXPORT Provider`
`36`	`36`	`/**`
`37`	`37`	`* Changes the singleton TracerProvider.`
`38`	`38`	`*/`
`39`		`- static void SetTracerProvider(nostd::shared_ptr<TracerProvider> tp) noexcept`
	`39`	`+ static void SetTracerProvider(const nostd::shared_ptr<TracerProvider>& tp) noexcept`
`40`	`40`	`{`
`41`	`41`	`std::lock_guard<common::SpinLockMutex> guard(GetLock());`
`42`	`42`	`GetProvider() = tp;`