diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index 5ffc3abaa2..5a0e0084f7 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: 215 + warning_limit: 172 - cmake_options: all-options-abiv2-preview - warning_limit: 216 + warning_limit: 173 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 c78a874baa..91a0ee72ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ Increment the: * [REMOVAL] Removed deprecated semantic convention header files [#3475](https://github.com/open-telemetry/opentelemetry-cpp/pull/3475) +* [CodeHealth] Fix clang-tidy warnings part 1 + [#3493](https://github.com/open-telemetry/opentelemetry-cpp/pull/3493) + Important changes: * [REMOVAL] Removed deprecated semantic convention header files diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index c6d1ff9565..cbd3f84825 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -117,10 +117,10 @@ int main(int argc, char *argv[]) } } } - std::cout << "Using endpoint: " << exporter_options.endpoint << std::endl; - std::cout << "Using example type: " << example_type << std::endl; - std::cout << "Using cacert path: " << exporter_options.ssl_credentials_cacert_path << std::endl; - std::cout << "Using ssl credentials: " << exporter_options.use_ssl_credentials << std::endl; + std::cout << "Using endpoint: " << exporter_options.endpoint << "\n"; + std::cout << "Using example type: " << example_type << "\n"; + std::cout << "Using cacert path: " << exporter_options.ssl_credentials_cacert_path << "\n"; + std::cout << "Using ssl credentials: " << exporter_options.use_ssl_credentials << "\n"; // Removing this line will leave the default noop MetricProvider in place. diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client.h index 987606ecdb..fbbe069ae3 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client.h @@ -81,9 +81,9 @@ class OtlpFileClient bool is_shutdown_; // The configuration options associated with this file client. - const OtlpFileClientOptions options_; + OtlpFileClientOptions options_; // The runtime options associated with this file client. - const OtlpFileClientRuntimeOptions runtime_options_; + OtlpFileClientRuntimeOptions runtime_options_; opentelemetry::nostd::shared_ptr backend_; }; diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client_runtime_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client_runtime_options.h index 2fc4c2e31c..5d5db4a714 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client_runtime_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_client_runtime_options.h @@ -19,9 +19,6 @@ namespace otlp */ struct OtlpFileClientRuntimeOptions { - OtlpFileClientRuntimeOptions() = default; - ~OtlpFileClientRuntimeOptions() = default; - std::shared_ptr thread_instrumentation = std::shared_ptr(nullptr); }; diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index fa639a0954..c638b17662 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -719,11 +719,11 @@ static inline char HexEncode(unsigned char byte) #endif if (byte >= 10) { - return byte - 10 + 'a'; + return static_cast(byte - 10 + 'a'); } else { - return byte + '0'; + return static_cast(byte + '0'); } } @@ -1004,6 +1004,11 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender } } + OtlpFileSystemBackend(const OtlpFileSystemBackend &) = delete; + OtlpFileSystemBackend &operator=(const OtlpFileSystemBackend &) = delete; + OtlpFileSystemBackend(OtlpFileSystemBackend &&) = delete; + OtlpFileSystemBackend &operator=(OtlpFileSystemBackend &&) = delete; + // Written size is not required to be precise, we can just ignore tsan report here. OPENTELEMETRY_SANITIZER_NO_THREAD void MaybeRotateLog(std::size_t data_size) { @@ -1588,11 +1593,9 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileOstreamBackend : public OtlpFileAppende public: explicit OtlpFileOstreamBackend(const std::reference_wrapper &os) : os_(os) {} - ~OtlpFileOstreamBackend() override {} - void Export(nostd::string_view data, std::size_t /*record_count*/) override { - os_.get().write(data.data(), data.size()); + os_.get().write(data.data(), static_cast(data.size())); } bool ForceFlush(std::chrono::microseconds /*timeout*/) noexcept override @@ -1611,8 +1614,8 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileOstreamBackend : public OtlpFileAppende OtlpFileClient::OtlpFileClient(OtlpFileClientOptions &&options, OtlpFileClientRuntimeOptions &&runtime_options) : is_shutdown_(false), - options_(std::move(options)), - runtime_options_(std::move(runtime_options)) + options_{std::move(options)}, + runtime_options_{std::move(runtime_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 cb578223eb..d9f5dd6c3e 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -175,7 +175,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: session create failed."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -207,7 +207,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: connection failed."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -232,7 +232,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: request send failed."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -250,7 +250,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: SSL handshake failed."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -261,7 +261,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: request time out."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -272,7 +272,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: network error."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -297,7 +297,7 @@ class ResponseHandler : public http_client::EventHandler error_message << "[OTLP HTTP Client] Session state: (manually) cancelled."; if (!reason.empty()) { - error_message.write(reason.data(), reason.size()); + error_message.write(reason.data(), static_cast(reason.size())); } OTEL_INTERNAL_LOG_ERROR(error_message.str()); } @@ -376,11 +376,11 @@ static inline char HexEncode(unsigned char byte) #endif if (byte >= 10) { - return byte - 10 + 'a'; + return static_cast(byte - 10 + 'a'); } else { - return byte + '0'; + return static_cast(byte + '0'); } } @@ -664,7 +664,7 @@ void ConvertListFieldToJson(nlohmann::json &value, OtlpHttpClient::OtlpHttpClient(OtlpHttpClientOptions &&options) : is_shutdown_(false), - options_(options), + options_(std::move(options)), http_client_(http_client::HttpClientFactory::Create(options.thread_instrumentation)), start_session_counter_(0), finished_session_counter_(0) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h index e8e6c40a30..43a381ba60 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h @@ -21,7 +21,6 @@ class LongLastValueAggregation : public Aggregation { public: LongLastValueAggregation(); - LongLastValueAggregation(LastValuePointData &&); LongLastValueAggregation(const LastValuePointData &); void Aggregate(int64_t value, const PointAttributes &attributes = {}) noexcept override; @@ -43,16 +42,15 @@ class DoubleLastValueAggregation : public Aggregation { public: DoubleLastValueAggregation(); - DoubleLastValueAggregation(LastValuePointData &&); DoubleLastValueAggregation(const LastValuePointData &); void Aggregate(int64_t /* value */, const PointAttributes & /* attributes */) noexcept override {} void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; + std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; + std::unique_ptr Diff(const Aggregation &next) const noexcept override; PointType ToPoint() const noexcept override; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index 035a8df84d..da7d4f1e0a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -22,7 +22,6 @@ class LongSumAggregation : public Aggregation { public: LongSumAggregation(bool is_monotonic); - LongSumAggregation(SumPointData &&); LongSumAggregation(const SumPointData &); void Aggregate(int64_t value, const PointAttributes &attributes = {}) noexcept override; @@ -44,7 +43,6 @@ class DoubleSumAggregation : public Aggregation { public: DoubleSumAggregation(bool is_monotonic); - DoubleSumAggregation(SumPointData &&); DoubleSumAggregation(const SumPointData &); void Aggregate(int64_t /* value */, const PointAttributes & /* attributes */) noexcept override {} diff --git a/sdk/src/logs/batch_log_record_processor.cc b/sdk/src/logs/batch_log_record_processor.cc index c07a6992fb..14c88eeed9 100644 --- a/sdk/src/logs/batch_log_record_processor.cc +++ b/sdk/src/logs/batch_log_record_processor.cc @@ -99,7 +99,7 @@ void BatchLogRecordProcessor::OnEmit(std::unique_ptr &&record) noexc return; } - if (buffer_.Add(std::unique_ptr(record.release())) == false) + if (buffer_.Add(std::move(record)) == false) { return; } diff --git a/sdk/src/logs/multi_log_record_processor.cc b/sdk/src/logs/multi_log_record_processor.cc index 1e3234579b..c16dc0b469 100644 --- a/sdk/src/logs/multi_log_record_processor.cc +++ b/sdk/src/logs/multi_log_record_processor.cc @@ -21,11 +21,13 @@ namespace logs MultiLogRecordProcessor::MultiLogRecordProcessor( std::vector> &&processors) { - for (auto &processor : processors) + auto log_record_processors = std::move(processors); + for (auto &processor : log_record_processors) { AddProcessor(std::move(processor)); } } + MultiLogRecordProcessor::~MultiLogRecordProcessor() { ForceFlush(); @@ -54,11 +56,12 @@ std::unique_ptr MultiLogRecordProcessor::MakeRecordable() noexcept void MultiLogRecordProcessor::OnEmit(std::unique_ptr &&record) noexcept { - if (!record) + auto log_record = std::move(record); + if (!log_record) { return; } - auto multi_recordable = static_cast(record.get()); + auto multi_recordable = static_cast(log_record.get()); for (auto &processor : processors_) { diff --git a/sdk/src/logs/simple_log_record_processor.cc b/sdk/src/logs/simple_log_record_processor.cc index 7e65f31ca4..5dd2a9e06f 100644 --- a/sdk/src/logs/simple_log_record_processor.cc +++ b/sdk/src/logs/simple_log_record_processor.cc @@ -41,7 +41,8 @@ std::unique_ptr SimpleLogRecordProcessor::MakeRecordable() noexcept */ void SimpleLogRecordProcessor::OnEmit(std::unique_ptr &&record) noexcept { - nostd::span> batch(&record, 1); + auto log_record = std::move(record); + nostd::span> batch(&log_record, 1); // Get lock to ensure Export() is never called concurrently const std::lock_guard locked(lock_); diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index 2bc0eddb0e..0dfbefe5f3 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -5,7 +5,6 @@ #include #include #include -#include #include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/common/timestamp.h" @@ -28,8 +27,6 @@ LongLastValueAggregation::LongLastValueAggregation() point_data_.value_ = static_cast(0); } -LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) : point_data_{data} {} - LongLastValueAggregation::LongLastValueAggregation(const LastValuePointData &data) : point_data_{data} {} @@ -50,12 +47,12 @@ std::unique_ptr LongLastValueAggregation::Merge( nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch()) { LastValuePointData merge_data = nostd::get(ToPoint()); - return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + return std::unique_ptr(new LongLastValueAggregation(merge_data)); } else { LastValuePointData merge_data = nostd::get(delta.ToPoint()); - return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + return std::unique_ptr(new LongLastValueAggregation(merge_data)); } } @@ -65,12 +62,12 @@ std::unique_ptr LongLastValueAggregation::Diff(const Aggregation &n nostd::get(next.ToPoint()).sample_ts_.time_since_epoch()) { LastValuePointData diff_data = nostd::get(ToPoint()); - return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + return std::unique_ptr(new LongLastValueAggregation(diff_data)); } else { LastValuePointData diff_data = nostd::get(next.ToPoint()); - return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + return std::unique_ptr(new LongLastValueAggregation(diff_data)); } } @@ -86,10 +83,6 @@ DoubleLastValueAggregation::DoubleLastValueAggregation() point_data_.value_ = 0.0; } -DoubleLastValueAggregation::DoubleLastValueAggregation(LastValuePointData &&data) - : point_data_{data} -{} - DoubleLastValueAggregation::DoubleLastValueAggregation(const LastValuePointData &data) : point_data_{data} {} @@ -110,12 +103,12 @@ std::unique_ptr DoubleLastValueAggregation::Merge( nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch()) { LastValuePointData merge_data = nostd::get(ToPoint()); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); + return std::unique_ptr(new DoubleLastValueAggregation(merge_data)); } else { LastValuePointData merge_data = nostd::get(delta.ToPoint()); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); + return std::unique_ptr(new DoubleLastValueAggregation(merge_data)); } } @@ -126,12 +119,12 @@ std::unique_ptr DoubleLastValueAggregation::Diff( nostd::get(next.ToPoint()).sample_ts_.time_since_epoch()) { LastValuePointData diff_data = nostd::get(ToPoint()); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(diff_data))); + return std::unique_ptr(new DoubleLastValueAggregation(diff_data)); } else { LastValuePointData diff_data = nostd::get(next.ToPoint()); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(diff_data))); + return std::unique_ptr(new DoubleLastValueAggregation(diff_data)); } } diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index 59119872b2..dc08721682 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -27,8 +27,6 @@ LongSumAggregation::LongSumAggregation(bool is_monotonic) point_data_.is_monotonic_ = is_monotonic; } -LongSumAggregation::LongSumAggregation(SumPointData &&data) : point_data_{data} {} - LongSumAggregation::LongSumAggregation(const SumPointData &data) : point_data_{data} {} void LongSumAggregation::Aggregate(int64_t value, const PointAttributes & /* attributes */) noexcept @@ -81,8 +79,6 @@ DoubleSumAggregation::DoubleSumAggregation(bool is_monotonic) point_data_.is_monotonic_ = is_monotonic; } -DoubleSumAggregation::DoubleSumAggregation(SumPointData &&data) : point_data_(data) {} - DoubleSumAggregation::DoubleSumAggregation(const SumPointData &data) : point_data_(data) {} void DoubleSumAggregation::Aggregate(double value, diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index 5c65a5b787..c20f3efaa7 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -20,7 +20,6 @@ #include "opentelemetry/sdk/metrics/meter_context.h" #include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/sdk/metrics/state/metric_collector.h" -#include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -28,7 +27,6 @@ namespace sdk { namespace metrics { -using opentelemetry::sdk::resource::Resource; MetricCollector::MetricCollector(opentelemetry::sdk::metrics::MeterContext *context, std::shared_ptr metric_reader, diff --git a/sdk/test/logs/batch_log_record_processor_test.cc b/sdk/test/logs/batch_log_record_processor_test.cc index 8ff6f1fd19..773d1283b5 100644 --- a/sdk/test/logs/batch_log_record_processor_test.cc +++ b/sdk/test/logs/batch_log_record_processor_test.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -143,7 +144,7 @@ class MockLogExporter final : public LogRecordExporter std::shared_ptr> force_flush_counter_; std::shared_ptr> is_shutdown_; std::shared_ptr> is_export_completed_; - const std::chrono::milliseconds export_delay_; + std::chrono::milliseconds export_delay_; }; /** diff --git a/sdk/test/logs/log_record_test.cc b/sdk/test/logs/log_record_test.cc index 48d0754060..cd6fa730f7 100644 --- a/sdk/test/logs/log_record_test.cc +++ b/sdk/test/logs/log_record_test.cc @@ -100,9 +100,10 @@ class TestBodyLogger : public opentelemetry::logs::Logger void EmitLogRecord(nostd::unique_ptr &&record) noexcept override { - if (record) + auto log_record = std::move(record); + if (log_record) { - last_body_ = static_cast(record.get())->GetBody(); + last_body_ = static_cast(log_record.get())->GetBody(); } } diff --git a/sdk/test/logs/logger_provider_set_test.cc b/sdk/test/logs/logger_provider_set_test.cc index 57f0547c87..f8b8564e9a 100644 --- a/sdk/test/logs/logger_provider_set_test.cc +++ b/sdk/test/logs/logger_provider_set_test.cc @@ -26,7 +26,6 @@ using opentelemetry::logs::EventLoggerProvider; #endif using opentelemetry::logs::Logger; using opentelemetry::logs::LoggerProvider; -using opentelemetry::logs::Provider; using opentelemetry::nostd::shared_ptr; namespace nostd = opentelemetry::nostd; diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index 04c14fc8a0..40ef5f2560 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -208,9 +208,10 @@ class MockProcessor final : public LogRecordProcessor // constructor void OnEmit(std::unique_ptr &&record) noexcept override { + auto log_record = std::move(record); // Cast the recordable received into a concrete MockLogRecordable type auto copy = - std::shared_ptr(static_cast(record.release())); + std::shared_ptr(static_cast(log_record.release())); // Copy over the received log record's severity, name, and body fields over to the recordable // passed in the constructor diff --git a/sdk/test/metrics/meter_provider_set_test.cc b/sdk/test/metrics/meter_provider_set_test.cc index bd8aae8cff..7282d2e6a1 100644 --- a/sdk/test/metrics/meter_provider_set_test.cc +++ b/sdk/test/metrics/meter_provider_set_test.cc @@ -18,7 +18,6 @@ using opentelemetry::sdk::common::unsetenv; using opentelemetry::metrics::MeterProvider; using opentelemetry::metrics::NoopMeterProvider; -using opentelemetry::metrics::Provider; namespace metrics_api = opentelemetry::metrics; namespace metrics_sdk = opentelemetry::sdk::metrics; diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 427889d907..002c2a665a 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -111,7 +111,7 @@ class TestLogHandler : public LogHandler { if (LogLevel::Warning == level) { - std::cout << msg << std::endl; + std::cout << msg << "\n"; warnings.push_back(msg); } } diff --git a/sdk/test/trace/batch_span_processor_test.cc b/sdk/test/trace/batch_span_processor_test.cc index fb78985b9f..af9bbeab5e 100644 --- a/sdk/test/trace/batch_span_processor_test.cc +++ b/sdk/test/trace/batch_span_processor_test.cc @@ -94,7 +94,7 @@ class MockSpanExporter final : public sdk::trace::SpanExporter std::shared_ptr> is_shutdown_; std::shared_ptr> is_export_completed_; // Meant exclusively to test force flush timeout - const std::chrono::milliseconds export_delay_; + std::chrono::milliseconds export_delay_; }; /**