From 9d7c871aa9553dc05d994bb8406683d0e149b3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ege=20=C3=87etin?= <64282645+egecetin@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:35:55 +0300 Subject: [PATCH] Prometheus naming + deduplication and use gersemi for CMake formatting (#325) * allow naming of prometheus stats * fix compile error * fix naming * deduplicate code * fix issues * move init to baseclass * move to private * init base variables * add gersemi --- .pre-commit-config.yaml | 5 ++ CMakeLists.txt | 1 + include/telnet/TelnetServer.hpp | 4 +- include/telnet/TelnetStats.hpp | 14 +++--- include/utils/BaseServerStats.hpp | 28 +++++++++++ include/zeromq/ZeroMQServer.hpp | 3 +- include/zeromq/ZeroMQStats.hpp | 13 ++--- scripts/runStaticAnalysis.sh | 15 ++++++ src/telnet/TelnetServer.cpp | 5 +- src/telnet/TelnetStats.cpp | 83 +++++++------------------------ src/utils/BaseServerStats.cpp | 60 ++++++++++++++++++++++ src/zeromq/ZeroMQServer.cpp | 4 +- src/zeromq/ZeroMQStats.cpp | 76 ++++++---------------------- 13 files changed, 163 insertions(+), 148 deletions(-) create mode 100644 include/utils/BaseServerStats.hpp create mode 100755 scripts/runStaticAnalysis.sh create mode 100644 src/utils/BaseServerStats.cpp diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a666fcf83..ab660df6d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,6 +25,11 @@ repos: hooks: - id: detect-secrets exclude: tests/data/config.json + - repo: https://github.com/BlankSpruce/gersemi + rev: 0.15.1 + hooks: + - id: gersemi + args: ['--line-length', '120'] - repo: https://github.com/lovesegfault/beautysh rev: v6.2.1 hooks: diff --git a/CMakeLists.txt b/CMakeLists.txt index ff61147fd..f0d13e802 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,6 +72,7 @@ file(GLOB ProjectSources ${PROJECT_SOURCE_DIR}/src/metrics/Status.cpp ${PROJECT_SOURCE_DIR}/src/telnet/TelnetServer.cpp ${PROJECT_SOURCE_DIR}/src/telnet/TelnetStats.cpp + ${PROJECT_SOURCE_DIR}/src/utils/BaseServerStats.cpp ${PROJECT_SOURCE_DIR}/src/utils/ConfigParser.cpp ${PROJECT_SOURCE_DIR}/src/utils/ErrorHelpers.cpp ${PROJECT_SOURCE_DIR}/src/utils/Tracer.cpp diff --git a/include/telnet/TelnetServer.hpp b/include/telnet/TelnetServer.hpp index e4d59b798..679592b2e 100644 --- a/include/telnet/TelnetServer.hpp +++ b/include/telnet/TelnetServer.hpp @@ -152,11 +152,13 @@ class TelnetServer : public std::enable_shared_from_this { * @param[in] listenPort Port to listen * @param[in] promptString Prompt string for connected users * @param[in] reg Prometheus registry for stats + * @param[in] prependName Prefix for Prometheus stats * @return true If initialized * @return false otherwise */ bool initialise(u_long listenPort, const std::shared_ptr &checkFlag, - std::string promptString = "", const std::shared_ptr ® = nullptr); + std::string promptString = "", const std::shared_ptr ® = nullptr, + const std::string &prependName = ""); /// Closes the Telnet Server void shutdown(); diff --git a/include/telnet/TelnetStats.hpp b/include/telnet/TelnetStats.hpp index 96becb308..ca8755143 100644 --- a/include/telnet/TelnetStats.hpp +++ b/include/telnet/TelnetStats.hpp @@ -1,5 +1,7 @@ #pragma once +#include "utils/BaseServerStats.hpp" + #include /** @@ -28,18 +30,12 @@ struct TelnetServerStats { /** * Prometheus statistics for Telnet server */ -class TelnetStats { +class TelnetStats : private BaseServerStats { private: prometheus::Family *_infoFamily; ///< Information metric family prometheus::Gauge *_activeConnection; ///< Number of active connections prometheus::Counter *_refusedConnection; ///< Number of refused connections prometheus::Counter *_totalConnection; ///< Number of total received connections - prometheus::Summary *_processingTime; ///< Value of the command processing performance - prometheus::Gauge *_maxProcessingTime; ///< Maximum value of the command processing performance - prometheus::Gauge *_minProcessingTime; ///< Minimum value of the command processing performance - prometheus::Counter *_succeededCommand; ///< Number of succeeded commands - prometheus::Counter *_failedCommand; ///< Number of failed commands - prometheus::Counter *_totalCommand; ///< Number of total received commands prometheus::Counter *_totalUploadBytes; ///< Total uploaded bytes prometheus::Counter *_totalDownloadBytes; ///< Total downloaded bytes prometheus::Summary *_sessionDuration; ///< Value of the duration of sessions @@ -51,8 +47,10 @@ class TelnetStats { * Construct a new Telnet statistics * @param[in] reg Prometheus registry * @param[in] portNumber Telnet server port + * @param[in] prependName Prefix for Prometheus stats */ - TelnetStats(const std::shared_ptr ®, uint16_t portNumber); + TelnetStats(const std::shared_ptr ®, uint16_t portNumber, + const std::string &prependName = ""); /** * Updates statistics with session values diff --git a/include/utils/BaseServerStats.hpp b/include/utils/BaseServerStats.hpp new file mode 100644 index 000000000..a07ebddd5 --- /dev/null +++ b/include/utils/BaseServerStats.hpp @@ -0,0 +1,28 @@ +#pragma once + +#include + +#define QUANTILE_DEFAULTS \ + prometheus::Summary::Quantiles \ + { \ + {0.5, 0.1}, {0.9, 0.1}, { 0.99, 0.1 } \ + } + +/** + * @class BaseServerStats + * Represents the base statistics for a server. + */ +class BaseServerStats { + private: + prometheus::Summary *_processingTime{nullptr}; ///< Value of the command processing performance + prometheus::Gauge *_maxProcessingTime{nullptr}; ///< Maximum value of the command processing performance + prometheus::Gauge *_minProcessingTime{nullptr}; ///< Minimum value of the command processing performance + prometheus::Counter *_succeededCommand{nullptr}; ///< Number of succeeded commands + prometheus::Counter *_failedCommand{nullptr}; ///< Number of failed commands + prometheus::Counter *_totalCommand{nullptr}; ///< Number of total received commands + + protected: + void initBaseStats(const std::shared_ptr ®, const std::string &name); + + void consumeBaseStats(uint64_t succeeded, uint64_t failed, double processingTime); +}; diff --git a/include/zeromq/ZeroMQServer.hpp b/include/zeromq/ZeroMQServer.hpp index c0bed2871..24065369d 100644 --- a/include/zeromq/ZeroMQServer.hpp +++ b/include/zeromq/ZeroMQServer.hpp @@ -33,9 +33,10 @@ class ZeroMQServer : private ZeroMQ, private ZeroMQMonitor { * @param[in] hostAddr Host address to connect. Can be anything supported by ZeroMQ reply socket * @param[in] checkFlag Flag to check if the server is running * @param[in] reg Prometheus registry for stats + * @param[in] prependName Prefix for Prometheus stats */ ZeroMQServer(const std::string &hostAddr, std::shared_ptr checkFlag, - const std::shared_ptr ® = nullptr); + const std::shared_ptr ® = nullptr, const std::string &prependName = ""); /// @brief Copy constructor ZeroMQServer(const ZeroMQServer & /*unused*/) = delete; diff --git a/include/zeromq/ZeroMQStats.hpp b/include/zeromq/ZeroMQStats.hpp index 42bae9ba9..57eeb833a 100644 --- a/include/zeromq/ZeroMQStats.hpp +++ b/include/zeromq/ZeroMQStats.hpp @@ -1,5 +1,7 @@ #pragma once +#include "utils/BaseServerStats.hpp" + #include #include @@ -17,15 +19,9 @@ struct ZeroMQServerStats { * @class ZeroMQStats * Represents the statistics of a ZeroMQ server. */ -class ZeroMQStats { +class ZeroMQStats : private BaseServerStats { private: prometheus::Family *_infoFamily; ///< Information metric family - prometheus::Summary *_processingTime; ///< Value of the command processing performance - prometheus::Gauge *_maxProcessingTime; ///< Maximum value of the command processing performance - prometheus::Gauge *_minProcessingTime; ///< Minimum value of the command processing performance - prometheus::Counter *_succeededCommand; ///< Number of succeeded commands - prometheus::Counter *_failedCommand; ///< Number of failed commands - prometheus::Counter *_totalCommand; ///< Number of total received commands prometheus::Counter *_succeededCommandParts; ///< Number of received succeeded message parts prometheus::Counter *_failedCommandParts; ///< Number of received failed message parts prometheus::Counter *_totalCommandParts; ///< Number of received total message parts @@ -36,8 +32,9 @@ class ZeroMQStats { /** * Construct a new ZeroMQStats object. * @param[in] reg Prometheus registry. + * @param[in] prependName Prefix for Prometheus stats. */ - explicit ZeroMQStats(const std::shared_ptr ®); + explicit ZeroMQStats(const std::shared_ptr ®, const std::string &prependName = ""); /** * Updates the statistics with messages. diff --git a/scripts/runStaticAnalysis.sh b/scripts/runStaticAnalysis.sh new file mode 100755 index 000000000..55f216cf4 --- /dev/null +++ b/scripts/runStaticAnalysis.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +set -e + +echo "Running pre-commit checks" +pre-commit run --all-files + +echo "Running clang-format" +clang-format include/**/*.hpp src/*.cpp src/**/*.cpp --verbose --dry-run --Werror + +echo "Running cppcheck" +cppcheck -Iinclude/ src --verbose --enable=all --error-exitcode=1 --std=c++14 --language=c++ --suppressions-list=cppcheckSuppressions.txt --inline-suppr + +echo "Running clang-tidy" +run-clang-tidy -j`nproc` -p=build -header-filter=`pwd`/include/ src/*.cpp src/**/*.cpp diff --git a/src/telnet/TelnetServer.cpp b/src/telnet/TelnetServer.cpp index cf840bda1..43b2e9c86 100644 --- a/src/telnet/TelnetServer.cpp +++ b/src/telnet/TelnetServer.cpp @@ -556,7 +556,8 @@ TelnetServer::~TelnetServer() } bool TelnetServer::initialise(u_long listenPort, const std::shared_ptr &checkFlag, - std::string promptString, const std::shared_ptr ®) + std::string promptString, const std::shared_ptr ®, + const std::string &prependName) { if (m_initialised) { @@ -616,7 +617,7 @@ bool TelnetServer::initialise(u_long listenPort, const std::shared_ptr(reg, listenPort); + m_stats = std::make_unique(reg, listenPort, prependName); } m_shouldStop.clear(); diff --git a/src/telnet/TelnetStats.cpp b/src/telnet/TelnetStats.cpp index 9aeedc9e1..8b283d616 100644 --- a/src/telnet/TelnetStats.cpp +++ b/src/telnet/TelnetStats.cpp @@ -8,21 +8,22 @@ #include -#define QUANTILE_DEFAULTS \ - prometheus::Summary::Quantiles \ - { \ - {0.5, 0.1}, {0.9, 0.1}, { 0.99, 0.1 } \ - } - -TelnetStats::TelnetStats(const std::shared_ptr ®, uint16_t portNumber) +TelnetStats::TelnetStats(const std::shared_ptr ®, uint16_t portNumber, + const std::string &prependName) + : BaseServerStats() { if (!reg) { throw std::invalid_argument("Can't init Telnet statistics. Registry is null"); } + const auto name = prependName.empty() ? "telnet_" : prependName + "_telnet_"; + + // Stats from base class + initBaseStats(reg, name); + // Basic information - _infoFamily = &prometheus::BuildInfo().Name("telnet").Help("Telnet server information").Register(*reg); + _infoFamily = &prometheus::BuildInfo().Name(name).Help("Telnet server information").Register(*reg); _infoFamily->Add({{"server_port", std::to_string(portNumber)}}); _infoFamily->Add({{"init_time", date::format("%FT%TZ", date::floor( @@ -31,91 +32,54 @@ TelnetStats::TelnetStats(const std::shared_ptr ®, uint1 // Connection stats _activeConnection = &prometheus::BuildGauge() - .Name("telnet_active_connections") + .Name(name + "active_connections") .Help("Number of active connections") .Register(*reg) .Add({}); _refusedConnection = &prometheus::BuildCounter() - .Name("telnet_refused_connections") + .Name(name + "refused_connections") .Help("Number of refused connections") .Register(*reg) .Add({}); _totalConnection = &prometheus::BuildCounter() - .Name("telnet_received_connections") + .Name(name + "received_connections") .Help("Number of received connections") .Register(*reg) .Add({}); - // Performance stats - _processingTime = &prometheus::BuildSummary() - .Name("telnet_processing_time") - .Help("Command processing performance") - .Register(*reg) - .Add({}, QUANTILE_DEFAULTS); - _maxProcessingTime = &prometheus::BuildGauge() - .Name("telnet_maximum_processing_time") - .Help("Maximum value of the command processing performance") - .Register(*reg) - .Add({}); - _minProcessingTime = &prometheus::BuildGauge() - .Name("telnet_minimum_processing_time") - .Help("Minimum value of the command processing performance") - .Register(*reg) - .Add({}); - - // Command stats - _succeededCommand = &prometheus::BuildCounter() - .Name("telnet_succeeded_commands") - .Help("Number of succeeded commands") - .Register(*reg) - .Add({}); - _failedCommand = &prometheus::BuildCounter() - .Name("telnet_failed_commands") - .Help("Number of failed commands") - .Register(*reg) - .Add({}); - _totalCommand = &prometheus::BuildCounter() - .Name("telnet_received_commands") - .Help("Number of received commands") - .Register(*reg) - .Add({}); - // Bandwidth stats _totalUploadBytes = - &prometheus::BuildCounter().Name("telnet_uploaded_bytes").Help("Total uploaded bytes").Register(*reg).Add({}); + &prometheus::BuildCounter().Name(name + "uploaded_bytes").Help("Total uploaded bytes").Register(*reg).Add({}); _totalDownloadBytes = &prometheus::BuildCounter() - .Name("telnet_downloaded_bytes") + .Name(name + "downloaded_bytes") .Help("Total downloaded bytes") .Register(*reg) .Add({}); // Session durations _sessionDuration = &prometheus::BuildSummary() - .Name("telnet_session_duration") + .Name(name + "session_duration") .Help("Duration of sessions") .Register(*reg) .Add({}, QUANTILE_DEFAULTS); _maxSessionDuration = &prometheus::BuildGauge() - .Name("telnet_maximum_session_duration") + .Name(name + "maximum_session_duration") .Help("Maximum duration of sessions") .Register(*reg) .Add({}); _minSessionDuration = &prometheus::BuildGauge() - .Name("telnet_minimum_session_duration") + .Name(name + "minimum_session_duration") .Help("Minimum duration of sessions") .Register(*reg) .Add({}); // Set defaults - _minProcessingTime->Set(std::numeric_limits::max()); _minSessionDuration->Set(std::numeric_limits::max()); } void TelnetStats::consumeStats(const TelnetSessionStats &stat, bool sessionClosed) { - _succeededCommand->Increment(static_cast(stat.successCmdCtr)); - _failedCommand->Increment(static_cast(stat.failCmdCtr)); - _totalCommand->Increment(static_cast(stat.successCmdCtr + stat.failCmdCtr)); + consumeBaseStats(stat.successCmdCtr, stat.failCmdCtr, 0); _totalUploadBytes->Increment(static_cast(stat.uploadBytes)); _totalDownloadBytes->Increment(static_cast(stat.downloadBytes)); @@ -147,15 +111,6 @@ void TelnetStats::consumeStats(const TelnetServerStats &stat) // Performance stats if there is an active connection if (stat.activeConnectionCtr > 0) { - const auto processTime = static_cast((stat.processingTimeEnd - stat.processingTimeStart).count()); - _processingTime->Observe(processTime); - if (processTime < _minProcessingTime->Value()) - { - _minProcessingTime->Set(processTime); - } - if (processTime > _maxProcessingTime->Value()) - { - _maxProcessingTime->Set(processTime); - } + consumeBaseStats(0, 0, static_cast((stat.processingTimeEnd - stat.processingTimeStart).count())); } } diff --git a/src/utils/BaseServerStats.cpp b/src/utils/BaseServerStats.cpp new file mode 100644 index 000000000..72b4e593b --- /dev/null +++ b/src/utils/BaseServerStats.cpp @@ -0,0 +1,60 @@ +#include "utils/BaseServerStats.hpp" + +#include +#include + +void BaseServerStats::initBaseStats(const std::shared_ptr ®, const std::string &name) +{ + // Command stats + _succeededCommand = &prometheus::BuildCounter() + .Name(name + "succeeded_commands") + .Help("Number of succeeded commands") + .Register(*reg) + .Add({}); + _failedCommand = &prometheus::BuildCounter() + .Name(name + "failed_commands") + .Help("Number of failed commands") + .Register(*reg) + .Add({}); + _totalCommand = &prometheus::BuildCounter() + .Name(name + "received_commands") + .Help("Number of received commands") + .Register(*reg) + .Add({}); + + // Performance stats + _processingTime = &prometheus::BuildSummary() + .Name(name + "processing_time") + .Help("Command processing performance") + .Register(*reg) + .Add({}, QUANTILE_DEFAULTS); + _maxProcessingTime = &prometheus::BuildGauge() + .Name(name + "maximum_processing_time") + .Help("Maximum value of the command processing performance") + .Register(*reg) + .Add({}); + _minProcessingTime = &prometheus::BuildGauge() + .Name(name + "minimum_processing_time") + .Help("Minimum value of the command processing performance") + .Register(*reg) + .Add({}); + + // Set defaults + _minProcessingTime->Set(std::numeric_limits::max()); +} + +void BaseServerStats::consumeBaseStats(uint64_t succeeded, uint64_t failed, double processingTime) +{ + // Command stats + _succeededCommand->Increment(static_cast(succeeded)); + _failedCommand->Increment(static_cast(failed)); + _totalCommand->Increment(static_cast(succeeded + failed)); + + // Performance stats + if (processingTime > 0) + { + _processingTime->Observe(processingTime); + _maxProcessingTime->Set(std::max(_maxProcessingTime->Value(), processingTime)); + _minProcessingTime->Set(std::min(_minProcessingTime->Value(), processingTime)); + } +} diff --git a/src/zeromq/ZeroMQServer.cpp b/src/zeromq/ZeroMQServer.cpp index 1577a5512..d1fcd8ee7 100644 --- a/src/zeromq/ZeroMQServer.cpp +++ b/src/zeromq/ZeroMQServer.cpp @@ -69,12 +69,12 @@ void ZeroMQServer::threadFunc() noexcept } ZeroMQServer::ZeroMQServer(const std::string &hostAddr, std::shared_ptr checkFlag, - const std::shared_ptr ®) + const std::shared_ptr ®, const std::string &prependName) : ZeroMQ(zmq::socket_type::rep, hostAddr, true), _checkFlag(std::move(checkFlag)) { if (reg) { - _stats = std::make_unique(reg); + _stats = std::make_unique(reg, prependName); } startMonitoring(getSocket().get(), "inproc://" + std::to_string(constHasher(hostAddr.c_str())) + ".rep"); diff --git a/src/zeromq/ZeroMQStats.cpp b/src/zeromq/ZeroMQStats.cpp index c3d6c616a..d9737e8e0 100644 --- a/src/zeromq/ZeroMQStats.cpp +++ b/src/zeromq/ZeroMQStats.cpp @@ -6,19 +6,19 @@ #include #include -#define QUANTILE_DEFAULTS \ - prometheus::Summary::Quantiles \ - { \ - {0.5, 0.1}, {0.9, 0.1}, { 0.99, 0.1 } \ - } - -ZeroMQStats::ZeroMQStats(const std::shared_ptr ®) +ZeroMQStats::ZeroMQStats(const std::shared_ptr ®, const std::string &prependName) + : BaseServerStats() { if (!reg) { throw std::invalid_argument("Can't init ZeroMQ statistics. Registry is null"); } + const auto name = prependName.empty() ? "zeromq_" : prependName + "_zeromq_"; + + // Init stats from base class + initBaseStats(reg, name); + // Basic information _infoFamily = &prometheus::BuildInfo().Name("zeromq").Help("ZeroMQ server information").Register(*reg); @@ -26,74 +26,38 @@ ZeroMQStats::ZeroMQStats(const std::shared_ptr ®) std::chrono::high_resolution_clock::now()))}}); _infoFamily->Add({{"performance_unit", "nanoseconds"}}); - // Performance stats - _processingTime = &prometheus::BuildSummary() - .Name("zeromq_processing_time") - .Help("Command processing performance") - .Register(*reg) - .Add({}, QUANTILE_DEFAULTS); - _maxProcessingTime = &prometheus::BuildGauge() - .Name("zeromq_maximum_processing_time") - .Help("Maximum value of the command processing performance") - .Register(*reg) - .Add({}); - _minProcessingTime = &prometheus::BuildGauge() - .Name("zeromq_minimum_processing_time") - .Help("Minimum value of the command processing performance") - .Register(*reg) - .Add({}); - // Command stats - _succeededCommand = &prometheus::BuildCounter() - .Name("zeromq_succeeded_commands") - .Help("Number of succeeded commands") - .Register(*reg) - .Add({}); - _failedCommand = &prometheus::BuildCounter() - .Name("zeromq_failed_commands") - .Help("Number of failed commands") - .Register(*reg) - .Add({}); - _totalCommand = &prometheus::BuildCounter() - .Name("zeromq_received_commands") - .Help("Number of received commands") - .Register(*reg) - .Add({}); _succeededCommandParts = &prometheus::BuildCounter() - .Name("zeromq_succeeded_command_parts") + .Name(name + "succeeded_command_parts") .Help("Number of received succeeded message parts") .Register(*reg) .Add({}); _failedCommandParts = &prometheus::BuildCounter() - .Name("zeromq_failed_command_parts") + .Name(name + "failed_command_parts") .Help("Number of received failed message parts") .Register(*reg) .Add({}); _totalCommandParts = &prometheus::BuildCounter() - .Name("zeromq_total_command_parts") + .Name(name + "total_command_parts") .Help("Number of received total message parts") .Register(*reg) .Add({}); // Bandwidth stats _totalUploadBytes = - &prometheus::BuildCounter().Name("zeromq_uploaded_bytes").Help("Total uploaded bytes").Register(*reg).Add({}); + &prometheus::BuildCounter().Name(name + "uploaded_bytes").Help("Total uploaded bytes").Register(*reg).Add({}); _totalDownloadBytes = &prometheus::BuildCounter() - .Name("zeromq_downloaded_bytes") + .Name(name + "downloaded_bytes") .Help("Total downloaded bytes") .Register(*reg) .Add({}); - - // Set defaults - _minProcessingTime->Set(std::numeric_limits::max()); } void ZeroMQStats::consumeStats(const std::vector &recvMsgs, const std::vector &sendMsgs, const ZeroMQServerStats &serverStats) { - _succeededCommand->Increment(static_cast(serverStats.isSuccessful)); - _failedCommand->Increment(static_cast(!serverStats.isSuccessful)); - _totalCommand->Increment(); + consumeBaseStats(static_cast(serverStats.isSuccessful), static_cast(!serverStats.isSuccessful), + static_cast((serverStats.processingTimeEnd - serverStats.processingTimeStart).count())); for (const auto &entry : recvMsgs) { @@ -114,16 +78,4 @@ void ZeroMQStats::consumeStats(const std::vector &recvMsgs, cons { _totalUploadBytes->Increment(static_cast(entry.size())); } - - const auto processTime = - static_cast((serverStats.processingTimeEnd - serverStats.processingTimeStart).count()); - _processingTime->Observe(processTime); - if (processTime < _minProcessingTime->Value()) - { - _minProcessingTime->Set(processTime); - } - if (processTime > _maxProcessingTime->Value()) - { - _maxProcessingTime->Set(processTime); - } }