diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index 3e1b6c3afc..434d6b456e 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -17,9 +17,9 @@ jobs: matrix: include: - cmake_options: all-options-abiv1-preview - warning_limit: 86 + warning_limit: 62 - cmake_options: all-options-abiv2-preview - warning_limit: 86 + warning_limit: 62 steps: - name: Harden the runner (Audit all outbound calls) uses: step-security/harden-runner@002fdce3c6a235733a90a27c80493a3241e56863 # v2.12.1 diff --git a/CHANGELOG.md b/CHANGELOG.md index dcff620345..d5f020d4d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,10 @@ Increment the: [#3496](https://github.com/open-telemetry/opentelemetry-cpp/pull/3496) * [CodeHealth] Fix clang-tidy warnings part 3 - [#3496](https://github.com/open-telemetry/opentelemetry-cpp/pull/3498) + [#3498](https://github.com/open-telemetry/opentelemetry-cpp/pull/3498) + +* [CodeHealth] Fix clang-tidy warnings part 4 + [#3501](https://github.com/open-telemetry/opentelemetry-cpp/pull/3501) Important changes: diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index d2994abfc4..7a75018e13 100644 --- a/exporters/otlp/src/otlp_http_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_exporter_options.cc @@ -21,6 +21,10 @@ OtlpHttpExporterOptions::OtlpHttpExporterOptions() console_debug(false), timeout(GetOtlpDefaultTracesTimeout()), http_headers(GetOtlpDefaultTracesHeaders()), +#ifdef ENABLE_ASYNC_EXPORT + max_concurrent_requests{64}, + max_requests_per_connection{8}, +#endif ssl_insecure_skip_verify(false), ssl_ca_cert_path(GetOtlpDefaultTracesSslCertificatePath()), ssl_ca_cert_string(GetOtlpDefaultTracesSslCertificateString()), @@ -37,12 +41,7 @@ OtlpHttpExporterOptions::OtlpHttpExporterOptions() retry_policy_initial_backoff(GetOtlpDefaultTracesRetryInitialBackoff()), retry_policy_max_backoff(GetOtlpDefaultTracesRetryMaxBackoff()), retry_policy_backoff_multiplier(GetOtlpDefaultTracesRetryBackoffMultiplier()) -{ -#ifdef ENABLE_ASYNC_EXPORT - max_concurrent_requests = 64; - max_requests_per_connection = 8; -#endif /* ENABLE_ASYNC_EXPORT */ -} +{} 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 69515cf330..212bc3f6cd 100644 --- a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc @@ -21,6 +21,10 @@ OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() console_debug(false), 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()), @@ -37,12 +41,7 @@ OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions() retry_policy_initial_backoff(GetOtlpDefaultLogsRetryInitialBackoff()), retry_policy_max_backoff(GetOtlpDefaultLogsRetryMaxBackoff()), retry_policy_backoff_multiplier(GetOtlpDefaultLogsRetryBackoffMultiplier()) -{ -#ifdef ENABLE_ASYNC_EXPORT - max_concurrent_requests = 64; - max_requests_per_connection = 8; -#endif -} +{} 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 8c84132aff..0ccdc0ab94 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter_options.cc @@ -23,6 +23,10 @@ OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() 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()), @@ -39,12 +43,7 @@ OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions() retry_policy_initial_backoff(GetOtlpDefaultMetricsRetryInitialBackoff()), retry_policy_max_backoff(GetOtlpDefaultMetricsRetryMaxBackoff()), retry_policy_backoff_multiplier(GetOtlpDefaultMetricsRetryBackoffMultiplier()) -{ -#ifdef ENABLE_ASYNC_EXPORT - max_concurrent_requests = 64; - max_requests_per_connection = 8; -#endif -} +{} OtlpHttpMetricExporterOptions::~OtlpHttpMetricExporterOptions() {} diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index b1e5e4c9ad..042ae4fe63 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -515,6 +515,7 @@ std::chrono::system_clock::time_point HttpOperation::NextRetryTime() # define HAVE_TLS_VERSION #endif +// NOLINTNEXTLINE(google-runtime-int) static long parse_min_ssl_version(const std::string &version) { #ifdef HAVE_TLS_VERSION @@ -532,6 +533,7 @@ static long parse_min_ssl_version(const std::string &version) return 0; } +// NOLINTNEXTLINE(google-runtime-int) static long parse_max_ssl_version(const std::string &version) { #ifdef HAVE_TLS_VERSION @@ -591,6 +593,7 @@ CURLcode HttpOperation::SetCurlPtrOption(CURLoption option, void *value) return rc; } +// NOLINTNEXTLINE(google-runtime-int) CURLcode HttpOperation::SetCurlLongOption(CURLoption option, long value) { CURLcode rc; @@ -877,8 +880,10 @@ CURLcode HttpOperation::Setup() #ifdef HAVE_TLS_VERSION /* By default, TLSv1.2 or better is required (if we have TLS). */ + // NOLINTNEXTLINE(google-runtime-int) long min_ssl_version = CURL_SSLVERSION_TLSv1_2; #else + // NOLINTNEXTLINE(google-runtime-int) long min_ssl_version = 0; #endif @@ -903,6 +908,7 @@ CURLcode HttpOperation::Setup() * The CURL + openssl library may be more recent than this code, * and support a version we do not know about. */ + // NOLINTNEXTLINE(google-runtime-int) long max_ssl_version = 0; if (!ssl_options_.ssl_max_tls.empty()) @@ -921,6 +927,7 @@ CURLcode HttpOperation::Setup() #endif } + // NOLINTNEXTLINE(google-runtime-int) long version_range = min_ssl_version | max_ssl_version; if (version_range != 0) { @@ -967,6 +974,7 @@ CURLcode HttpOperation::Setup() if (ssl_options_.ssl_insecure_skip_verify) { /* 6 - DO NOT ENFORCE VERIFICATION, This is not secure. */ + // NOLINTNEXTLINE(google-runtime-int) rc = SetCurlLongOption(CURLOPT_USE_SSL, static_cast(CURLUSESSL_NONE)); if (rc != CURLE_OK) { @@ -988,6 +996,7 @@ CURLcode HttpOperation::Setup() else { /* 6 - ENFORCE VERIFICATION */ + // NOLINTNEXTLINE(google-runtime-int) rc = SetCurlLongOption(CURLOPT_USE_SSL, static_cast(CURLUSESSL_ALL)); if (rc != CURLE_OK) { @@ -1042,7 +1051,7 @@ CURLcode HttpOperation::Setup() // TODO: control local port to use // curl_easy_setopt(curl, CURLOPT_LOCALPORT, dcf_port); - + // NOLINTNEXTLINE(google-runtime-int) rc = SetCurlLongOption(CURLOPT_TIMEOUT_MS, static_cast(http_conn_timeout_.count())); if (rc != CURLE_OK) { diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 5337af8a68..fda9669287 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -35,7 +35,7 @@ #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/string_view.h" -#define HTTP_PORT 19000 +constexpr int HTTP_PORT{19000}; namespace curl = opentelemetry::ext::http::client::curl; namespace http_client = opentelemetry::ext::http::client; diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index e2afc6f161..a88bf25a76 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -60,7 +60,7 @@ class HistogramPointData HistogramPointData &operator=(HistogramPointData &&) = default; HistogramPointData(const HistogramPointData &) = default; HistogramPointData() = default; - HistogramPointData(std::vector &boundaries) : boundaries_(boundaries) {} + HistogramPointData(const std::vector &boundaries) : boundaries_(boundaries) {} HistogramPointData &operator=(const HistogramPointData &other) = default; ~HistogramPointData() = default; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 42fd73d89d..c91e86c4a7 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -131,7 +131,7 @@ void Base2ExponentialHistogramAggregation::Aggregate( int64_t value, const PointAttributes & /* attributes */) noexcept { - Aggregate(double(value)); + Aggregate(static_cast(value)); } void Base2ExponentialHistogramAggregation::Aggregate( diff --git a/sdk/test/common/attribute_utils_test.cc b/sdk/test/common/attribute_utils_test.cc index 5b5f0ac5c0..c0bcc9f453 100644 --- a/sdk/test/common/attribute_utils_test.cc +++ b/sdk/test/common/attribute_utils_test.cc @@ -175,7 +175,8 @@ TEST(AttributeMapTest, EqualTo) Attributes attributes_different_size = {{"key0", "some value"}}; // check for the case where the number of attributes is the same but all keys are different - Attributes attributes_different_all = {{"a", "b"}, {"c", "d"}, {"e", 4.0}, {"f", uint8_t(5)}}; + Attributes attributes_different_all = { + {"a", "b"}, {"c", "d"}, {"e", 4.0}, {"f", static_cast(5)}}; auto kv_iterable_different_value = opentelemetry::common::MakeKeyValueIterableView(attributes_different_value); diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 1588ac319c..9216ce3b60 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -254,7 +254,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) // Create a new aggreagte based in point data { - auto point_data = histo_point; + const auto &point_data = histo_point; Base2ExponentialHistogramAggregation scale0_aggr2(point_data); scale0_aggr2.Aggregate(0.0, {}); diff --git a/sdk/test/metrics/instrument_descriptor_test.cc b/sdk/test/metrics/instrument_descriptor_test.cc index ee398a2c0c..f17189355c 100644 --- a/sdk/test/metrics/instrument_descriptor_test.cc +++ b/sdk/test/metrics/instrument_descriptor_test.cc @@ -49,7 +49,7 @@ TEST(InstrumentDescriptorUtilTest, IsDuplicate) InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_different_name)); // not a duplicate - identical instrument - auto instrument_identical = instrument_existing; + const auto &instrument_identical = instrument_existing; EXPECT_FALSE(InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_identical)); // not a duplicate - instrument with same case-insensitive name @@ -87,8 +87,8 @@ TEST(InstrumentDescriptorTest, EqualNameCaseInsensitiveOperator) { // equal by name, description, unit, type and value type InstrumentEqualNameCaseInsensitive equal_operator{}; - auto instrument_existing = CreateInstrumentDescriptor("counter"); - auto instrument_identical = instrument_existing; + auto instrument_existing = CreateInstrumentDescriptor("counter"); + const auto &instrument_identical = instrument_existing; EXPECT_TRUE(equal_operator(instrument_existing, instrument_identical)); // equal by name with different case @@ -127,8 +127,8 @@ TEST(InstrumentDescriptorTest, HashOperator) InstrumentDescriptorHash hash_operator{}; // identical instrument - hash must match - auto instrument_existing = CreateInstrumentDescriptor("counter"); - auto instrument_identical = instrument_existing; + auto instrument_existing = CreateInstrumentDescriptor("counter"); + const auto &instrument_identical = instrument_existing; EXPECT_EQ(hash_operator(instrument_existing), hash_operator(instrument_identical)); // name case conflict - hash must match diff --git a/sdk/test/resource/resource_test.cc b/sdk/test/resource/resource_test.cc index e6f1af244f..696509f892 100644 --- a/sdk/test/resource/resource_test.cc +++ b/sdk/test/resource/resource_test.cc @@ -283,10 +283,10 @@ TEST(ResourceTest, DerivedResourceDetector) { TestResourceDetector detector; - detector.attributes = {{"key", "value"}}; - detector.schema_url = "https://opentelemetry.io/schemas/v3.1.4"; - const auto resource = detector.Detect(); - const auto received_attributes = resource.GetAttributes(); + detector.attributes = {{"key", "value"}}; + detector.schema_url = "https://opentelemetry.io/schemas/v3.1.4"; + const auto resource = detector.Detect(); + const auto &received_attributes = resource.GetAttributes(); EXPECT_EQ(received_attributes.size(), 1); EXPECT_EQ(resource.GetSchemaURL(), detector.schema_url);