Skip to content

Commit

Permalink
[Code Health] Clang Tidy cleanup, Part 2 (#3038)
Browse files Browse the repository at this point in the history
  • Loading branch information
msiddhu authored Aug 25, 2024
1 parent 242d52a commit a71642f
Show file tree
Hide file tree
Showing 34 changed files with 127 additions and 125 deletions.
3 changes: 3 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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-*
2 changes: 1 addition & 1 deletion .github/workflows/clang-tidy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions api/test/baggage/propagation/baggage_propagator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions api/test/context/propagation/composite_propagator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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);
}
Expand All @@ -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_;
Expand Down
8 changes: 4 additions & 4 deletions api/test/logs/logger_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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,
Expand Down Expand Up @@ -84,14 +84,14 @@ static void ThreadRoutine(Barrier &barrier,

void MultiThreadRunner(benchmark::State &state, const std::function<void()> &func)
{
int num_threads = std::thread::hardware_concurrency();
uint32_t num_threads = std::thread::hardware_concurrency();

Barrier barrier(num_threads);

std::vector<std::thread> 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);
}
Expand Down
2 changes: 1 addition & 1 deletion api/test/nostd/shared_ptr_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class C
class D : public C
{
public:
virtual ~D() {}
~D() override {}
};

TEST(SharedPtrTest, DefaultConstruction)
Expand Down
4 changes: 2 additions & 2 deletions api/test/trace/propagation/b3_propagation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions api/test/trace/propagation/http_text_format_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions api/test/trace/propagation/jaeger_propagation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions examples/http/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/src/otlp_file_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<FileStats>();
file_->is_shutdown.store(false);
Expand Down Expand Up @@ -1539,7 +1539,7 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender
std::shared_ptr<FileStats> file_;

std::atomic<bool> is_initialized_;
std::time_t check_file_path_interval_;
std::time_t check_file_path_interval_{0};
};

class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileOstreamBackend : public OtlpFileAppender
Expand Down Expand Up @@ -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<OtlpFileClientFileSystemOptions>(options_.backend_options))
{
Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/src/otlp_file_metric_exporter_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace otlp
{

OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions()
: aggregation_temporality(PreferredAggregationTemporality::kCumulative)
{
console_debug = false;

Expand All @@ -25,8 +26,6 @@ OtlpFileMetricExporterOptions::OtlpFileMetricExporterOptions()
fs_options.rotate_size = 10;

backend_options = fs_options;

aggregation_temporality = PreferredAggregationTemporality::kCumulative;
}

OtlpFileMetricExporterOptions::~OtlpFileMetricExporterOptions() {}
Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/src/otlp_grpc_metric_exporter_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace otlp
{

OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions()
: aggregation_temporality(PreferredAggregationTemporality::kCumulative)
{
endpoint = GetOtlpDefaultGrpcMetricsEndpoint();
use_ssl_credentials = !GetOtlpDefaultGrpcMetricsIsInsecure(); /* negation intended. */
Expand All @@ -28,8 +29,6 @@ OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions()
metadata = GetOtlpDefaultMetricsHeaders();
user_agent = GetOtlpDefaultUserAgent();

aggregation_temporality = PreferredAggregationTemporality::kCumulative;

max_threads = 0;

compression = GetOtlpDefaultMetricsCompression();
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/src/otlp_http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ OtlpHttpClient::~OtlpHttpClient()
OtlpHttpClient::OtlpHttpClient(OtlpHttpClientOptions &&options,
std::shared_ptr<ext::http::client::HttpClient> 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)
Expand Down
29 changes: 15 additions & 14 deletions exporters/otlp/src/otlp_http_exporter_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
29 changes: 15 additions & 14 deletions exporters/otlp/src/otlp_http_log_record_exporter_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
31 changes: 16 additions & 15 deletions exporters/otlp/src/otlp_http_metric_exporter_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

2 comments on commit a71642f

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: a71642f Previous: 242d52a Ratio
BM_BaselineBuffer/4 16244995.594024658 ns/iter 5962328.910827637 ns/iter 2.72
BM_LockFreeBuffer/1 2765486.478805542 ns/iter 1307056.6654205322 ns/iter 2.12

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: a71642f Previous: 242d52a Ratio
BM_SpinLockThrashing/1/process_time/real_time 0.21839090351409582 ms/iter 0.08107887159145784 ms/iter 2.69
BM_SpinLockThrashing/2/process_time/real_time 0.545456295921689 ms/iter 0.20065668929490824 ms/iter 2.72

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.