From 9375819625a6837b8e6795cdbe84f5ff066e7233 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 17 Nov 2024 17:01:47 -0500 Subject: [PATCH 01/14] First pass --- .../opentelemetry/baggage/baggage_context.h | 2 +- api/include/opentelemetry/context/context.h | 31 ++++++++++--------- .../context/propagation/global_propagator.h | 2 +- .../opentelemetry/context/runtime_context.h | 2 +- api/include/opentelemetry/logs/event_id.h | 4 +-- api/include/opentelemetry/logs/provider.h | 4 +-- api/include/opentelemetry/trace/context.h | 3 +- .../opentelemetry/trace/default_span.h | 2 +- api/include/opentelemetry/trace/provider.h | 2 +- .../opentelemetry/trace/span_context.h | 4 +-- examples/grpc/tracer_common.h | 4 +-- exporters/otlp/src/otlp_file_client.cc | 5 ++- exporters/otlp/src/otlp_http_client.cc | 21 +++++++------ .../ext/http/client/curl/http_client_curl.h | 14 ++++----- .../opentracingshim/span_context_shim.h | 2 +- opentracing-shim/src/shim_utils.cc | 6 +--- .../sdk/common/atomic_unique_ptr.h | 3 +- .../sdk/common/global_log_handler.h | 2 +- .../metrics/aggregation/default_aggregation.h | 7 +++-- .../sdk/metrics/state/metric_storage.h | 2 +- .../sdk/metrics/view/attributes_processor.h | 8 +++-- 21 files changed, 66 insertions(+), 64 deletions(-) diff --git a/api/include/opentelemetry/baggage/baggage_context.h b/api/include/opentelemetry/baggage/baggage_context.h index e5b9556d3f..7755880426 100644 --- a/api/include/opentelemetry/baggage/baggage_context.h +++ b/api/include/opentelemetry/baggage/baggage_context.h @@ -27,7 +27,7 @@ inline nostd::shared_ptr GetBaggage(const context::Context &context) no } inline context::Context SetBaggage(context::Context &context, - nostd::shared_ptr baggage) noexcept + const nostd::shared_ptr& baggage) noexcept { return context.SetValue(kBaggageHeader, baggage); } diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 620ca51c8b..963a1302bc 100644 --- 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 Context(const T &keys_and_values) noexcept - { - head_ = nostd::shared_ptr{new DataList(keys_and_values)}; - } + : head_{nostd::shared_ptr{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{new DataList(key, value)}; - } + : head_{nostd::shared_ptr{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 next_; + size_t key_length_ = 0UL; - size_t key_length_; + nostd::shared_ptr 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 - DataList(const T &keys_and_vals) : key_{nullptr}, next_(nostd::shared_ptr{nullptr}) + DataList(const T &keys_and_vals) : next_(nostd::shared_ptr{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{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 index a62146e100..52cb56af9f 100644 --- 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(GetPropagator()); } - static void SetGlobalPropagator(nostd::shared_ptr prop) noexcept + static void SetGlobalPropagator(const nostd::shared_ptr& prop) noexcept { std::lock_guard guard(GetLock()); GetPropagator() = prop; diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 73d4dcd908..cd75de0f96 100644 --- 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 storage) noexcept + static void SetRuntimeContextStorage(const nostd::shared_ptr& storage) noexcept { GetStorage() = storage; } diff --git a/api/include/opentelemetry/logs/event_id.h b/api/include/opentelemetry/logs/event_id.h index dc07484d09..65306ef793 100644 --- 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{new char[name.length() + 1]}} { - name_ = nostd::unique_ptr{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 index 8f65a1139d..e0999f1f60 100644 --- 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 tp) noexcept + static void SetLoggerProvider(const nostd::shared_ptr& tp) noexcept { std::lock_guard guard(GetLock()); GetProvider() = tp; @@ -60,7 +60,7 @@ class OPENTELEMETRY_EXPORT Provider /** * Changes the singleton EventLoggerProvider. */ - static void SetEventLoggerProvider(nostd::shared_ptr tp) noexcept + static void SetEventLoggerProvider(const nostd::shared_ptr& tp) noexcept { std::lock_guard guard(GetLock()); GetEventProvider() = tp; diff --git a/api/include/opentelemetry/trace/context.h b/api/include/opentelemetry/trace/context.h index cd8395d768..08b1e327ca 100644 --- 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) noexcept +inline context::Context SetSpan(context::Context &context, + const nostd::shared_ptr &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 index 7ad6a3726e..098b202af5 100644 --- 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 index 1439b579be..1b29e0bcca 100644 --- 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 tp) noexcept + static void SetTracerProvider(const nostd::shared_ptr& tp) noexcept { std::lock_guard guard(GetLock()); GetProvider() = tp; diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 9403a74fb6..61044d3c9d 100644 --- 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 trace_state = TraceState::GetDefault()) noexcept + const nostd::shared_ptr& 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_state() const noexcept { return trace_state_; } + const nostd::shared_ptr& 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 index e26f6bd2c6..f0f99bc59b 100644 --- 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 index bdd6eac27d..799dd5b7d3 100644 --- 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(); 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 file_; + std::shared_ptr file_ = std::make_shared(); - std::atomic is_initialized_; + std::atomic 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 index c330ffae2f..6a9fefa1e6 100644 --- 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(session)) { - sdk::common::ExportResult result = - opentelemetry::nostd::get(session); - if (result_callback) - { - result_callback(result); - } - return result; + return opentelemetry::nostd::get(session); } addSession(std::move(opentelemetry::nostd::get(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 index ef65388fe1..be48855b64 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 @@ -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 CreateRequest() noexcept override { @@ -225,7 +223,7 @@ class Session : public opentelemetry::ext::http::client::Session, std::shared_ptr http_request_; std::string host_; std::unique_ptr curl_operation_; - uint64_t session_id_; + uint64_t session_id_ = 0UL; HttpClient &http_client_; std::atomic 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 index 62fec23c1a..eadfd84fc9 100644 --- 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 index 8829eafdf3..6cfb9a5328 100644 --- a/opentracing-shim/src/shim_utils.cc +++ b/opentracing-shim/src/shim_utils.cc @@ -71,11 +71,7 @@ nostd::shared_ptr 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 index 5945df98c6..e554526666 100644 --- 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 index 59a08e08a6..4a5c0599a7 100644 --- 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 eh) noexcept + static inline void SetLogHandler(const nostd::shared_ptr& 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 index b33afb9d4e..9b1b1c2d7d 100644 --- 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 CloneAggregation(AggregationType aggregation_type, - InstrumentDescriptor instrument_descriptor, - const Aggregation &to_copy) + static std::unique_ptr 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 index abeebf45a8..26adc37c08 100644 --- 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 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 index 5d53959e1b..4e20e7d65f 100644 --- 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 allowed_attribute_keys = {}) + FilteringAttributesProcessor(std::unordered_map &&allowed_attribute_keys = {}) : allowed_attribute_keys_(std::move(allowed_attribute_keys)) {} + FilteringAttributesProcessor( + const std::unordered_map &allowed_attribute_keys = {}) + : allowed_attribute_keys_(allowed_attribute_keys) + {} + MetricAttributes process( const opentelemetry::common::KeyValueIterable &attributes) const noexcept override { From b89fbdee85d68e1d03790e6942d1f093c5210576 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 17 Nov 2024 22:34:56 -0500 Subject: [PATCH 02/14] Second pass --- api/include/opentelemetry/metrics/provider.h | 2 +- .../internal/absl/types/internal/variant.h | 3 ++ api/include/opentelemetry/nostd/shared_ptr.h | 2 +- api/include/opentelemetry/plugin/tracer.h | 2 +- examples/http/server.h | 4 +- .../elasticsearch/es_log_record_exporter.h | 12 +++--- exporters/otlp/src/otlp_http_client.cc | 2 +- .../otlp/src/otlp_http_exporter_options.cc | 39 ++++++++----------- .../otlp_http_log_record_exporter_options.cc | 39 ++++++++----------- .../src/otlp_http_metric_exporter_options.cc | 39 ++++++++----------- .../exporters/prometheus/exporter_utils.h | 4 +- exporters/prometheus/src/exporter_utils.cc | 4 +- exporters/zipkin/src/zipkin_exporter.cc | 11 ++++-- .../ext/http/client/curl/http_client_curl.h | 1 + .../ext/http/server/http_server.h | 4 +- .../ext/http/server/socket_tools.h | 2 +- .../sdk/common/circular_buffer.h | 2 + .../sdk/logs/batch_log_record_processor.h | 1 + .../sdk/logs/multi_log_record_processor.h | 2 + .../sdk/trace/batch_span_processor.h | 1 + .../sdk/trace/multi_span_processor.h | 1 + .../sdk/trace/simple_processor.h | 1 + sdk/src/trace/tracer_provider.cc | 9 +++-- 23 files changed, 94 insertions(+), 93 deletions(-) diff --git a/api/include/opentelemetry/metrics/provider.h b/api/include/opentelemetry/metrics/provider.h index b2fa3e20e4..15fb3b3f91 100644 --- a/api/include/opentelemetry/metrics/provider.h +++ b/api/include/opentelemetry/metrics/provider.h @@ -38,7 +38,7 @@ class Provider /** * Changes the singleton MeterProvider. */ - static void SetMeterProvider(nostd::shared_ptr tp) noexcept + static void SetMeterProvider(const nostd::shared_ptr& tp) noexcept { std::lock_guard guard(GetLock()); GetProvider() = tp; diff --git a/api/include/opentelemetry/nostd/internal/absl/types/internal/variant.h b/api/include/opentelemetry/nostd/internal/absl/types/internal/variant.h index ee42da7c93..a3223608c7 100644 --- a/api/include/opentelemetry/nostd/internal/absl/types/internal/variant.h +++ b/api/include/opentelemetry/nostd/internal/absl/types/internal/variant.h @@ -221,6 +221,7 @@ template struct MakeVisitationMatrix, index_sequence> { using ResultType = ReturnType (*)(FunctionObject&&); + // cppcheck-suppress [duplInheritedMember] static constexpr ResultType Run() { return &call_with_indices; @@ -722,6 +723,7 @@ struct VariantCoreAccess { Self* self, Args&&... args) { Destroy(*self); using New = typename absl::variant_alternative::type; + // cppcheck-suppress [legacyUninitvar] New* const result = ::new (static_cast(&self->state_)) New(absl::forward(args)...); self->index_ = NewIndex; @@ -1310,6 +1312,7 @@ class VariantStateBaseDestructorNontrivial : protected VariantStateBase { VariantStateBaseDestructorNontrivial* self; }; + // cppcheck-suppress [duplInheritedMember] void destroy() { VisitIndices::Run(Destroyer{this}, index_); } ~VariantStateBaseDestructorNontrivial() { destroy(); } diff --git a/api/include/opentelemetry/nostd/shared_ptr.h b/api/include/opentelemetry/nostd/shared_ptr.h index 0222352030..681c4eb377 100644 --- a/api/include/opentelemetry/nostd/shared_ptr.h +++ b/api/include/opentelemetry/nostd/shared_ptr.h @@ -37,7 +37,7 @@ class shared_ptr struct alignas(kAlignment) PlacementBuffer { - char data[kMaxSize]; + char data[kMaxSize]{}; }; class shared_ptr_wrapper diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index ae78109dab..ea430eddbe 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -20,7 +20,7 @@ class DynamicLibraryHandle; class Span final : public trace::Span { public: - Span(std::shared_ptr &&tracer, nostd::shared_ptr span) noexcept + Span(std::shared_ptr &&tracer, const nostd::shared_ptr& span) noexcept : tracer_{std::move(tracer)}, span_{span} {} diff --git a/examples/http/server.h b/examples/http/server.h index 1f73744d65..5691a6e465 100644 --- a/examples/http/server.h +++ b/examples/http/server.h @@ -19,13 +19,13 @@ class HttpServer : public HTTP_SERVER_NS::HttpRequestCallback std::atomic is_running_{false}; public: - HttpServer(std::string server_name = "test_server", uint16_t port = 8800) : port_(port) + HttpServer(const std::string &server_name = "test_server", uint16_t port = 8800) : port_(port) { server_.setServerName(server_name); server_.setKeepalive(false); } - void AddHandler(std::string path, HTTP_SERVER_NS::HttpRequestCallback *request_handler) + void AddHandler(const std::string &path, HTTP_SERVER_NS::HttpRequestCallback *request_handler) { server_.addHandler(path, *request_handler); } diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h index 36f2dfbca4..a8b23dc088 100644 --- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h +++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h @@ -52,12 +52,12 @@ struct ElasticsearchExporterOptions * from elasticsearch * @param console_debug If true, print the status of the exporter methods in the console */ - ElasticsearchExporterOptions(std::string host = "localhost", - int port = 9200, - std::string index = "logs", - int response_timeout = 30, - bool console_debug = false, - HttpHeaders http_headers = {}) + ElasticsearchExporterOptions(const std::string &host = "localhost", + int port = 9200, + const std::string &index = "logs", + int response_timeout = 30, + bool console_debug = false, + const HttpHeaders &http_headers = {}) : host_{host}, port_{port}, index_{index}, diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index 6a9fefa1e6..506295ad7f 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -898,7 +898,7 @@ OtlpHttpClient::createSession( { std::cerr << error_message << '\n'; } - OTEL_INTERNAL_LOG_ERROR(error_message.c_str()); + OTEL_INTERNAL_LOG_ERROR(error_message); const auto result = opentelemetry::sdk::common::ExportResult::kFailure; result_callback(result); diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 0b00fc9708..553f36f9d5 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -16,35 +16,30 @@ namespace otlp { OtlpHttpExporterOptions::OtlpHttpExporterOptions() - : json_bytes_mapping(JsonBytesMappingKind::kHexId), + : url(GetOtlpDefaultHttpTracesEndpoint()), + content_type(GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol())), + json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), - ssl_insecure_skip_verify(false) + timeout(GetOtlpDefaultTracesTimeout()), + http_headers(GetOtlpDefaultTracesHeaders()), + ssl_insecure_skip_verify(false), + ssl_ca_cert_path(GetOtlpDefaultTracesSslCertificatePath()), + ssl_ca_cert_string(GetOtlpDefaultTracesSslCertificateString()), + ssl_client_key_path(GetOtlpDefaultTracesSslClientKeyPath()), + ssl_client_key_string(GetOtlpDefaultTracesSslClientKeyString()), + ssl_client_cert_path(GetOtlpDefaultTracesSslClientCertificatePath()), + ssl_client_cert_string(GetOtlpDefaultTracesSslClientCertificateString()), + ssl_min_tls(GetOtlpDefaultTracesSslTlsMinVersion()), + ssl_max_tls(GetOtlpDefaultTracesSslTlsMaxVersion()), + ssl_cipher(GetOtlpDefaultTracesSslTlsCipher()), + ssl_cipher_suite(GetOtlpDefaultTracesSslTlsCipherSuite()), + compression(GetOtlpDefaultTracesCompression()) { - url = GetOtlpDefaultHttpTracesEndpoint(); - content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol()); - - timeout = GetOtlpDefaultTracesTimeout(); - http_headers = GetOtlpDefaultTracesHeaders(); - #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; max_requests_per_connection = 8; #endif /* ENABLE_ASYNC_EXPORT */ - - ssl_ca_cert_path = GetOtlpDefaultTracesSslCertificatePath(); - ssl_ca_cert_string = GetOtlpDefaultTracesSslCertificateString(); - ssl_client_key_path = GetOtlpDefaultTracesSslClientKeyPath(); - ssl_client_key_string = GetOtlpDefaultTracesSslClientKeyString(); - ssl_client_cert_path = GetOtlpDefaultTracesSslClientCertificatePath(); - ssl_client_cert_string = GetOtlpDefaultTracesSslClientCertificateString(); - - ssl_min_tls = GetOtlpDefaultTracesSslTlsMinVersion(); - ssl_max_tls = GetOtlpDefaultTracesSslTlsMaxVersion(); - ssl_cipher = GetOtlpDefaultTracesSslTlsCipher(); - ssl_cipher_suite = GetOtlpDefaultTracesSslTlsCipherSuite(); - - compression = GetOtlpDefaultTracesCompression(); } OtlpHttpExporterOptions::~OtlpHttpExporterOptions() {} diff --git a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc index f193ea10df..497cccc435 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -16,35 +16,30 @@ namespace otlp { OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() - : json_bytes_mapping(JsonBytesMappingKind::kHexId), + : url(GetOtlpDefaultHttpLogsEndpoint()), + content_type(GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpLogsProtocol())), + json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), - ssl_insecure_skip_verify(false) + timeout(GetOtlpDefaultLogsTimeout()), + http_headers(GetOtlpDefaultLogsHeaders()), + ssl_insecure_skip_verify(false), + ssl_ca_cert_path(GetOtlpDefaultLogsSslCertificatePath()), + ssl_ca_cert_string(GetOtlpDefaultLogsSslCertificateString()), + ssl_client_key_path(GetOtlpDefaultLogsSslClientKeyPath()), + ssl_client_key_string(GetOtlpDefaultLogsSslClientKeyString()), + ssl_client_cert_path(GetOtlpDefaultLogsSslClientCertificatePath()), + ssl_client_cert_string(GetOtlpDefaultLogsSslClientCertificateString()), + ssl_min_tls(GetOtlpDefaultLogsSslTlsMinVersion()), + ssl_max_tls(GetOtlpDefaultLogsSslTlsMaxVersion()), + ssl_cipher(GetOtlpDefaultLogsSslTlsCipher()), + ssl_cipher_suite(GetOtlpDefaultLogsSslTlsCipherSuite()), + compression(GetOtlpDefaultLogsCompression()) { - url = GetOtlpDefaultHttpLogsEndpoint(); - content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpLogsProtocol()); - - timeout = GetOtlpDefaultLogsTimeout(); - http_headers = GetOtlpDefaultLogsHeaders(); - #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; max_requests_per_connection = 8; #endif - - ssl_ca_cert_path = GetOtlpDefaultLogsSslCertificatePath(); - ssl_ca_cert_string = GetOtlpDefaultLogsSslCertificateString(); - ssl_client_key_path = GetOtlpDefaultLogsSslClientKeyPath(); - ssl_client_key_string = GetOtlpDefaultLogsSslClientKeyString(); - ssl_client_cert_path = GetOtlpDefaultLogsSslClientCertificatePath(); - ssl_client_cert_string = GetOtlpDefaultLogsSslClientCertificateString(); - - ssl_min_tls = GetOtlpDefaultLogsSslTlsMinVersion(); - ssl_max_tls = GetOtlpDefaultLogsSslTlsMaxVersion(); - ssl_cipher = GetOtlpDefaultLogsSslTlsCipher(); - ssl_cipher_suite = GetOtlpDefaultLogsSslTlsCipherSuite(); - - compression = GetOtlpDefaultLogsCompression(); } OtlpHttpLogRecordExporterOptions::~OtlpHttpLogRecordExporterOptions() {} diff --git a/exporters/otlp/src/otlp_http_metric_exporter_options.cc b/exporters/otlp/src/otlp_http_metric_exporter_options.cc index 95a8dd885c..831f3cec42 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -17,36 +17,31 @@ namespace otlp { OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() - : json_bytes_mapping(JsonBytesMappingKind::kHexId), + : url(GetOtlpDefaultMetricsEndpoint()), + content_type(GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpMetricsProtocol())), + json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), + timeout(GetOtlpDefaultMetricsTimeout()), + http_headers(GetOtlpDefaultMetricsHeaders()), aggregation_temporality(PreferredAggregationTemporality::kCumulative), - ssl_insecure_skip_verify(false) + ssl_insecure_skip_verify(false), + ssl_ca_cert_path(GetOtlpDefaultMetricsSslCertificatePath()), + ssl_ca_cert_string(GetOtlpDefaultMetricsSslCertificateString()), + ssl_client_key_path(GetOtlpDefaultMetricsSslClientKeyPath()), + ssl_client_key_string(GetOtlpDefaultMetricsSslClientKeyString()), + ssl_client_cert_path(GetOtlpDefaultMetricsSslClientCertificatePath()), + ssl_client_cert_string(GetOtlpDefaultMetricsSslClientCertificateString()), + ssl_min_tls(GetOtlpDefaultMetricsSslTlsMinVersion()), + ssl_max_tls(GetOtlpDefaultMetricsSslTlsMaxVersion()), + ssl_cipher(GetOtlpDefaultMetricsSslTlsCipher()), + ssl_cipher_suite(GetOtlpDefaultMetricsSslTlsCipherSuite()), + compression(GetOtlpDefaultMetricsCompression()) { - url = GetOtlpDefaultMetricsEndpoint(); - content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpMetricsProtocol()); - - timeout = GetOtlpDefaultMetricsTimeout(); - http_headers = GetOtlpDefaultMetricsHeaders(); - #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; max_requests_per_connection = 8; #endif - - ssl_ca_cert_path = GetOtlpDefaultMetricsSslCertificatePath(); - ssl_ca_cert_string = GetOtlpDefaultMetricsSslCertificateString(); - ssl_client_key_path = GetOtlpDefaultMetricsSslClientKeyPath(); - ssl_client_key_string = GetOtlpDefaultMetricsSslClientKeyString(); - ssl_client_cert_path = GetOtlpDefaultMetricsSslClientCertificatePath(); - ssl_client_cert_string = GetOtlpDefaultMetricsSslClientCertificateString(); - - ssl_min_tls = GetOtlpDefaultMetricsSslTlsMinVersion(); - ssl_max_tls = GetOtlpDefaultMetricsSslTlsMaxVersion(); - ssl_cipher = GetOtlpDefaultMetricsSslTlsCipher(); - ssl_cipher_suite = GetOtlpDefaultMetricsSslTlsCipherSuite(); - - compression = GetOtlpDefaultMetricsCompression(); } OtlpHttpMetricExporterOptions::~OtlpHttpMetricExporterOptions() {} diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h index 29c0f66daf..496ec9bd34 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h @@ -204,7 +204,7 @@ class PrometheusExporterUtils * Handle Counter and Gauge. */ template - static void SetValue(std::vector values, + static void SetValue(const std::vector &values, ::prometheus::MetricType type, ::prometheus::ClientMetric *metric); @@ -217,7 +217,7 @@ class PrometheusExporterUtils * Handle Histogram */ template - static void SetValue(std::vector values, + static void SetValue(const std::vector &values, const std::vector &boundaries, const std::vector &counts, ::prometheus::ClientMetric *metric); diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index f54b129ab8..598c88cb44 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -769,7 +769,7 @@ std::string PrometheusExporterUtils::AttributeValueToString( * Handle Counter. */ template -void PrometheusExporterUtils::SetValue(std::vector values, +void PrometheusExporterUtils::SetValue(const std::vector &values, prometheus_client::MetricType type, prometheus_client::ClientMetric *metric) { @@ -807,7 +807,7 @@ void PrometheusExporterUtils::SetValue(std::vector values, * Handle Histogram */ template -void PrometheusExporterUtils::SetValue(std::vector values, +void PrometheusExporterUtils::SetValue(const std::vector &values, const std::vector &boundaries, const std::vector &counts, prometheus_client::ClientMetric *metric) diff --git a/exporters/zipkin/src/zipkin_exporter.cc b/exporters/zipkin/src/zipkin_exporter.cc index 5d5dd728a6..f93746aeb9 100644 --- a/exporters/zipkin/src/zipkin_exporter.cc +++ b/exporters/zipkin/src/zipkin_exporter.cc @@ -37,15 +37,18 @@ namespace zipkin // -------------------------------- Constructors -------------------------------- ZipkinExporter::ZipkinExporter(const ZipkinExporterOptions &options) - : options_(options), url_parser_(options_.endpoint) + : options_(options), + http_client_(ext::http::client::HttpClientFactory::CreateSync()), + url_parser_(options_.endpoint) { - http_client_ = ext::http::client::HttpClientFactory::CreateSync(); InitializeLocalEndpoint(); } -ZipkinExporter::ZipkinExporter() : options_(ZipkinExporterOptions()), url_parser_(options_.endpoint) +ZipkinExporter::ZipkinExporter() + : options_(ZipkinExporterOptions()), + http_client_(ext::http::client::HttpClientFactory::CreateSync()), + url_parser_(options_.endpoint) { - http_client_ = ext::http::client::HttpClientFactory::CreateSync(); InitializeLocalEndpoint(); } 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 be48855b64..f51bf9cd67 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 @@ -307,6 +307,7 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient std::shared_ptr CreateSession( nostd::string_view url) noexcept override; + // cppcheck-suppress [virtualCallInConstructor] bool CancelAllSessions() noexcept override; bool FinishAllSessions() noexcept override; diff --git a/ext/include/opentelemetry/ext/http/server/http_server.h b/ext/include/opentelemetry/ext/http/server/http_server.h index 21efac94e0..0b30282752 100644 --- a/ext/include/opentelemetry/ext/http/server/http_server.h +++ b/ext/include/opentelemetry/ext/http/server/http_server.h @@ -120,7 +120,7 @@ class HttpServer : private SocketTools::Reactor::SocketCallback class HttpRequestHandler : public std::pair { public: - HttpRequestHandler(std::string key, HttpRequestCallback *value) + HttpRequestHandler(const std::string &key, HttpRequestCallback *value) { first = key; second = value; @@ -168,7 +168,7 @@ class HttpServer : private SocketTools::Reactor::SocketCallback m_maxRequestContentSize(2 * 1024 * 1024) {} - HttpServer(std::string serverHost, int port = 30000) : HttpServer() + HttpServer(const std::string &serverHost, int port = 30000) : HttpServer() { std::ostringstream os; os << serverHost << ":" << port; diff --git a/ext/include/opentelemetry/ext/http/server/socket_tools.h b/ext/include/opentelemetry/ext/http/server/socket_tools.h index bad76e640a..317d08e847 100644 --- a/ext/include/opentelemetry/ext/http/server/socket_tools.h +++ b/ext/include/opentelemetry/ext/http/server/socket_tools.h @@ -270,7 +270,7 @@ struct Socket Socket(Type sock = Invalid) : m_sock(sock) {} - Socket(int af, int type, int proto) { m_sock = ::socket(af, type, proto); } + Socket(int af, int type, int proto) : m_sock(::socket(af, type, proto)) {} ~Socket() {} diff --git a/sdk/include/opentelemetry/sdk/common/circular_buffer.h b/sdk/include/opentelemetry/sdk/common/circular_buffer.h index a35498ecea..928ff48f7f 100644 --- a/sdk/include/opentelemetry/sdk/common/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/common/circular_buffer.h @@ -185,8 +185,10 @@ class CircularBuffer if (tail_index < head_index) { return CircularBufferRange>{nostd::span>{ + // cppcheck-suppress [arithOperationsOnVoidPointer] data + tail_index, static_cast(head_index - tail_index)}}; } + // cppcheck-suppress [arithOperationsOnVoidPointer] return {nostd::span>{data + tail_index, static_cast(capacity_ - tail_index)}, nostd::span>{data, static_cast(head_index)}}; 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 b52476071a..ae0cd34553 100644 --- a/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h @@ -85,6 +85,7 @@ class BatchLogRecordProcessor : public LogRecordProcessor * * NOTE: Timeout functionality not supported yet. */ + // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; 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..44f061b515 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h @@ -43,6 +43,7 @@ class MultiLogRecordProcessor : public LogRecordProcessor * @param timeout that the forceflush is required to finish within. * @return a result code indicating whether it succeeded, failed or timed out */ + // cppcheck-suppress [virtualCallInConstructor] bool ForceFlush( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; @@ -53,6 +54,7 @@ class MultiLogRecordProcessor : public LogRecordProcessor * shutdown before giving up and returning failure. * @return true if the shutdown succeeded, false otherwise */ + // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h index e069a461e6..af20047fa1 100644 --- a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h @@ -83,6 +83,7 @@ class BatchSpanProcessor : public SpanProcessor * * NOTE: Timeout functionality not supported yet. */ + // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h index 094c5b3e24..b18892b0f8 100644 --- a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h @@ -121,6 +121,7 @@ class MultiSpanProcessor : public SpanProcessor return result; } + // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override { diff --git a/sdk/include/opentelemetry/sdk/trace/simple_processor.h b/sdk/include/opentelemetry/sdk/trace/simple_processor.h index ce6d378b79..7fc4c1a5e2 100644 --- a/sdk/include/opentelemetry/sdk/trace/simple_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/simple_processor.h @@ -74,6 +74,7 @@ class SimpleSpanProcessor : public SpanProcessor return true; } + // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override { diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 3b5462a083..9ca7854a5d 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -52,10 +52,11 @@ TracerProvider::TracerProvider(std::vector> &&pro const resource::Resource &resource, std::unique_ptr sampler, std::unique_ptr id_generator) noexcept -{ - context_ = std::make_shared(std::move(processors), resource, std::move(sampler), - std::move(id_generator)); -} + : context_(std::make_shared(std::move(processors), + resource, + std::move(sampler), + std::move(id_generator))) +{} TracerProvider::~TracerProvider() { From 558560e64d4f154b60fa82fe444b5048dd3086b1 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 00:25:32 -0500 Subject: [PATCH 03/14] Add cppcheck in CI --- .github/workflows/cppcheck.yml | 77 ++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 .github/workflows/cppcheck.yml diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml new file mode 100644 index 0000000000..0cdaee419f --- /dev/null +++ b/.github/workflows/cppcheck.yml @@ -0,0 +1,77 @@ + +name: cppcheck + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + cppcheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: 'recursive' + + - name: Set up dependencies + run: | + sudo apt update -y + sudo apt install -y --no-install-recommends --no-install-suggests \ + build-essential \ + cppcheck \ + cmake \ + ninja-build \ + libssl-dev \ + libcurl4-openssl-dev \ + libprotobuf-dev \ + protobuf-compiler \ + libgmock-dev \ + libgtest-dev \ + libbenchmark-dev + + - name: Prepare CMake + run: | + mkdir build && cd build + CC="clang" CXX="clang++" cmake .. + + - name: Run cppcheck + run: | + cppcheck \ + --force \ + --quiet \ + --enable=warning,performance,portability \ + --inline-suppr \ + --suppress=unknownMacro:exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h \ + --language=c++ \ + --std=c++14 \ + -I api/include \ + -I exporters/elasticsearch/include \ + -I exporters/etw/include \ + -I exporters/memory/include \ + -I exporters/ostream/include \ + -I exporters/otlp/include \ + -I exporters/prometheus/include \ + -I exporters/zipkin/include \ + -I ext/include \ + -I opentracing-shim/include \ + -I sdk/include \ + -i build \ + -i test \ + -i third_party \ + -j $(nproc) \ + . 2>&1 | tee cppcheck.log + + - uses: actions/upload-artifact@v4 + if: success() || failure() + with: + name: Logs (cppcheck) + path: ./cppcheck.log + + - name: Count warnings + run: | + set +e + COUNT=`grep -c -E "\[.+\]" cppcheck.log` + echo "cppcheck reported ${COUNT} warning(s)" + if [ $COUNT -ne 0 ] ; then exit 1 ; fi From 67ed142040661d30553e8311c5a0d6b656de6d93 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 01:06:02 -0500 Subject: [PATCH 04/14] Re-order initialization of member variables --- .../opentelemetry/ext/http/client/curl/http_client_curl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f51bf9cd67..397d238a59 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 @@ -172,7 +172,7 @@ class Session : public opentelemetry::ext::http::client::Session, const std::string &scheme = "http", const std::string &host = "", uint16_t port = 80) - : http_client_(http_client), host_{scheme + "://" + host + ":" + std::to_string(port) + "/"} + : host_{scheme + "://" + host + ":" + std::to_string(port) + "/"}, http_client_(http_client) {} std::shared_ptr CreateRequest() noexcept override From e8e1b85176e6bda9aecf22eba27787d4278ad305 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 01:18:23 -0500 Subject: [PATCH 05/14] Fix formatting --- .github/workflows/cppcheck.yml | 154 +++++++++--------- .../opentelemetry/baggage/baggage_context.h | 2 +- .../context/propagation/global_propagator.h | 2 +- .../opentelemetry/context/runtime_context.h | 3 +- api/include/opentelemetry/logs/provider.h | 4 +- api/include/opentelemetry/metrics/provider.h | 2 +- api/include/opentelemetry/plugin/tracer.h | 2 +- .../opentelemetry/trace/default_span.h | 2 +- api/include/opentelemetry/trace/provider.h | 2 +- .../opentelemetry/trace/span_context.h | 4 +- .../opentracingshim/span_context_shim.h | 2 +- .../sdk/common/global_log_handler.h | 2 +- 12 files changed, 91 insertions(+), 90 deletions(-) diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml index 0cdaee419f..8a73e530e0 100644 --- a/.github/workflows/cppcheck.yml +++ b/.github/workflows/cppcheck.yml @@ -1,77 +1,77 @@ - -name: cppcheck - -on: - push: - branches: [ main ] - pull_request: - branches: [ main ] - -jobs: - cppcheck: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - submodules: 'recursive' - - - name: Set up dependencies - run: | - sudo apt update -y - sudo apt install -y --no-install-recommends --no-install-suggests \ - build-essential \ - cppcheck \ - cmake \ - ninja-build \ - libssl-dev \ - libcurl4-openssl-dev \ - libprotobuf-dev \ - protobuf-compiler \ - libgmock-dev \ - libgtest-dev \ - libbenchmark-dev - - - name: Prepare CMake - run: | - mkdir build && cd build - CC="clang" CXX="clang++" cmake .. - - - name: Run cppcheck - run: | - cppcheck \ - --force \ - --quiet \ - --enable=warning,performance,portability \ - --inline-suppr \ - --suppress=unknownMacro:exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h \ - --language=c++ \ - --std=c++14 \ - -I api/include \ - -I exporters/elasticsearch/include \ - -I exporters/etw/include \ - -I exporters/memory/include \ - -I exporters/ostream/include \ - -I exporters/otlp/include \ - -I exporters/prometheus/include \ - -I exporters/zipkin/include \ - -I ext/include \ - -I opentracing-shim/include \ - -I sdk/include \ - -i build \ - -i test \ - -i third_party \ - -j $(nproc) \ - . 2>&1 | tee cppcheck.log - - - uses: actions/upload-artifact@v4 - if: success() || failure() - with: - name: Logs (cppcheck) - path: ./cppcheck.log - - - name: Count warnings - run: | - set +e - COUNT=`grep -c -E "\[.+\]" cppcheck.log` - echo "cppcheck reported ${COUNT} warning(s)" - if [ $COUNT -ne 0 ] ; then exit 1 ; fi + +name: cppcheck + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + cppcheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: 'recursive' + + - name: Set up dependencies + run: | + sudo apt update -y + sudo apt install -y --no-install-recommends --no-install-suggests \ + build-essential \ + cppcheck \ + cmake \ + ninja-build \ + libssl-dev \ + libcurl4-openssl-dev \ + libprotobuf-dev \ + protobuf-compiler \ + libgmock-dev \ + libgtest-dev \ + libbenchmark-dev + + - name: Prepare CMake + run: | + mkdir build && cd build + CC="clang" CXX="clang++" cmake .. + + - name: Run cppcheck + run: | + cppcheck \ + --force \ + --quiet \ + --enable=warning,performance,portability \ + --inline-suppr \ + --suppress=unknownMacro:exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h \ + --language=c++ \ + --std=c++14 \ + -I api/include \ + -I exporters/elasticsearch/include \ + -I exporters/etw/include \ + -I exporters/memory/include \ + -I exporters/ostream/include \ + -I exporters/otlp/include \ + -I exporters/prometheus/include \ + -I exporters/zipkin/include \ + -I ext/include \ + -I opentracing-shim/include \ + -I sdk/include \ + -i build \ + -i test \ + -i third_party \ + -j $(nproc) \ + . 2>&1 | tee cppcheck.log + + - uses: actions/upload-artifact@v4 + if: success() || failure() + with: + name: Logs (cppcheck) + path: ./cppcheck.log + + - name: Count warnings + run: | + set +e + COUNT=`grep -c -E "\[.+\]" cppcheck.log` + echo "cppcheck reported ${COUNT} warning(s)" + if [ $COUNT -ne 0 ] ; then exit 1 ; fi diff --git a/api/include/opentelemetry/baggage/baggage_context.h b/api/include/opentelemetry/baggage/baggage_context.h index 7755880426..a0016353ba 100644 --- a/api/include/opentelemetry/baggage/baggage_context.h +++ b/api/include/opentelemetry/baggage/baggage_context.h @@ -27,7 +27,7 @@ inline nostd::shared_ptr GetBaggage(const context::Context &context) no } inline context::Context SetBaggage(context::Context &context, - const nostd::shared_ptr& baggage) noexcept + const nostd::shared_ptr &baggage) noexcept { return context.SetValue(kBaggageHeader, baggage); } diff --git a/api/include/opentelemetry/context/propagation/global_propagator.h b/api/include/opentelemetry/context/propagation/global_propagator.h index 52cb56af9f..85293202f6 100644 --- 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(GetPropagator()); } - static void SetGlobalPropagator(const nostd::shared_ptr& prop) noexcept + static void SetGlobalPropagator(const nostd::shared_ptr &prop) noexcept { std::lock_guard guard(GetLock()); GetPropagator() = prop; diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index cd75de0f96..e3f88254cb 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -152,7 +152,8 @@ class OPENTELEMETRY_EXPORT RuntimeContext * * @param storage a custom runtime context storage */ - static void SetRuntimeContextStorage(const nostd::shared_ptr& storage) noexcept + static void SetRuntimeContextStorage( + const nostd::shared_ptr &storage) noexcept { GetStorage() = storage; } diff --git a/api/include/opentelemetry/logs/provider.h b/api/include/opentelemetry/logs/provider.h index e0999f1f60..1d8a1901d8 100644 --- 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(const nostd::shared_ptr& tp) noexcept + static void SetLoggerProvider(const nostd::shared_ptr &tp) noexcept { std::lock_guard guard(GetLock()); GetProvider() = tp; @@ -60,7 +60,7 @@ class OPENTELEMETRY_EXPORT Provider /** * Changes the singleton EventLoggerProvider. */ - static void SetEventLoggerProvider(const nostd::shared_ptr& tp) noexcept + static void SetEventLoggerProvider(const nostd::shared_ptr &tp) noexcept { std::lock_guard guard(GetLock()); GetEventProvider() = tp; diff --git a/api/include/opentelemetry/metrics/provider.h b/api/include/opentelemetry/metrics/provider.h index 15fb3b3f91..b8ca87843a 100644 --- a/api/include/opentelemetry/metrics/provider.h +++ b/api/include/opentelemetry/metrics/provider.h @@ -38,7 +38,7 @@ class Provider /** * Changes the singleton MeterProvider. */ - static void SetMeterProvider(const nostd::shared_ptr& tp) noexcept + static void SetMeterProvider(const nostd::shared_ptr &tp) noexcept { std::lock_guard guard(GetLock()); GetProvider() = tp; diff --git a/api/include/opentelemetry/plugin/tracer.h b/api/include/opentelemetry/plugin/tracer.h index ea430eddbe..3b0c5ff2cc 100644 --- a/api/include/opentelemetry/plugin/tracer.h +++ b/api/include/opentelemetry/plugin/tracer.h @@ -20,7 +20,7 @@ class DynamicLibraryHandle; class Span final : public trace::Span { public: - Span(std::shared_ptr &&tracer, const nostd::shared_ptr& span) noexcept + Span(std::shared_ptr &&tracer, const nostd::shared_ptr &span) noexcept : tracer_{std::move(tracer)}, span_{span} {} diff --git a/api/include/opentelemetry/trace/default_span.h b/api/include/opentelemetry/trace/default_span.h index 098b202af5..65ca19f3da 100644 --- 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(const 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 index 1b29e0bcca..7e22dc69f5 100644 --- 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(const nostd::shared_ptr& tp) noexcept + static void SetTracerProvider(const nostd::shared_ptr &tp) noexcept { std::lock_guard guard(GetLock()); GetProvider() = tp; diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 61044d3c9d..50d25ac12f 100644 --- 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, - const nostd::shared_ptr& trace_state = TraceState::GetDefault()) noexcept + const nostd::shared_ptr &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_state() const noexcept { return trace_state_; } + const nostd::shared_ptr &trace_state() const noexcept { return trace_state_; } /* * @param that SpanContext for comparing. diff --git a/opentracing-shim/include/opentelemetry/opentracingshim/span_context_shim.h b/opentracing-shim/include/opentelemetry/opentracingshim/span_context_shim.h index eadfd84fc9..55cdd5d86d 100644 --- 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/sdk/include/opentelemetry/sdk/common/global_log_handler.h b/sdk/include/opentelemetry/sdk/common/global_log_handler.h index 4a5c0599a7..eadd408762 100644 --- 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(const nostd::shared_ptr& eh) noexcept + static inline void SetLogHandler(const nostd::shared_ptr &eh) noexcept { GetHandlerAndLevel().first = eh; } From d79e186cd705ec8a4668ee4ea308cc1d18c5cb4e Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 02:25:01 -0500 Subject: [PATCH 06/14] Put back some verbosity --- .github/workflows/cppcheck.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml index 8a73e530e0..79344f14e0 100644 --- a/.github/workflows/cppcheck.yml +++ b/.github/workflows/cppcheck.yml @@ -40,7 +40,6 @@ jobs: run: | cppcheck \ --force \ - --quiet \ --enable=warning,performance,portability \ --inline-suppr \ --suppress=unknownMacro:exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h \ From 207c3e3e49ff31cb88ceaa2975fcd135e2a2b159 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 17:29:18 -0500 Subject: [PATCH 07/14] Fix unintialized buffer in copy ctor --- api/include/opentelemetry/context/context.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 963a1302bc..718016b1dd 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -135,7 +135,10 @@ class Context } DataList(const DataList &other) - : key_length_(other.key_length_), next_(other.next_), value_(other.value_) + : key_(new char[other.key_length_]), + key_length_(other.key_length_), + next_(other.next_), + value_(other.value_) { memcpy(key_, other.key_, other.key_length_ * sizeof(char)); } From 9305c6c4fb1610f44927f6746ce15ed6e9c92bfa Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 20:20:25 -0500 Subject: [PATCH 08/14] Revert ordering of members --- api/include/opentelemetry/context/context.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 718016b1dd..a7e19cb3e6 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -94,10 +94,10 @@ class Context { char *key_ = nullptr; - size_t key_length_ = 0UL; - nostd::shared_ptr next_; + size_t key_length_ = 0UL; + ContextValue value_; DataList() : next_{nullptr} {} From f1b18df10b6581b6fc8c2c3f090e4ac7e7f68d96 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 18 Nov 2024 23:57:11 -0500 Subject: [PATCH 09/14] Fix reorder error in maintainer mode --- api/include/opentelemetry/context/context.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index a7e19cb3e6..5dc82faca0 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -94,13 +94,13 @@ class Context { char *key_ = nullptr; - nostd::shared_ptr next_; + nostd::shared_ptr next_{nullptr}; size_t key_length_ = 0UL; ContextValue value_; - DataList() : next_{nullptr} {} + DataList() = default; // Builds a data list off of a key and value iterable and returns the head template @@ -136,8 +136,8 @@ class Context DataList(const DataList &other) : key_(new char[other.key_length_]), - key_length_(other.key_length_), next_(other.next_), + key_length_(other.key_length_), value_(other.value_) { memcpy(key_, other.key_, other.key_length_ * sizeof(char)); From e9b225295e05a2baf71708e6486036042ed69542 Mon Sep 17 00:00:00 2001 From: Alex E Date: Tue, 19 Nov 2024 19:20:21 -0500 Subject: [PATCH 10/14] Code review feedback + minor touchup --- api/include/opentelemetry/context/context.h | 2 +- api/include/opentelemetry/trace/default_span.h | 2 +- api/include/opentelemetry/trace/span_context.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 5dc82faca0..6d97659696 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -104,7 +104,7 @@ class Context // Builds a data list off of a key and value iterable and returns the head template - DataList(const T &keys_and_vals) : next_(nostd::shared_ptr{nullptr}) + DataList(const T &keys_and_vals) { bool first = true; auto *node = this; diff --git a/api/include/opentelemetry/trace/default_span.h b/api/include/opentelemetry/trace/default_span.h index 65ca19f3da..1677a32ea5 100644 --- 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(const SpanContext &span_context) noexcept : span_context_(span_context) {} + DefaultSpan(SpanContext span_context) noexcept : span_context_(std::move(span_context)) {} // movable and copiable DefaultSpan(DefaultSpan &&spn) noexcept : Span(), span_context_(spn.GetContext()) {} diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 50d25ac12f..2062147e55 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -41,12 +41,12 @@ class SpanContext final SpanId span_id, TraceFlags trace_flags, bool is_remote, - const nostd::shared_ptr &trace_state = TraceState::GetDefault()) noexcept + nostd::shared_ptr trace_state = TraceState::GetDefault()) noexcept : trace_id_(trace_id), span_id_(span_id), trace_flags_(trace_flags), is_remote_(is_remote), - trace_state_(trace_state) + trace_state_(std::move(trace_state)) {} SpanContext(const SpanContext &ctx) = default; From 6c062f833f9377a273a23b6fd7637c759f9ef811 Mon Sep 17 00:00:00 2001 From: Alex E Date: Thu, 21 Nov 2024 22:51:12 -0500 Subject: [PATCH 11/14] Code review part 2 --- .github/workflows/cppcheck.yml | 15 +++++++++++---- .../ext/http/client/curl/http_client_curl.h | 1 - .../opentelemetry/sdk/common/circular_buffer.h | 2 -- .../sdk/logs/batch_log_record_processor.h | 1 - .../sdk/logs/multi_log_record_processor.h | 2 -- .../sdk/trace/batch_span_processor.h | 1 - .../sdk/trace/multi_span_processor.h | 1 - .../opentelemetry/sdk/trace/simple_processor.h | 1 - 8 files changed, 11 insertions(+), 13 deletions(-) diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml index a6581634bc..708f075c85 100644 --- a/.github/workflows/cppcheck.yml +++ b/.github/workflows/cppcheck.yml @@ -56,7 +56,14 @@ jobs: - name: Count warnings run: | set +e - COUNT=`grep -c -E "\[.+\]" cppcheck.log` - echo "cppcheck reported ${COUNT} warning(s)" - # TODO: uncomment to enforce failing the build - # if [ $COUNT -ne 0 ] ; then exit 1 ; fi + 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 + echo "::warning::cppcheck reported ${WARNING_COUNT} warning(s)" + 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 397d238a59..97386890f2 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 @@ -307,7 +307,6 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient std::shared_ptr CreateSession( nostd::string_view url) noexcept override; - // cppcheck-suppress [virtualCallInConstructor] bool CancelAllSessions() noexcept override; bool FinishAllSessions() noexcept override; diff --git a/sdk/include/opentelemetry/sdk/common/circular_buffer.h b/sdk/include/opentelemetry/sdk/common/circular_buffer.h index 928ff48f7f..a35498ecea 100644 --- a/sdk/include/opentelemetry/sdk/common/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/common/circular_buffer.h @@ -185,10 +185,8 @@ class CircularBuffer if (tail_index < head_index) { return CircularBufferRange>{nostd::span>{ - // cppcheck-suppress [arithOperationsOnVoidPointer] data + tail_index, static_cast(head_index - tail_index)}}; } - // cppcheck-suppress [arithOperationsOnVoidPointer] return {nostd::span>{data + tail_index, static_cast(capacity_ - tail_index)}, nostd::span>{data, static_cast(head_index)}}; 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 ae0cd34553..b52476071a 100644 --- a/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h @@ -85,7 +85,6 @@ class BatchLogRecordProcessor : public LogRecordProcessor * * NOTE: Timeout functionality not supported yet. */ - // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; 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 44f061b515..46432ef22e 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h @@ -43,7 +43,6 @@ class MultiLogRecordProcessor : public LogRecordProcessor * @param timeout that the forceflush is required to finish within. * @return a result code indicating whether it succeeded, failed or timed out */ - // cppcheck-suppress [virtualCallInConstructor] bool ForceFlush( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; @@ -54,7 +53,6 @@ class MultiLogRecordProcessor : public LogRecordProcessor * shutdown before giving up and returning failure. * @return true if the shutdown succeeded, false otherwise */ - // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h index af20047fa1..e069a461e6 100644 --- a/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/batch_span_processor.h @@ -83,7 +83,6 @@ class BatchSpanProcessor : public SpanProcessor * * NOTE: Timeout functionality not supported yet. */ - // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h index b18892b0f8..094c5b3e24 100644 --- a/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/multi_span_processor.h @@ -121,7 +121,6 @@ class MultiSpanProcessor : public SpanProcessor return result; } - // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override { diff --git a/sdk/include/opentelemetry/sdk/trace/simple_processor.h b/sdk/include/opentelemetry/sdk/trace/simple_processor.h index 7fc4c1a5e2..ce6d378b79 100644 --- a/sdk/include/opentelemetry/sdk/trace/simple_processor.h +++ b/sdk/include/opentelemetry/sdk/trace/simple_processor.h @@ -74,7 +74,6 @@ class SimpleSpanProcessor : public SpanProcessor return true; } - // cppcheck-suppress [virtualCallInConstructor] bool Shutdown( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override { From f22a295be89ebb83cee04d1819c7a4d316b81b3d Mon Sep 17 00:00:00 2001 From: Alex E Date: Fri, 22 Nov 2024 19:38:24 -0500 Subject: [PATCH 12/14] Code review part 3 --- api/include/opentelemetry/context/context.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 6d97659696..924036efad 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -129,7 +129,7 @@ class Context { key_ = new char[key.size()]; key_length_ = key.size(); - memcpy(key_, key.data(), key.size() * sizeof(char)); + std::memcpy(key_, key.data(), key.size() * sizeof(char)); next_ = nostd::shared_ptr{nullptr}; value_ = value; } @@ -140,7 +140,7 @@ class Context key_length_(other.key_length_), value_(other.value_) { - memcpy(key_, other.key_, other.key_length_ * sizeof(char)); + std::memcpy(key_, other.key_, other.key_length_ * sizeof(char)); } DataList &operator=(DataList &&other) noexcept From 6c6925bfdd7e1a317ca088a533e9655c020712eb Mon Sep 17 00:00:00 2001 From: Alex E Date: Wed, 27 Nov 2024 20:02:58 -0500 Subject: [PATCH 13/14] Fix iwyu warnings --- api/include/opentelemetry/trace/span_context.h | 2 ++ exporters/otlp/src/otlp_http_exporter_options.cc | 3 --- exporters/otlp/src/otlp_http_log_record_exporter_options.cc | 3 --- exporters/otlp/src/otlp_http_metric_exporter_options.cc | 3 --- 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index 2062147e55..a1948231bd 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -5,6 +5,8 @@ #include +#include + #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_flags.h" diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 553f36f9d5..26ef3f3775 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -1,9 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include -#include - #include "opentelemetry/exporters/otlp/otlp_environment.h" #include "opentelemetry/exporters/otlp/otlp_http.h" #include "opentelemetry/exporters/otlp/otlp_http_exporter_options.h" diff --git a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc index 497cccc435..f8dfb6f9f7 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -1,9 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include -#include - #include "opentelemetry/exporters/otlp/otlp_environment.h" #include "opentelemetry/exporters/otlp/otlp_http.h" #include "opentelemetry/exporters/otlp/otlp_http_log_record_exporter_options.h" diff --git a/exporters/otlp/src/otlp_http_metric_exporter_options.cc b/exporters/otlp/src/otlp_http_metric_exporter_options.cc index 831f3cec42..4d462c6fa2 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -1,9 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include -#include - #include "opentelemetry/exporters/otlp/otlp_environment.h" #include "opentelemetry/exporters/otlp/otlp_http.h" #include "opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h" From fabee6ab4902da2f4c9a6e48c76e66ea6fd0654c Mon Sep 17 00:00:00 2001 From: Alex E Date: Wed, 27 Nov 2024 20:05:48 -0500 Subject: [PATCH 14/14] Fix format order of includes --- exporters/otlp/src/otlp_http_exporter_options.cc | 2 +- exporters/otlp/src/otlp_http_log_record_exporter_options.cc | 2 +- exporters/otlp/src/otlp_http_metric_exporter_options.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 26ef3f3775..f6f7027f50 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -1,9 +1,9 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "opentelemetry/exporters/otlp/otlp_http_exporter_options.h" #include "opentelemetry/exporters/otlp/otlp_environment.h" #include "opentelemetry/exporters/otlp/otlp_http.h" -#include "opentelemetry/exporters/otlp/otlp_http_exporter_options.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc index f8dfb6f9f7..b38a292476 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -1,9 +1,9 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "opentelemetry/exporters/otlp/otlp_http_log_record_exporter_options.h" #include "opentelemetry/exporters/otlp/otlp_environment.h" #include "opentelemetry/exporters/otlp/otlp_http.h" -#include "opentelemetry/exporters/otlp/otlp_http_log_record_exporter_options.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/exporters/otlp/src/otlp_http_metric_exporter_options.cc b/exporters/otlp/src/otlp_http_metric_exporter_options.cc index 4d462c6fa2..b5fb83b090 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -1,9 +1,9 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h" #include "opentelemetry/exporters/otlp/otlp_environment.h" #include "opentelemetry/exporters/otlp/otlp_http.h" -#include "opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h" #include "opentelemetry/exporters/otlp/otlp_preferred_temporality.h" #include "opentelemetry/version.h"