From a71642fe678f90ba6d379d699e533ce441394bdf Mon Sep 17 00:00:00 2001 From: Siddhartha Malladi Date: Sun, 25 Aug 2024 23:28:32 +0530 Subject: [PATCH] [Code Health] Clang Tidy cleanup, Part 2 (#3038) --- .clang-tidy | 3 ++ .github/workflows/clang-tidy.yaml | 2 +- .../propagation/baggage_propagator_test.cc | 4 +-- .../propagation/composite_propagator_test.cc | 6 ++-- api/test/logs/logger_benchmark.cc | 8 ++--- 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 | 4 +-- exporters/otlp/src/otlp_file_client.cc | 6 ++-- .../src/otlp_file_metric_exporter_options.cc | 3 +- .../src/otlp_grpc_metric_exporter_options.cc | 3 +- exporters/otlp/src/otlp_http_client.cc | 2 +- .../otlp/src/otlp_http_exporter_options.cc | 29 ++++++++--------- .../otlp_http_log_record_exporter_options.cc | 29 ++++++++--------- .../src/otlp_http_metric_exporter_options.cc | 31 ++++++++++--------- .../otlp/src/otlp_populate_attribute_utils.cc | 15 ++++++--- exporters/prometheus/src/exporter_utils.cc | 10 +++--- exporters/prometheus/test/collector_test.cc | 4 +-- exporters/zipkin/src/zipkin_exporter.cc | 6 ++-- ext/src/http/client/curl/http_client_curl.cc | 7 ++--- ext/test/http/curl_http_test.cc | 13 ++++---- ext/test/w3c_tracecontext_test/main.cc | 4 +-- .../sdk/common/circular_buffer.h | 1 - sdk/src/common/base64.cc | 7 +---- sdk/src/logs/logger_provider.cc | 4 +-- .../aggregation/lastvalue_aggregation.cc | 22 ++++++------- .../metrics/aggregation/sum_aggregation.cc | 4 +-- sdk/src/metrics/state/sync_metric_storage.cc | 2 +- sdk/src/trace/batch_span_processor.cc | 2 +- sdk/src/trace/samplers/parent_factory.cc | 2 +- sdk/test/common/random_test.cc | 1 + .../periodic_exporting_metric_reader_test.cc | 4 +-- 34 files changed, 127 insertions(+), 125 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 9a30fbef54..db61b810c9 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -31,9 +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/.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: 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..7e8913a3f7 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, @@ -84,14 +84,14 @@ static void ThreadRoutine(Barrier &barrier, void MultiThreadRunner(benchmark::State &state, const std::function &func) { - int 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 (int 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); } 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..85cccc234d 100644 --- a/examples/http/server.cc +++ b/examples/http/server.cc @@ -21,8 +21,8 @@ constexpr const char *server_name = "localhost"; class RequestHandler : public HTTP_SERVER_NS::HttpRequestCallback { public: - virtual int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, - HTTP_SERVER_NS::HttpResponse &response) override + int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, + 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 cc6e074bfd..bdd6eac27d 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 @@ -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_file_metric_exporter_options.cc b/exporters/otlp/src/otlp_file_metric_exporter_options.cc index 3ad8eaa457..e1b1f5ca7a 100644 --- a/exporters/otlp/src/otlp_file_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_file_metric_exporter_options.cc @@ -13,6 +13,7 @@ namespace otlp { OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() + : aggregation_temporality(PreferredAggregationTemporality::kCumulative) { console_debug = false; @@ -25,8 +26,6 @@ OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions() fs_options.rotate_size = 10; 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..0d84ec276f 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc @@ -11,6 +11,7 @@ namespace otlp { OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() + : aggregation_temporality(PreferredAggregationTemporality::kCumulative) { endpoint = GetOtlpDefaultGrpcMetricsEndpoint(); use_ssl_credentials = !GetOtlpDefaultGrpcMetricsIsInsecure(); /* negation intended. */ @@ -28,8 +29,6 @@ OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() metadata = GetOtlpDefaultMetricsHeaders(); user_agent = GetOtlpDefaultUserAgent(); - aggregation_temporality = PreferredAggregationTemporality::kCumulative; - max_threads = 0; compression = GetOtlpDefaultMetricsCompression(); 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_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index 753efcb481..0b00fc9708 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -16,27 +16,28 @@ namespace otlp { 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(); + 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_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_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(); 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..f193ea10df 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -16,27 +16,28 @@ namespace otlp { 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()); - json_bytes_mapping = JsonBytesMappingKind::kHexId; - use_json_name = false; - console_debug = false; - 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_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_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 66d65fa54b..95a8dd885c 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -17,28 +17,29 @@ 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) { - 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; + 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_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_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/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/prometheus/test/collector_test.cc b/exporters/prometheus/test/collector_test.cc index 63f840bc9a..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}, 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..4c5723c674 100644 --- a/exporters/zipkin/src/zipkin_exporter.cc +++ b/exporters/zipkin/src/zipkin_exporter.cc @@ -50,9 +50,11 @@ ZipkinExporter::ZipkinExporter() : options_(ZipkinExporterOptions()), url_parser ZipkinExporter::ZipkinExporter( std::shared_ptr http_client) - : options_(ZipkinExporterOptions()), url_parser_(options_.endpoint) + : options_(ZipkinExporterOptions()), + http_client_(std::move(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..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() - : 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..d5351d2314 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -26,12 +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, - nostd::string_view /* reason */) noexcept override + void OnEvent(http_client::SessionState state, nostd::string_view /* reason */) noexcept override { switch (state) { @@ -113,7 +112,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 +131,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,8 +139,8 @@ class BasicCurlHttpTests : public ::testing::Test, public HTTP_SERVER_NS::HttpRe is_running_ = false; } - virtual int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, - HTTP_SERVER_NS::HttpResponse &response) override + int onHttpRequest(HTTP_SERVER_NS::HttpRequest const &request, + HTTP_SERVER_NS::HttpResponse &response) override { int response_status = 404; if (request.uri == "/get/") 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/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/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 { 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..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_{std::move(data)} -{} +LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) : point_data_{data} {} LongLastValueAggregation::LongLastValueAggregation(const LastValuePointData &data) : point_data_{data} @@ -51,12 +49,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 +64,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 +87,7 @@ DoubleLastValueAggregation::DoubleLastValueAggregation() } DoubleLastValueAggregation::DoubleLastValueAggregation(LastValuePointData &&data) - : point_data_{std::move(data)} + : point_data_{data} {} DoubleLastValueAggregation::DoubleLastValueAggregation(const LastValuePointData &data) @@ -111,12 +109,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 +125,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/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; 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..e22e0508fc 100644 --- a/sdk/test/common/random_test.cc +++ b/sdk/test/common/random_test.cc @@ -51,6 +51,7 @@ TEST(RandomTest, AtomicFlagMultiThreadTest) { std::vector threads; std::atomic_uint count(0); + 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..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}, 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)