Merge pull request #732 from open-telemetry/main

[CODE HEALTH] Fix clang-tidy performance warnings (open-telemetry#3941)

Author: malkia

.github/workflows/clang-tidy.yaml

@@ -17,9 +17,9 @@ jobs:
       matrix:
         include:
           - cmake_options: all-options-abiv1-preview
-            warning_limit: 220
+            warning_limit: 204
           - cmake_options: all-options-abiv2-preview
-            warning_limit: 222
+            warning_limit: 206
     env:
       CC: /usr/bin/clang-18
       CXX: /usr/bin/clang++-18

CHANGELOG.md

@@ -75,6 +75,9 @@ Increment the:
 * [TEST] CMake component install test for resource_detectors
   [#3940](https://github.com/open-telemetry/opentelemetry-cpp/pull/3940)
 
+* [CODE HEALTH] Fix clang-tidy performance warnings
+  [#3941](https://github.com/open-telemetry/opentelemetry-cpp/pull/3941)
+
 Important changes:
 
 * [BUILD] Revisit EventLogger deprecation

api/include/opentelemetry/context/context.h

@@ -35,7 +35,7 @@ class Context
   // 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, std::move(value))}}
   {}
 
   // Accepts a new iterable and then returns a new context that
@@ -59,7 +59,7 @@ class Context
   // exisiting list to the end of the new list.
   Context SetValue(nostd::string_view key, ContextValue value) noexcept
   {
-    Context context      = Context(key, value);
+    Context context      = Context(key, std::move(value));
     context.head_->next_ = head_;
     return context;
   }
@@ -125,13 +125,13 @@ class Context
 
     // Builds a data list with just a key and value, so it will just be the head
     // and returns that head.
-    DataList(nostd::string_view key, const ContextValue &value)
+    DataList(nostd::string_view key, ContextValue value)
     {
       key_        = new char[key.size()];
       key_length_ = key.size();
       std::memcpy(key_, key.data(), key.size() * sizeof(char));
       next_  = nostd::shared_ptr<DataList>{nullptr};
-      value_ = value;
+      value_ = std::move(value);
     }
 
     DataList(const DataList &other)

api/include/opentelemetry/trace/span_context_kv_iterable_view.h

@@ -24,15 +24,16 @@ namespace trace
 namespace detail
 {
 template <class T>
-inline void take_span_context_kv(SpanContext, opentelemetry::common::KeyValueIterableView<T>)
+inline void take_span_context_kv(const SpanContext &,
+                                 opentelemetry::common::KeyValueIterableView<T>)
 {}
 
 template <class T, nostd::enable_if_t<common::detail::is_key_value_iterable<T>::value> * = nullptr>
-inline void take_span_context_kv(SpanContext, T &)
+inline void take_span_context_kv(const SpanContext &, T &)
 {}
 
 inline void take_span_context_kv(
-    SpanContext,
+    const SpanContext &,
     std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>)
 {}
 
@@ -81,7 +82,7 @@ class SpanContextKeyValueIterableView final : public SpanContextKeyValueIterable
 private:
   const T *container_;
 
-  bool do_callback(SpanContext span_context,
+  bool do_callback(const SpanContext &span_context,
                    const common::KeyValueIterable &attributes,
                    nostd::function_ref<bool(SpanContext, const common::KeyValueIterable &)>
                        callback) const noexcept
@@ -95,7 +96,7 @@ class SpanContextKeyValueIterableView final : public SpanContextKeyValueIterable
 
   template <class U,
             nostd::enable_if_t<common::detail::is_key_value_iterable<U>::value> * = nullptr>
-  bool do_callback(SpanContext span_context,
+  bool do_callback(const SpanContext &span_context,
                    const U &attributes,
                    nostd::function_ref<bool(SpanContext, const common::KeyValueIterable &)>
                        callback) const noexcept
@@ -104,7 +105,7 @@ class SpanContextKeyValueIterableView final : public SpanContextKeyValueIterable
   }
 
   bool do_callback(
-      SpanContext span_context,
+      const SpanContext &span_context,
       std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>> attributes,
       nostd::function_ref<bool(SpanContext, const common::KeyValueIterable &)> callback)
       const noexcept

examples/environment_carrier/main.cc

@@ -107,8 +107,9 @@ void RunChild()
   opts.parent     = parent_ctx;
   auto child_span = tracer->StartSpan("child-operation", opts);
 
-  std::cout << "[child]  trace_id: " << Hex(child_span->GetContext().trace_id()) << std::endl;
-  std::cout << "[child]  span_id:  " << Hex(child_span->GetContext().span_id()) << std::endl;
+  std::cout << "[child]  trace_id: " << Hex(child_span->GetContext().trace_id()) << '\n';
+  std::cout << "[child]  span_id:  " << Hex(child_span->GetContext().span_id()) << '\n';
+  std::cout << std::flush;
 
   child_span->End();
   CleanupTracer();
@@ -124,8 +125,9 @@ void RunParent()
   auto root_span = tracer->StartSpan("parent-operation");
   auto scope     = trace_api::Scope(root_span);
 
-  std::cout << "[parent] trace_id: " << Hex(root_span->GetContext().trace_id()) << std::endl;
-  std::cout << "[parent] span_id:  " << Hex(root_span->GetContext().span_id()) << std::endl;
+  std::cout << "[parent] trace_id: " << Hex(root_span->GetContext().trace_id()) << '\n';
+  std::cout << "[parent] span_id:  " << Hex(root_span->GetContext().span_id()) << '\n';
+  std::cout << std::flush;
 
   // Inject context into a map via EnvironmentCarrier
   auto env_map = std::make_shared<std::map<std::string, std::string>>();
@@ -155,7 +157,7 @@ void RunParent()
   }
   else
   {
-    std::cerr << "fork() failed" << std::endl;
+    std::cerr << "fork() failed\n";
   }
 
   root_span->End();

exporters/otlp/test/otlp_grpc_metric_exporter_test.cc

@@ -72,15 +72,15 @@ class OtlpGrpcMetricExporterTestPeer : public ::testing::Test
       const std::shared_ptr<OtlpGrpcClient> &client)
   {
     return std::unique_ptr<sdk::metrics::PushMetricExporter>(
-        new OtlpGrpcMetricExporter(std::move(stub_interface), std::move(client)));
+        new OtlpGrpcMetricExporter(std::move(stub_interface), client));
   }
 
   std::unique_ptr<sdk::metrics::PushMetricExporter> GetExporter(
       const OtlpGrpcMetricExporterOptions &options,
       const std::shared_ptr<OtlpGrpcClient> &client)
   {
     return std::unique_ptr<sdk::metrics::PushMetricExporter>(
-        new OtlpGrpcMetricExporter(options, std::move(client)));
+        new OtlpGrpcMetricExporter(options, client));
   }
 
   // Get the options associated with the given exporter.

sdk/include/opentelemetry/sdk/instrumentationscope/scope_configurator.h

@@ -49,7 +49,7 @@ class ScopeConfigurator
     Builder &AddCondition(std::function<bool(const InstrumentationScope &)> scope_matcher,
                           T scope_config)
     {
-      conditions_.emplace_back(std::move(scope_matcher), scope_config);
+      conditions_.emplace_back(std::move(scope_matcher), std::move(scope_config));
       return *this;
     }
 
@@ -67,7 +67,7 @@ class ScopeConfigurator
           [scope_name = std::string(scope_name)](const InstrumentationScope &scope_info) {
             return scope_info.GetName() == scope_name;
           };
-      conditions_.emplace_back(name_equals_matcher, scope_config);
+      conditions_.emplace_back(std::move(name_equals_matcher), std::move(scope_config));
       return *this;
     }
 
@@ -112,8 +112,8 @@ class ScopeConfigurator
       std::function<bool(const InstrumentationScope &)> scope_matcher;
       T scope_config;
 
-      Condition(const std::function<bool(const InstrumentationScope &)> &matcher, const T &config)
-          : scope_matcher(matcher), scope_config(config)
+      Condition(std::function<bool(const InstrumentationScope &)> matcher, T config)
+          : scope_matcher(std::move(matcher)), scope_config(std::move(config))
       {}
     };