From 9e26946a29fa42954590f5d8554b02c116f30474 Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sat, 27 Jul 2024 10:49:39 -0400 Subject: [PATCH 01/10] Clang-tidy clean up 2 --- .../propagation/baggage_propagator_test.cc | 4 ++-- .../propagation/composite_propagator_test.cc | 6 +++--- api/test/logs/logger_benchmark.cc | 4 ++-- api/test/nostd/shared_ptr_test.cc | 2 +- .../trace/propagation/b3_propagation_test.cc | 4 ++-- .../propagation/http_text_format_test.cc | 4 ++-- .../propagation/jaeger_propagation_test.cc | 4 ++-- examples/http/server.cc | 2 +- exporters/otlp/src/otlp_file_client.cc | 4 ++-- .../src/otlp_file_metric_exporter_options.cc | 4 ++-- .../src/otlp_grpc_metric_exporter_options.cc | 4 ++-- .../otlp/src/otlp_http_exporter_options.cc | 8 ++++---- .../otlp_http_log_record_exporter_options.cc | 10 +++++----- .../src/otlp_http_metric_exporter_options.cc | 11 +++++----- exporters/prometheus/test/collector_test.cc | 4 ++-- exporters/zipkin/src/zipkin_exporter.cc | 4 ++-- ext/src/http/client/curl/http_client_curl.cc | 4 ++-- ext/test/http/curl_http_test.cc | 10 +++++----- ext/test/w3c_tracecontext_test/main.cc | 4 ++-- sdk/src/logs/logger_provider.cc | 4 ++-- .../aggregation/lastvalue_aggregation.cc | 20 +++++++++---------- .../metrics/aggregation/sum_aggregation.cc | 4 ++-- sdk/src/metrics/state/sync_metric_storage.cc | 2 +- sdk/src/trace/samplers/parent_factory.cc | 2 +- sdk/test/common/random_test.cc | 3 ++- .../periodic_exporting_metric_reader_test.cc | 4 ++-- 26 files changed, 68 insertions(+), 68 deletions(-) diff --git a/api/test/baggage/propagation/baggage_propagator_test.cc b/api/test/baggage/propagation/baggage_propagator_test.cc index 83de75757b..c966fe7603 100644 --- a/api/test/baggage/propagation/baggage_propagator_test.cc +++ b/api/test/baggage/propagation/baggage_propagator_test.cc @@ -15,7 +15,7 @@ class BaggageCarrierTest : public context::propagation::TextMapCarrier { public: BaggageCarrierTest() = default; - virtual nostd::string_view Get(nostd::string_view key) const noexcept override + nostd::string_view Get(nostd::string_view key) const noexcept override { auto it = headers_.find(std::string(key)); if (it != headers_.end()) @@ -24,7 +24,7 @@ class BaggageCarrierTest : public context::propagation::TextMapCarrier } return ""; } - virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + void Set(nostd::string_view key, nostd::string_view value) noexcept override { headers_[std::string(key)] = std::string(value); } diff --git a/api/test/context/propagation/composite_propagator_test.cc b/api/test/context/propagation/composite_propagator_test.cc index 5bf18710ed..075841080f 100644 --- a/api/test/context/propagation/composite_propagator_test.cc +++ b/api/test/context/propagation/composite_propagator_test.cc @@ -31,7 +31,7 @@ static std::string Hex(const T &id_item) class TextMapCarrierTest : public context::propagation::TextMapCarrier { public: - virtual nostd::string_view Get(nostd::string_view key) const noexcept override + nostd::string_view Get(nostd::string_view key) const noexcept override { auto it = headers_.find(std::string(key)); if (it != headers_.end()) @@ -40,7 +40,7 @@ class TextMapCarrierTest : public context::propagation::TextMapCarrier } return ""; } - virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + void Set(nostd::string_view key, nostd::string_view value) noexcept override { headers_[std::string(key)] = std::string(value); } @@ -66,7 +66,7 @@ class CompositePropagatorTest : public ::testing::Test new context::propagation::CompositePropagator(std::move(propogator_list)); } - ~CompositePropagatorTest() { delete composite_propagator_; } + ~CompositePropagatorTest() override { delete composite_propagator_; } protected: context::propagation::CompositePropagator *composite_propagator_; diff --git a/api/test/logs/logger_benchmark.cc b/api/test/logs/logger_benchmark.cc index db225d496a..ff333511b4 100644 --- a/api/test/logs/logger_benchmark.cc +++ b/api/test/logs/logger_benchmark.cc @@ -32,7 +32,7 @@ constexpr int64_t kMaxIterations = 1000000000; class Barrier { public: - explicit Barrier(std::size_t iCount) : mThreshold(iCount), mCount(iCount), mGeneration(0) {} + explicit Barrier(std::size_t iCount) : mThreshold(iCount), mCount(iCount) {} void Wait() { @@ -55,7 +55,7 @@ class Barrier std::condition_variable mCond; std::size_t mThreshold; std::size_t mCount; - std::size_t mGeneration; + std::size_t mGeneration{0}; }; static void ThreadRoutine(Barrier &barrier, diff --git a/api/test/nostd/shared_ptr_test.cc b/api/test/nostd/shared_ptr_test.cc index 5c290c6284..2ebba82628 100644 --- a/api/test/nostd/shared_ptr_test.cc +++ b/api/test/nostd/shared_ptr_test.cc @@ -35,7 +35,7 @@ class C class D : public C { public: - virtual ~D() {} + ~D() override {} }; TEST(SharedPtrTest, DefaultConstruction) diff --git a/api/test/trace/propagation/b3_propagation_test.cc b/api/test/trace/propagation/b3_propagation_test.cc index 5546916abc..732db1bd34 100644 --- a/api/test/trace/propagation/b3_propagation_test.cc +++ b/api/test/trace/propagation/b3_propagation_test.cc @@ -15,7 +15,7 @@ using namespace opentelemetry; class TextMapCarrierTest : public context::propagation::TextMapCarrier { public: - virtual nostd::string_view Get(nostd::string_view key) const noexcept override + nostd::string_view Get(nostd::string_view key) const noexcept override { auto it = headers_.find(std::string(key)); if (it != headers_.end()) @@ -24,7 +24,7 @@ class TextMapCarrierTest : public context::propagation::TextMapCarrier } return ""; } - virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + void Set(nostd::string_view key, nostd::string_view value) noexcept override { headers_[std::string(key)] = std::string(value); } diff --git a/api/test/trace/propagation/http_text_format_test.cc b/api/test/trace/propagation/http_text_format_test.cc index e41e0fe65c..99f7117928 100644 --- a/api/test/trace/propagation/http_text_format_test.cc +++ b/api/test/trace/propagation/http_text_format_test.cc @@ -18,7 +18,7 @@ using namespace opentelemetry; class TextMapCarrierTest : public context::propagation::TextMapCarrier { public: - virtual nostd::string_view Get(nostd::string_view key) const noexcept override + nostd::string_view Get(nostd::string_view key) const noexcept override { auto it = headers_.find(std::string(key)); if (it != headers_.end()) @@ -27,7 +27,7 @@ class TextMapCarrierTest : public context::propagation::TextMapCarrier } return ""; } - virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + void Set(nostd::string_view key, nostd::string_view value) noexcept override { headers_[std::string(key)] = std::string(value); } diff --git a/api/test/trace/propagation/jaeger_propagation_test.cc b/api/test/trace/propagation/jaeger_propagation_test.cc index add38eac6c..f0fdaf734d 100644 --- a/api/test/trace/propagation/jaeger_propagation_test.cc +++ b/api/test/trace/propagation/jaeger_propagation_test.cc @@ -14,7 +14,7 @@ using namespace opentelemetry; class TextMapCarrierTest : public context::propagation::TextMapCarrier { public: - virtual nostd::string_view Get(nostd::string_view key) const noexcept override + nostd::string_view Get(nostd::string_view key) const noexcept override { auto it = headers_.find(std::string(key)); if (it != headers_.end()) @@ -23,7 +23,7 @@ class TextMapCarrierTest : public context::propagation::TextMapCarrier } return ""; } - virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + void Set(nostd::string_view key, nostd::string_view value) noexcept override { headers_[std::string(key)] = std::string(value); } diff --git a/examples/http/server.cc b/examples/http/server.cc index 3dbdae8512..2a5bcb615f 100644 --- a/examples/http/server.cc +++ b/examples/http/server.cc @@ -21,7 +21,7 @@ constexpr const char *server_name = "localhost"; class RequestHandler : public HTTP_SERVER_NS::HttpRequestCallback { public: - virtual int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, + int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, HTTP_SERVER_NS::HttpResponse &response) override { StartSpanOptions options; diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index cc6e074bfd..8e34dcb567 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -966,7 +966,7 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender { public: explicit OtlpFileSystemBackend(const OtlpFileClientFileSystemOptions &options) - : options_(options), is_initialized_{false}, check_file_path_interval_{0} + : options_(options), is_initialized_{false} { file_ = std::make_shared(); file_->is_shutdown.store(false); @@ -1539,7 +1539,7 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender std::shared_ptr file_; std::atomic is_initialized_; - std::time_t check_file_path_interval_; + std::time_t check_file_path_interval_{0}; }; class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileOstreamBackend : public OtlpFileAppender diff --git a/exporters/otlp/src/otlp_file_metric_exporter_options.cc b/exporters/otlp/src/otlp_file_metric_exporter_options.cc index 3ad8eaa457..5b27c0a8b8 100644 --- a/exporters/otlp/src/otlp_file_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_file_metric_exporter_options.cc @@ -12,7 +12,7 @@ namespace exporter namespace otlp { -OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() +OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() : aggregation_temporality(PreferredAggregationTemporality::kCumulative) { console_debug = false; @@ -26,7 +26,7 @@ OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() backend_options = fs_options; - aggregation_temporality = PreferredAggregationTemporality::kCumulative; + } OtlpFileMetricExporterOptions::~OtlpFileMetricExporterOptions() {} diff --git a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc index 983561424f..9b9ee188ca 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc @@ -10,7 +10,7 @@ namespace exporter namespace otlp { -OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() +OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() : aggregation_temporality(PreferredAggregationTemporality::kCumulative) { endpoint = GetOtlpDefaultGrpcMetricsEndpoint(); use_ssl_credentials = !GetOtlpDefaultGrpcMetricsIsInsecure(); /* negation intended. */ @@ -28,7 +28,7 @@ OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() metadata = GetOtlpDefaultMetricsHeaders(); user_agent = GetOtlpDefaultUserAgent(); - aggregation_temporality = PreferredAggregationTemporality::kCumulative; + max_threads = 0; diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 753efcb481..c02ae11ce1 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -15,13 +15,13 @@ namespace exporter namespace otlp { -OtlpHttpExporterOptions::OtlpHttpExporterOptions() +OtlpHttpExporterOptions::OtlpHttpExporterOptions() : json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), ssl_insecure_skip_verify(false) { url = GetOtlpDefaultHttpTracesEndpoint(); content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol()); - json_bytes_mapping = JsonBytesMappingKind::kHexId; - use_json_name = false; - console_debug = false; + + + timeout = GetOtlpDefaultTracesTimeout(); http_headers = GetOtlpDefaultTracesHeaders(); 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 08a04cfa80..b1e007b9c6 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -15,13 +15,13 @@ namespace exporter namespace otlp { -OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() +OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() : json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), max_concurrent_requests(64), max_requests_per_connection(8), ssl_insecure_skip_verify(false) { url = GetOtlpDefaultHttpLogsEndpoint(); content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpLogsProtocol()); - json_bytes_mapping = JsonBytesMappingKind::kHexId; - use_json_name = false; - console_debug = false; + + + timeout = GetOtlpDefaultLogsTimeout(); http_headers = GetOtlpDefaultLogsHeaders(); @@ -30,7 +30,7 @@ OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() max_requests_per_connection = 8; #endif - ssl_insecure_skip_verify = false; + ssl_ca_cert_path = GetOtlpDefaultLogsSslCertificatePath(); ssl_ca_cert_string = GetOtlpDefaultLogsSslCertificateString(); ssl_client_key_path = GetOtlpDefaultLogsSslClientKeyPath(); diff --git a/exporters/otlp/src/otlp_http_metric_exporter_options.cc b/exporters/otlp/src/otlp_http_metric_exporter_options.cc index 66d65fa54b..48771f40e3 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -16,23 +16,22 @@ namespace exporter namespace otlp { -OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() +OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() : json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), aggregation_temporality(PreferredAggregationTemporality::kCumulative), ssl_insecure_skip_verify(false) { 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; + #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; max_requests_per_connection = 8; #endif - ssl_insecure_skip_verify = false; ssl_ca_cert_path = GetOtlpDefaultMetricsSslCertificatePath(); ssl_ca_cert_string = GetOtlpDefaultMetricsSslCertificateString(); ssl_client_key_path = GetOtlpDefaultMetricsSslClientKeyPath(); diff --git a/exporters/prometheus/test/collector_test.cc b/exporters/prometheus/test/collector_test.cc index 63f840bc9a..d49efb94ab 100644 --- a/exporters/prometheus/test/collector_test.cc +++ b/exporters/prometheus/test/collector_test.cc @@ -23,7 +23,7 @@ class MockMetricProducer : public opentelemetry::sdk::metrics::MetricProducer public: MockMetricProducer(std::chrono::microseconds sleep_ms = std::chrono::microseconds::zero()) - : sleep_ms_{sleep_ms}, data_sent_size_(0) + : sleep_ms_{sleep_ms} {} bool Collect(nostd::function_ref callback) noexcept override @@ -39,7 +39,7 @@ class MockMetricProducer : public opentelemetry::sdk::metrics::MetricProducer private: std::chrono::microseconds sleep_ms_; - size_t data_sent_size_; + size_t data_sent_size_{0}; }; class MockMetricReader : public opentelemetry::sdk::metrics::MetricReader diff --git a/exporters/zipkin/src/zipkin_exporter.cc b/exporters/zipkin/src/zipkin_exporter.cc index a554cbf4cc..2be9c02b74 100644 --- a/exporters/zipkin/src/zipkin_exporter.cc +++ b/exporters/zipkin/src/zipkin_exporter.cc @@ -50,9 +50,9 @@ ZipkinExporter::ZipkinExporter() : options_(ZipkinExporterOptions()), url_parser ZipkinExporter::ZipkinExporter( std::shared_ptr http_client) - : options_(ZipkinExporterOptions()), url_parser_(options_.endpoint) + : options_(ZipkinExporterOptions()), http_client_(http_client), url_parser_(options_.endpoint) { - http_client_ = http_client; + InitializeLocalEndpoint(); } diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index 67b0b93a54..c3e0119029 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -185,12 +185,12 @@ void Session::FinishOperation() } HttpClient::HttpClient() - : next_session_id_{0}, + : multi_handle_(curl_multi_init()), next_session_id_{0}, max_sessions_per_connection_{8}, scheduled_delay_milliseconds_{std::chrono::milliseconds(256)}, curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance()) { - multi_handle_ = curl_multi_init(); + } HttpClient::~HttpClient() diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 0dc6e5d5e8..f0591d1d5d 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -26,11 +26,11 @@ namespace nostd = opentelemetry::nostd; class CustomEventHandler : public http_client::EventHandler { public: - virtual void OnResponse(http_client::Response & /* response */) noexcept override + void OnResponse(http_client::Response & /* response */) noexcept override { got_response_ = true; } - virtual void OnEvent(http_client::SessionState state, + void OnEvent(http_client::SessionState state, nostd::string_view /* reason */) noexcept override { switch (state) @@ -113,7 +113,7 @@ class BasicCurlHttpTests : public ::testing::Test, public HTTP_SERVER_NS::HttpRe public: BasicCurlHttpTests() : is_setup_(false), is_running_(false) {} - virtual void SetUp() override + void SetUp() override { if (is_setup_.exchange(true)) { @@ -132,7 +132,7 @@ class BasicCurlHttpTests : public ::testing::Test, public HTTP_SERVER_NS::HttpRe is_running_ = true; } - virtual void TearDown() override + void TearDown() override { if (!is_setup_.exchange(false)) return; @@ -140,7 +140,7 @@ class BasicCurlHttpTests : public ::testing::Test, public HTTP_SERVER_NS::HttpRe is_running_ = false; } - virtual int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, + int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, HTTP_SERVER_NS::HttpResponse &response) override { int response_status = 404; diff --git a/ext/test/w3c_tracecontext_test/main.cc b/ext/test/w3c_tracecontext_test/main.cc index d9819cde0d..4e9fb36796 100644 --- a/ext/test/w3c_tracecontext_test/main.cc +++ b/ext/test/w3c_tracecontext_test/main.cc @@ -30,7 +30,7 @@ class TextMapCarrierTest : public context::propagation::TextMapCarrier { public: TextMapCarrierTest(std::map &headers) : headers_(headers) {} - virtual nostd::string_view Get(nostd::string_view key) const noexcept override + nostd::string_view Get(nostd::string_view key) const noexcept override { auto it = headers_.find(std::string(key)); if (it != headers_.end()) @@ -39,7 +39,7 @@ class TextMapCarrierTest : public context::propagation::TextMapCarrier } return ""; } - virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + void Set(nostd::string_view key, nostd::string_view value) noexcept override { headers_[std::string(key)] = std::string(value); } diff --git a/sdk/src/logs/logger_provider.cc b/sdk/src/logs/logger_provider.cc index fdea3eb51a..46bf1ade33 100644 --- a/sdk/src/logs/logger_provider.cc +++ b/sdk/src/logs/logger_provider.cc @@ -31,13 +31,13 @@ LoggerProvider::LoggerProvider(std::unique_ptr &&processor, { std::vector> processors; processors.emplace_back(std::move(processor)); - context_ = std::make_shared(std::move(processors), std::move(resource)); + context_ = std::make_shared(std::move(processors), resource); OTEL_INTERNAL_LOG_DEBUG("[LoggerProvider] LoggerProvider created."); } LoggerProvider::LoggerProvider(std::vector> &&processors, const opentelemetry::sdk::resource::Resource &resource) noexcept - : context_{std::make_shared(std::move(processors), std::move(resource))} + : context_{std::make_shared(std::move(processors), resource)} {} LoggerProvider::LoggerProvider() noexcept diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index 0380015234..c0ede66923 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -29,7 +29,7 @@ LongLastValueAggregation::LongLastValueAggregation() } LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) - : point_data_{std::move(data)} + : point_data_{data} {} LongLastValueAggregation::LongLastValueAggregation(const LastValuePointData &data) @@ -51,12 +51,12 @@ std::unique_ptr LongLastValueAggregation::Merge( if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch()) { - LastValuePointData merge_data = std::move(nostd::get(ToPoint())); + LastValuePointData merge_data = nostd::get(ToPoint()); return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); } else { - LastValuePointData merge_data = std::move(nostd::get(delta.ToPoint())); + LastValuePointData merge_data = nostd::get(delta.ToPoint()); return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); } } @@ -66,12 +66,12 @@ std::unique_ptr LongLastValueAggregation::Diff(const Aggregation &n if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > nostd::get(next.ToPoint()).sample_ts_.time_since_epoch()) { - LastValuePointData diff_data = std::move(nostd::get(ToPoint())); + LastValuePointData diff_data = nostd::get(ToPoint()); return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); } else { - LastValuePointData diff_data = std::move(nostd::get(next.ToPoint())); + LastValuePointData diff_data = nostd::get(next.ToPoint()); return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); } } @@ -89,7 +89,7 @@ DoubleLastValueAggregation::DoubleLastValueAggregation() } DoubleLastValueAggregation::DoubleLastValueAggregation(LastValuePointData &&data) - : point_data_{std::move(data)} + : point_data_{data} {} DoubleLastValueAggregation::DoubleLastValueAggregation(const LastValuePointData &data) @@ -111,12 +111,12 @@ std::unique_ptr DoubleLastValueAggregation::Merge( if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch()) { - LastValuePointData merge_data = std::move(nostd::get(ToPoint())); + LastValuePointData merge_data = nostd::get(ToPoint()); return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); } else { - LastValuePointData merge_data = std::move(nostd::get(delta.ToPoint())); + LastValuePointData merge_data = nostd::get(delta.ToPoint()); return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); } } @@ -127,12 +127,12 @@ std::unique_ptr DoubleLastValueAggregation::Diff( if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > nostd::get(next.ToPoint()).sample_ts_.time_since_epoch()) { - LastValuePointData diff_data = std::move(nostd::get(ToPoint())); + LastValuePointData diff_data = nostd::get(ToPoint()); return std::unique_ptr(new DoubleLastValueAggregation(std::move(diff_data))); } else { - LastValuePointData diff_data = std::move(nostd::get(next.ToPoint())); + LastValuePointData diff_data = nostd::get(next.ToPoint()); return std::unique_ptr(new DoubleLastValueAggregation(std::move(diff_data))); } } diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index 1e0442e92c..ce0d4c5de5 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -27,7 +27,7 @@ LongSumAggregation::LongSumAggregation(bool is_monotonic) point_data_.is_monotonic_ = is_monotonic; } -LongSumAggregation::LongSumAggregation(SumPointData &&data) : point_data_{std::move(data)} {} +LongSumAggregation::LongSumAggregation(SumPointData &&data) : point_data_{data} {} LongSumAggregation::LongSumAggregation(const SumPointData &data) : point_data_{data} {} @@ -81,7 +81,7 @@ DoubleSumAggregation::DoubleSumAggregation(bool is_monotonic) point_data_.is_monotonic_ = is_monotonic; } -DoubleSumAggregation::DoubleSumAggregation(SumPointData &&data) : point_data_(std::move(data)) {} +DoubleSumAggregation::DoubleSumAggregation(SumPointData &&data) : point_data_(data) {} DoubleSumAggregation::DoubleSumAggregation(const SumPointData &data) : point_data_(data) {} diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc index fe736e64f3..ad6ea27821 100644 --- a/sdk/src/metrics/state/sync_metric_storage.cc +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -39,7 +39,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, } return temporal_metric_storage_.buildMetrics(collector, collectors, sdk_start_ts, collection_ts, - std::move(delta_metrics), callback); + delta_metrics, callback); } } // namespace metrics diff --git a/sdk/src/trace/samplers/parent_factory.cc b/sdk/src/trace/samplers/parent_factory.cc index 7f1bc088d6..63a7eff1c4 100644 --- a/sdk/src/trace/samplers/parent_factory.cc +++ b/sdk/src/trace/samplers/parent_factory.cc @@ -16,7 +16,7 @@ namespace trace std::unique_ptr ParentBasedSamplerFactory::Create( const std::shared_ptr &delegate_sampler) { - std::unique_ptr sampler(new ParentBasedSampler(std::move(delegate_sampler))); + std::unique_ptr sampler(new ParentBasedSampler(delegate_sampler)); return sampler; } diff --git a/sdk/test/common/random_test.cc b/sdk/test/common/random_test.cc index 243a998041..f6d0a39cd7 100644 --- a/sdk/test/common/random_test.cc +++ b/sdk/test/common/random_test.cc @@ -51,7 +51,8 @@ TEST(RandomTest, AtomicFlagMultiThreadTest) { std::vector threads; std::atomic_uint count(0); - for (int i = 0; i < 10; ++i) + threads.reserve(10); +for (int i = 0; i < 10; ++i) { threads.push_back(std::thread(doSomethingOnce, &count)); } diff --git a/sdk/test/metrics/periodic_exporting_metric_reader_test.cc b/sdk/test/metrics/periodic_exporting_metric_reader_test.cc index f65c10ca05..ceb668ca25 100644 --- a/sdk/test/metrics/periodic_exporting_metric_reader_test.cc +++ b/sdk/test/metrics/periodic_exporting_metric_reader_test.cc @@ -51,7 +51,7 @@ class MockMetricProducer : public MetricProducer { public: MockMetricProducer(std::chrono::microseconds sleep_ms = std::chrono::microseconds::zero()) - : sleep_ms_{sleep_ms}, data_sent_size_(0) + : sleep_ms_{sleep_ms} {} bool Collect(nostd::function_ref callback) noexcept override @@ -67,7 +67,7 @@ class MockMetricProducer : public MetricProducer private: std::chrono::microseconds sleep_ms_; - size_t data_sent_size_; + size_t data_sent_size_{0}; }; TEST(PeriodicExporingMetricReader, BasicTests) From eea65083ca7a007b96d4e6f5ed85506e086923a2 Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sat, 27 Jul 2024 11:28:57 -0400 Subject: [PATCH 02/10] Clang-tidy clean up 2 --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index 9a30fbef54..2972e8a81b 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -31,6 +31,7 @@ Checks: > -misc-unused-alias-decls, -misc-use-anonymous-namespace, cppcoreguidelines-*, + -cppcoreguidelines-avoid-do-while, -cppcoreguidelines-avoid-c-arrays, -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-init-variables, From 21a22ad175e5f007bf667abe9432e8dcbad7f1fb Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sat, 27 Jul 2024 14:07:24 -0400 Subject: [PATCH 03/10] Clang-tidy clean up 2 --- examples/http/server.cc | 2 +- exporters/otlp/src/otlp_file_client.cc | 2 +- .../src/otlp_file_metric_exporter_options.cc | 5 ++- .../src/otlp_grpc_metric_exporter_options.cc | 5 ++- .../otlp/src/otlp_http_exporter_options.cc | 18 ++++++----- .../otlp_http_log_record_exporter_options.cc | 31 +++++++++--------- .../src/otlp_http_metric_exporter_options.cc | 32 ++++++++++--------- exporters/prometheus/test/collector_test.cc | 2 +- exporters/zipkin/src/zipkin_exporter.cc | 2 +- ext/src/http/client/curl/http_client_curl.cc | 7 ++-- ext/test/http/curl_http_test.cc | 5 ++- .../aggregation/lastvalue_aggregation.cc | 4 +-- sdk/test/common/random_test.cc | 2 +- .../periodic_exporting_metric_reader_test.cc | 2 +- 14 files changed, 59 insertions(+), 60 deletions(-) diff --git a/examples/http/server.cc b/examples/http/server.cc index 2a5bcb615f..85cccc234d 100644 --- a/examples/http/server.cc +++ b/examples/http/server.cc @@ -22,7 +22,7 @@ class RequestHandler : public HTTP_SERVER_NS::HttpRequestCallback { public: int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, - HTTP_SERVER_NS::HttpResponse &response) override + HTTP_SERVER_NS::HttpResponse &response) override { StartSpanOptions options; options.kind = SpanKind::kServer; // server diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index 8e34dcb567..652ad6dfd2 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -966,7 +966,7 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender { public: explicit OtlpFileSystemBackend(const OtlpFileClientFileSystemOptions &options) - : options_(options), is_initialized_{false} + : options_(options), is_initialized_{false} { file_ = std::make_shared(); file_->is_shutdown.store(false); diff --git a/exporters/otlp/src/otlp_file_metric_exporter_options.cc b/exporters/otlp/src/otlp_file_metric_exporter_options.cc index 5b27c0a8b8..e1b1f5ca7a 100644 --- a/exporters/otlp/src/otlp_file_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_file_metric_exporter_options.cc @@ -12,7 +12,8 @@ namespace exporter namespace otlp { -OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() : aggregation_temporality(PreferredAggregationTemporality::kCumulative) +OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() + : aggregation_temporality(PreferredAggregationTemporality::kCumulative) { console_debug = false; @@ -25,8 +26,6 @@ OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() : aggregation_tem fs_options.rotate_size = 10; backend_options = fs_options; - - } OtlpFileMetricExporterOptions::~OtlpFileMetricExporterOptions() {} diff --git a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc index 9b9ee188ca..0d84ec276f 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc @@ -10,7 +10,8 @@ namespace exporter namespace otlp { -OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() : aggregation_temporality(PreferredAggregationTemporality::kCumulative) +OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() + : aggregation_temporality(PreferredAggregationTemporality::kCumulative) { endpoint = GetOtlpDefaultGrpcMetricsEndpoint(); use_ssl_credentials = !GetOtlpDefaultGrpcMetricsIsInsecure(); /* negation intended. */ @@ -28,8 +29,6 @@ OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() : aggregation_tem metadata = GetOtlpDefaultMetricsHeaders(); user_agent = GetOtlpDefaultUserAgent(); - - max_threads = 0; compression = GetOtlpDefaultMetricsCompression(); diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index c02ae11ce1..9d55110deb 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -15,15 +15,17 @@ namespace exporter namespace otlp { -OtlpHttpExporterOptions::OtlpHttpExporterOptions() : json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), ssl_insecure_skip_verify(false) +OtlpHttpExporterOptions::OtlpHttpExporterOptions() + : json_bytes_mapping(JsonBytesMappingKind::kHexId), + use_json_name(false), + console_debug(false), + ssl_insecure_skip_verify(false) { - url = GetOtlpDefaultHttpTracesEndpoint(); - content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol()); - - - - timeout = GetOtlpDefaultTracesTimeout(); - http_headers = GetOtlpDefaultTracesHeaders(); + url = GetOtlpDefaultHttpTracesEndpoint(); + content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol()); + + timeout = GetOtlpDefaultTracesTimeout(); + http_headers = GetOtlpDefaultTracesHeaders(); #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; 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 b1e007b9c6..f193ea10df 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -15,28 +15,29 @@ namespace exporter namespace otlp { -OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() : json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), max_concurrent_requests(64), max_requests_per_connection(8), ssl_insecure_skip_verify(false) +OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() + : json_bytes_mapping(JsonBytesMappingKind::kHexId), + use_json_name(false), + console_debug(false), + ssl_insecure_skip_verify(false) { - url = GetOtlpDefaultHttpLogsEndpoint(); - content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpLogsProtocol()); - - - - timeout = GetOtlpDefaultLogsTimeout(); - http_headers = GetOtlpDefaultLogsHeaders(); + 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_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(); diff --git a/exporters/otlp/src/otlp_http_metric_exporter_options.cc b/exporters/otlp/src/otlp_http_metric_exporter_options.cc index 48771f40e3..95a8dd885c 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -16,28 +16,30 @@ namespace exporter namespace otlp { -OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() : json_bytes_mapping(JsonBytesMappingKind::kHexId), use_json_name(false), console_debug(false), aggregation_temporality(PreferredAggregationTemporality::kCumulative), ssl_insecure_skip_verify(false) +OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() + : json_bytes_mapping(JsonBytesMappingKind::kHexId), + use_json_name(false), + console_debug(false), + aggregation_temporality(PreferredAggregationTemporality::kCumulative), + ssl_insecure_skip_verify(false) { - url = GetOtlpDefaultMetricsEndpoint(); - content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpMetricsProtocol()); - - - - timeout = GetOtlpDefaultMetricsTimeout(); - http_headers = GetOtlpDefaultMetricsHeaders(); - + 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_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(); diff --git a/exporters/prometheus/test/collector_test.cc b/exporters/prometheus/test/collector_test.cc index d49efb94ab..a70fd317b3 100644 --- a/exporters/prometheus/test/collector_test.cc +++ b/exporters/prometheus/test/collector_test.cc @@ -23,7 +23,7 @@ class MockMetricProducer : public opentelemetry::sdk::metrics::MetricProducer public: MockMetricProducer(std::chrono::microseconds sleep_ms = std::chrono::microseconds::zero()) - : sleep_ms_{sleep_ms} + : sleep_ms_{sleep_ms} {} bool Collect(nostd::function_ref callback) noexcept override diff --git a/exporters/zipkin/src/zipkin_exporter.cc b/exporters/zipkin/src/zipkin_exporter.cc index 2be9c02b74..bd30f80c3e 100644 --- a/exporters/zipkin/src/zipkin_exporter.cc +++ b/exporters/zipkin/src/zipkin_exporter.cc @@ -52,7 +52,7 @@ ZipkinExporter::ZipkinExporter( std::shared_ptr http_client) : options_(ZipkinExporterOptions()), http_client_(http_client), url_parser_(options_.endpoint) { - + InitializeLocalEndpoint(); } diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index c3e0119029..22f20fb0e0 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -185,13 +185,12 @@ void Session::FinishOperation() } HttpClient::HttpClient() - : multi_handle_(curl_multi_init()), next_session_id_{0}, + : multi_handle_(curl_multi_init()), + next_session_id_{0}, max_sessions_per_connection_{8}, scheduled_delay_milliseconds_{std::chrono::milliseconds(256)}, curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance()) -{ - -} +{} HttpClient::~HttpClient() { diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index f0591d1d5d..d5351d2314 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -30,8 +30,7 @@ class CustomEventHandler : public http_client::EventHandler { got_response_ = true; } - void OnEvent(http_client::SessionState state, - nostd::string_view /* reason */) noexcept override + void OnEvent(http_client::SessionState state, nostd::string_view /* reason */) noexcept override { switch (state) { @@ -141,7 +140,7 @@ class BasicCurlHttpTests : public ::testing::Test, public HTTP_SERVER_NS::HttpRe } int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, - HTTP_SERVER_NS::HttpResponse &response) override + HTTP_SERVER_NS::HttpResponse &response) override { int response_status = 404; if (request.uri == "/get/") diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index c0ede66923..2bc0eddb0e 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -28,9 +28,7 @@ LongLastValueAggregation::LongLastValueAggregation() point_data_.value_ = static_cast(0); } -LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) - : point_data_{data} -{} +LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) : point_data_{data} {} LongLastValueAggregation::LongLastValueAggregation(const LastValuePointData &data) : point_data_{data} diff --git a/sdk/test/common/random_test.cc b/sdk/test/common/random_test.cc index f6d0a39cd7..e22e0508fc 100644 --- a/sdk/test/common/random_test.cc +++ b/sdk/test/common/random_test.cc @@ -52,7 +52,7 @@ TEST(RandomTest, AtomicFlagMultiThreadTest) std::vector threads; std::atomic_uint count(0); threads.reserve(10); -for (int i = 0; i < 10; ++i) + for (int i = 0; i < 10; ++i) { threads.push_back(std::thread(doSomethingOnce, &count)); } diff --git a/sdk/test/metrics/periodic_exporting_metric_reader_test.cc b/sdk/test/metrics/periodic_exporting_metric_reader_test.cc index ceb668ca25..277a20582f 100644 --- a/sdk/test/metrics/periodic_exporting_metric_reader_test.cc +++ b/sdk/test/metrics/periodic_exporting_metric_reader_test.cc @@ -51,7 +51,7 @@ class MockMetricProducer : public MetricProducer { public: MockMetricProducer(std::chrono::microseconds sleep_ms = std::chrono::microseconds::zero()) - : sleep_ms_{sleep_ms} + : sleep_ms_{sleep_ms} {} bool Collect(nostd::function_ref callback) noexcept override From 6456644d5691f20bd8c32e65c629c7ef58cb9c8e Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sat, 27 Jul 2024 23:28:56 -0400 Subject: [PATCH 04/10] Clang-tidy clean up 2 --- exporters/otlp/src/otlp_file_client.cc | 2 +- exporters/otlp/src/otlp_http_client.cc | 2 +- .../otlp/src/otlp_populate_attribute_utils.cc | 15 ++++++++++----- exporters/prometheus/src/exporter_utils.cc | 10 +++++----- exporters/zipkin/src/zipkin_exporter.cc | 4 +++- .../opentelemetry/sdk/common/circular_buffer.h | 1 - sdk/src/trace/batch_span_processor.cc | 2 +- 7 files changed, 21 insertions(+), 15 deletions(-) diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index 652ad6dfd2..bdd6eac27d 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -1568,7 +1568,7 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileOstreamBackend : public OtlpFileAppende }; OtlpFileClient::OtlpFileClient(OtlpFileClientOptions &&options) - : is_shutdown_(false), options_(options) + : is_shutdown_(false), options_(std::move(options)) { if (nostd::holds_alternative(options_.backend_options)) { diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index 07b9108a3a..876719fa1e 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -707,7 +707,7 @@ OtlpHttpClient::~OtlpHttpClient() OtlpHttpClient::OtlpHttpClient(OtlpHttpClientOptions &&options, std::shared_ptr http_client) : is_shutdown_(false), - options_(options), + options_(std::move(options)), http_client_(std::move(http_client)), start_session_counter_(0), finished_session_counter_(0) diff --git a/exporters/otlp/src/otlp_populate_attribute_utils.cc b/exporters/otlp/src/otlp_populate_attribute_utils.cc index a2ef2edafa..490e4c771b 100644 --- a/exporters/otlp/src/otlp_populate_attribute_utils.cc +++ b/exporters/otlp/src/otlp_populate_attribute_utils.cc @@ -72,7 +72,8 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( } else if (nostd::holds_alternative(value)) { - proto_value->set_int_value(nostd::get(value)); + proto_value->set_int_value( + nostd::get(value)); // NOLINT(cppcoreguidelines-narrowing-conversions) } else if (nostd::holds_alternative(value)) { @@ -132,7 +133,8 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( auto array_value = proto_value->mutable_array_value(); for (const auto &val : nostd::get>(value)) { - array_value->add_values()->set_int_value(val); + array_value->add_values()->set_int_value( + val); // NOLINT(cppcoreguidelines-narrowing-conversions) } } else if (nostd::holds_alternative>(value)) @@ -186,7 +188,8 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( } else if (nostd::holds_alternative(value)) { - proto_value->set_int_value(nostd::get(value)); + proto_value->set_int_value( + nostd::get(value)); // NOLINT(cppcoreguidelines-narrowing-conversions) } else if (nostd::holds_alternative(value)) { @@ -217,7 +220,8 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( auto array_value = proto_value->mutable_array_value(); for (const auto &val : nostd::get>(value)) { - array_value->add_values()->set_int_value(val); + array_value->add_values()->set_int_value( + val); // NOLINT(cppcoreguidelines-narrowing-conversions) } } else if (nostd::holds_alternative>(value)) @@ -233,7 +237,8 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( auto array_value = proto_value->mutable_array_value(); for (const auto &val : nostd::get>(value)) { - array_value->add_values()->set_int_value(val); + array_value->add_values()->set_int_value( + val); // NOLINT(cppcoreguidelines-narrowing-conversions) } } else if (nostd::holds_alternative>(value)) diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index 35ee748273..77fcd56ad2 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -284,11 +284,11 @@ std::string PrometheusExporterUtils::SanitizeNames(std::string name) } #if OPENTELEMETRY_HAVE_WORKING_REGEX -std::regex INVALID_CHARACTERS_PATTERN("[^a-zA-Z0-9]"); -std::regex CHARACTERS_BETWEEN_BRACES_PATTERN("\\{(.*?)\\}"); -std::regex SANITIZE_LEADING_UNDERSCORES("^_+"); -std::regex SANITIZE_TRAILING_UNDERSCORES("_+$"); -std::regex SANITIZE_CONSECUTIVE_UNDERSCORES("[_]{2,}"); +const std::regex INVALID_CHARACTERS_PATTERN("[^a-zA-Z0-9]"); +const std::regex CHARACTERS_BETWEEN_BRACES_PATTERN("\\{(.*?)\\}"); +const std::regex SANITIZE_LEADING_UNDERSCORES("^_+"); +const std::regex SANITIZE_TRAILING_UNDERSCORES("_+$"); +const std::regex SANITIZE_CONSECUTIVE_UNDERSCORES("[_]{2,}"); #endif std::string PrometheusExporterUtils::GetEquivalentPrometheusUnit( diff --git a/exporters/zipkin/src/zipkin_exporter.cc b/exporters/zipkin/src/zipkin_exporter.cc index bd30f80c3e..4c5723c674 100644 --- a/exporters/zipkin/src/zipkin_exporter.cc +++ b/exporters/zipkin/src/zipkin_exporter.cc @@ -50,7 +50,9 @@ ZipkinExporter::ZipkinExporter() : options_(ZipkinExporterOptions()), url_parser ZipkinExporter::ZipkinExporter( std::shared_ptr http_client) - : options_(ZipkinExporterOptions()), http_client_(http_client), url_parser_(options_.endpoint) + : options_(ZipkinExporterOptions()), + http_client_(std::move(http_client)), + url_parser_(options_.endpoint) { InitializeLocalEndpoint(); diff --git a/sdk/include/opentelemetry/sdk/common/circular_buffer.h b/sdk/include/opentelemetry/sdk/common/circular_buffer.h index c2548012b1..6541612158 100644 --- a/sdk/include/opentelemetry/sdk/common/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/common/circular_buffer.h @@ -113,7 +113,6 @@ class CircularBuffer data_[head_index].Swap(ptr); } } - return true; } bool Add(std::unique_ptr &&ptr) noexcept diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index 6aa6a49949..f0f55d20e8 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -66,7 +66,7 @@ void BatchSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept return; } - if (buffer_.Add(span) == false) + if (buffer_.Add(std::move(span)) == false) { OTEL_INTERNAL_LOG_WARN("BatchSpanProcessor queue is full - dropping span."); return; From 5dfa576b767fa9162a8b33f6b9dc50c390a372ae Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sun, 28 Jul 2024 00:32:53 -0400 Subject: [PATCH 05/10] Clang-tidy clean up 2 --- .clang-tidy | 2 ++ api/test/logs/logger_benchmark.cc | 2 +- sdk/src/common/base64.cc | 7 +------ 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 2972e8a81b..db61b810c9 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -31,10 +31,12 @@ Checks: > -misc-unused-alias-decls, -misc-use-anonymous-namespace, cppcoreguidelines-*, + -cppcoreguidelines-owning-memory, -cppcoreguidelines-avoid-do-while, -cppcoreguidelines-avoid-c-arrays, -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-init-variables, -cppcoreguidelines-macro-usage, -cppcoreguidelines-non-private-member-variables-in-classes, + -cppcoreguidelines-avoid-non-const-global-variables, -cppcoreguidelines-pro-* \ No newline at end of file diff --git a/api/test/logs/logger_benchmark.cc b/api/test/logs/logger_benchmark.cc index ff333511b4..b3eb4d2981 100644 --- a/api/test/logs/logger_benchmark.cc +++ b/api/test/logs/logger_benchmark.cc @@ -84,7 +84,7 @@ static void ThreadRoutine(Barrier &barrier, void MultiThreadRunner(benchmark::State &state, const std::function &func) { - int num_threads = std::thread::hardware_concurrency(); + uint num_threads = std::thread::hardware_concurrency(); Barrier barrier(num_threads); diff --git a/sdk/src/common/base64.cc b/sdk/src/common/base64.cc index 3c572fe344..a78b88ad68 100644 --- a/sdk/src/common/base64.cc +++ b/sdk/src/common/base64.cc @@ -200,15 +200,10 @@ static int Base64UnescapeInternal(unsigned char *dst, ++line_len; if (src[i] == padding_char) { - if (++j > 2) + if (++j > 2 || (valid_slen & 3) == 1 || (valid_slen & 3) == 2) { return -2; } - else if ((valid_slen & 3) == 1 || (valid_slen & 3) == 2) - { - // First and second char of every group can not be padding char - return -2; - } } else { From 2f6401ca693d3973b4cbc8b90ba28751232e4a22 Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sun, 28 Jul 2024 01:35:21 -0400 Subject: [PATCH 06/10] Clang-tidy clean up 2 --- api/test/logs/logger_benchmark.cc | 2 +- exporters/otlp/src/otlp_http_exporter_options.cc | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/test/logs/logger_benchmark.cc b/api/test/logs/logger_benchmark.cc index b3eb4d2981..c81c81b11b 100644 --- a/api/test/logs/logger_benchmark.cc +++ b/api/test/logs/logger_benchmark.cc @@ -91,7 +91,7 @@ void MultiThreadRunner(benchmark::State &state, const std::function &fun std::vector threads; threads.reserve(num_threads); - for (int i = 0; i < num_threads; i++) + for (uint i = 0; i < num_threads; i++) { threads.emplace_back(ThreadRoutine, std::ref(barrier), std::ref(state), i, func); } diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 9d55110deb..6dea2ee05f 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -32,7 +32,6 @@ OtlpHttpExporterOptions::OtlpHttpExporterOptions() max_requests_per_connection = 8; #endif /* ENABLE_ASYNC_EXPORT */ - ssl_insecure_skip_verify = false; ssl_ca_cert_path = GetOtlpDefaultTracesSslCertificatePath(); ssl_ca_cert_string = GetOtlpDefaultTracesSslCertificateString(); ssl_client_key_path = GetOtlpDefaultTracesSslClientKeyPath(); From a421c617c895e87c368a5e349098c48f98e998e2 Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sun, 28 Jul 2024 01:42:53 -0400 Subject: [PATCH 07/10] Clang-tidy clean up 2 --- exporters/otlp/src/otlp_http_exporter_options.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 6dea2ee05f..0b00fc9708 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -32,12 +32,12 @@ OtlpHttpExporterOptions::OtlpHttpExporterOptions() 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_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(); From 8fd5bfac858a6c402e690bd5e8e23434d7331c72 Mon Sep 17 00:00:00 2001 From: msiddhu Date: Wed, 21 Aug 2024 00:34:21 +0000 Subject: [PATCH 08/10] Added uint32_t instead of uint --- api/test/logs/logger_benchmark.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/test/logs/logger_benchmark.cc b/api/test/logs/logger_benchmark.cc index c81c81b11b..b14e7711b4 100644 --- a/api/test/logs/logger_benchmark.cc +++ b/api/test/logs/logger_benchmark.cc @@ -84,14 +84,14 @@ static void ThreadRoutine(Barrier &barrier, void MultiThreadRunner(benchmark::State &state, const std::function &func) { - uint num_threads = std::thread::hardware_concurrency(); + uint32_t num_threads = std::thread::hardware_concurrency(); Barrier barrier(num_threads); std::vector threads; threads.reserve(num_threads); - for (uint i = 0; i < num_threads; i++) + for (uint32_t i = 0; i < num_threads; i++) { threads.emplace_back(ThreadRoutine, std::ref(barrier), std::ref(state), i, func); } From 9c6de8a16eca5c9726d6507941157c2c5c2b21b1 Mon Sep 17 00:00:00 2001 From: msiddhu Date: Wed, 21 Aug 2024 02:34:43 +0000 Subject: [PATCH 09/10] Added uint32_t instead of uin --- .github/workflows/clang-tidy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index 9e5f6c26db..612ff05270 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -68,7 +68,7 @@ jobs: - name: Run clang-tidy run: | cd build - make -j$(nproc) 2>&1 | tee -a clang-tidy.log || exit 1 + make 2>&1 | tee -a clang-tidy.log || exit 1 - uses: actions/upload-artifact@v4 with: From c8f61192b33f7fb8ca2a61aea9845571a7344b31 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 22 Aug 2024 11:31:54 +0200 Subject: [PATCH 10/10] Update api/test/logs/logger_benchmark.cc --- api/test/logs/logger_benchmark.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/logs/logger_benchmark.cc b/api/test/logs/logger_benchmark.cc index b14e7711b4..7e8913a3f7 100644 --- a/api/test/logs/logger_benchmark.cc +++ b/api/test/logs/logger_benchmark.cc @@ -84,7 +84,7 @@ static void ThreadRoutine(Barrier &barrier, void MultiThreadRunner(benchmark::State &state, const std::function &func) { - uint32_t num_threads = std::thread::hardware_concurrency(); + uint32_t num_threads = std::thread::hardware_concurrency(); Barrier barrier(num_threads);