From de17711c94ae42602cd69eb902e21fc0a61cfd5f Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Wed, 7 Aug 2019 16:00:26 +0300 Subject: [PATCH] review fixes --- CMakeLists.txt | 6 +-- docs/monitoring.md | 2 +- features/step_definitions/monitoring.js | 13 +++--- features/support/route.js | 2 +- include/monitoring/monitoring.hpp | 3 ++ .../monitoring/monitoring_request_handler.hpp | 30 +------------ include/osrm/osrm.hpp | 6 +-- include/server/request_handler.hpp | 2 +- include/server/service/base_service.hpp | 2 +- include/server/service_handler.hpp | 4 +- src/monitoring/monitoring_request_handler.cpp | 43 +++++++++++++++++++ src/osrm/osrm.cpp | 5 +-- src/tools/routed.cpp | 7 +-- 13 files changed, 67 insertions(+), 58 deletions(-) create mode 100644 src/monitoring/monitoring_request_handler.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index e6feb6de9..b949220a9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -149,7 +149,7 @@ file(GLOB CustomizerGlob src/customize/*.cpp) file(GLOB ContractorGlob src/contractor/*.cpp) file(GLOB UpdaterGlob src/updater/*.cpp) file(GLOB StorageGlob src/storage/*.cpp) -file(GLOB ServerGlob src/server/*.cpp src/server/**/*.cpp) +file(GLOB ServerGlob src/server/*.cpp src/server/**/*.cpp src/monitoring/*.cpp) file(GLOB EngineGlob src/engine/*.cpp src/engine/**/*.cpp) file(GLOB ErrorcodesGlob src/osrm/errorcodes.cpp) @@ -726,7 +726,7 @@ file(GLOB FlatbuffersGlob third_party/flatbuffers/include/flatbuffers/*.h) file(GLOB LibraryGlob include/osrm/*.hpp) file(GLOB ParametersGlob include/engine/api/*_parameters.hpp) set(ApiHeader include/engine/api/base_result.hpp) -set(EngineHeader include/engine/status.hpp include/engine/engine_config.hpp include/engine/hint.hpp include/engine/bearing.hpp include/engine/approach.hpp include/engine/phantom_node.hpp) +set(EngineHeader include/engine/status.hpp include/engine/engine_config.hpp include/engine/hint.hpp include/engine/bearing.hpp include/engine/approach.hpp include/engine/phantom_node.hpp include/engine/engine_info.hpp) set(UtilHeader include/util/coordinate.hpp include/util/json_container.hpp include/util/typedefs.hpp include/util/alias.hpp include/util/exception.hpp include/util/bearing.hpp) set(ExtractorHeader include/extractor/extractor.hpp include/storage/io_config.hpp include/extractor/extractor_config.hpp include/extractor/travel_mode.hpp) set(PartitionerHeader include/partitioner/partitioner.hpp include/partitioner/partitioner_config.hpp) @@ -862,4 +862,4 @@ if (ENABLE_NODE_BINDINGS) endforeach() add_library(check-headers STATIC EXCLUDE_FROM_ALL ${sources}) set_target_properties(check-headers PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${check_headers_dir}) -endif() \ No newline at end of file +endif() diff --git a/docs/monitoring.md b/docs/monitoring.md index 7193bf099..e3ce12831 100644 --- a/docs/monitoring.md +++ b/docs/monitoring.md @@ -1,6 +1,6 @@ ## Abstract -OSRM routed daemon publish helath information in prometheus format. +OSRM routed daemon publish health information in prometheus format. This option switched off by default. To enable this feature please set `-P ` option on OSRM routed startup. diff --git a/features/step_definitions/monitoring.js b/features/step_definitions/monitoring.js index fe05d43da..f600c08d3 100644 --- a/features/step_definitions/monitoring.js +++ b/features/step_definitions/monitoring.js @@ -1,7 +1,5 @@ 'use strict'; -var util = require('util'); - module.exports = function () { this.When(/^I monitor I should get$/, (table, callback) => { var got = {}; @@ -16,9 +14,8 @@ module.exports = function () { var metrics = {}; rows.forEach((r) => { if (r.includes('{')) { - var k = r.split('{')[0], - v = r.split('}')[1].split(' ')[1]; - metrics[k] = v; + var k = r.split('{')[0]; + metrics[k] = r.split('}')[1].split(' ')[1]; } else { var kv = r.split(' '); metrics[kv[0]] = kv[1]; @@ -34,13 +31,13 @@ module.exports = function () { } - if (!metrics.hasOwnProperty("osrm_routed_instance_info")) { - throw new Error("Have no instance information inside the monitoring!"); + if (!metrics.hasOwnProperty('osrm_routed_instance_info')) { + throw new Error('Have no instance information inside the monitoring!'); } cb(null, got); }; this.requestMonitoring(afterRequest); - } + }; this.processRowsAndDiff(table, testRow, callback); }); }); diff --git a/features/support/route.js b/features/support/route.js index 8e8a33dc7..ecb00390e 100644 --- a/features/support/route.js +++ b/features/support/route.js @@ -27,7 +27,7 @@ module.exports = function () { if (err) { if (err.statusCode === 408) throw new Error('*** osrm monitoring endpoint did not respond'); else if (err.code === 'ECONNREFUSED') - throw new Error('*** osrm monitoring endpoint is not running', uri); + throw new Error('*** osrm monitoring endpoint is not running'); } else return callback(err, res, body); })); diff --git a/include/monitoring/monitoring.hpp b/include/monitoring/monitoring.hpp index ef6908624..8661bb6d8 100644 --- a/include/monitoring/monitoring.hpp +++ b/include/monitoring/monitoring.hpp @@ -9,6 +9,9 @@ #include #include +#include +#include + namespace osrm { namespace server diff --git a/include/monitoring/monitoring_request_handler.hpp b/include/monitoring/monitoring_request_handler.hpp index 6c76faf0a..701dc44e1 100644 --- a/include/monitoring/monitoring_request_handler.hpp +++ b/include/monitoring/monitoring_request_handler.hpp @@ -3,9 +3,6 @@ #include "server/request_handler.hpp" -#include "util/log.hpp" - -#include #include namespace osrm @@ -18,32 +15,7 @@ class MonitoringRequestHandler : public RequestHandler public: MonitoringRequestHandler(const unsigned _working_threads) : working_threads(_working_threads) {} - void HandleRequest(const http::request &, http::reply ¤t_reply) - { - std::stringstream out_stream; - out_stream << "osrm_routed_instance_info{"; - auto engine_info = service_handler->GetEngineInfo(); - for (auto record : engine_info) - { - out_stream << record.first << "=\"" << record.second << '"'; - out_stream << ','; - } - out_stream << "working_threads=\"" << working_threads << '"'; - out_stream << "} 1\n"; - - auto counters = service_handler->GetUsage(); - for (auto counter : counters) - { - out_stream << "http_requests_count{plugin=\"" << counter.first << "\"} " - << counter.second << "\n"; - } - - out_stream << "workers_busy " << service_handler->GetLoad() << "\n"; - - auto result = out_stream.str(); - current_reply.content.resize(result.size()); - std::copy(result.cbegin(), result.cend(), current_reply.content.begin()); - } + void HandleRequest(const http::request &, http::reply ¤t_reply) override; private: unsigned working_threads; diff --git a/include/osrm/osrm.hpp b/include/osrm/osrm.hpp index c9c455101..f6c5a514d 100644 --- a/include/osrm/osrm.hpp +++ b/include/osrm/osrm.hpp @@ -29,9 +29,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define OSRM_HPP #include "engine/api/base_result.hpp" +#include "osrm/engine_info.hpp" #include "osrm/osrm_fwd.hpp" #include "osrm/status.hpp" -#include "osrm/engine_info.hpp" #include #include @@ -135,11 +135,11 @@ class OSRM final /** * Tile: engine state information for monitoring purposes */ - const EngineInfo & Info() const; + const EngineInfo &Info() const; private: std::unique_ptr engine_; }; -} +} // namespace osrm #endif // OSRM_HPP diff --git a/include/server/request_handler.hpp b/include/server/request_handler.hpp index d657fa948..f5862e364 100644 --- a/include/server/request_handler.hpp +++ b/include/server/request_handler.hpp @@ -26,7 +26,7 @@ class RequestHandler void RegisterServiceHandler(std::shared_ptr service_handler); - //TODO make interface and two siblings + // TODO make interface and two siblings for the regular request handler and for the monitoring virtual void HandleRequest(const http::request ¤t_request, http::reply ¤t_reply); protected: diff --git a/include/server/service/base_service.hpp b/include/server/service/base_service.hpp index fa0ccf9e5..8dca106db 100644 --- a/include/server/service/base_service.hpp +++ b/include/server/service/base_service.hpp @@ -28,7 +28,7 @@ class BaseService RunQuery(std::size_t prefix_length, std::string &query, osrm::engine::api::ResultT &result) = 0; virtual unsigned GetVersion() = 0; - uint32_t GetUsage() {return usage;} + uint32_t GetUsage() { return usage; } protected: OSRM &routing_machine; diff --git a/include/server/service_handler.hpp b/include/server/service_handler.hpp index 8840c28aa..7d2c8ae70 100644 --- a/include/server/service_handler.hpp +++ b/include/server/service_handler.hpp @@ -4,8 +4,8 @@ #include "server/service/base_service.hpp" #include "engine/api/base_api.hpp" -#include "osrm/osrm.hpp" #include "engine/engine_info.hpp" +#include "osrm/osrm.hpp" #include #include @@ -47,7 +47,7 @@ class ServiceHandler final : public ServiceHandlerInterface virtual engine::Status RunQuery(api::ParsedURL parsed_url, ResultT &result) override; - virtual const engine::EngineInfo & GetEngineInfo() const override; + virtual const engine::EngineInfo &GetEngineInfo() const override; virtual const HandlersCounter GetUsage() const override; virtual std::uint32_t GetLoad() const override; diff --git a/src/monitoring/monitoring_request_handler.cpp b/src/monitoring/monitoring_request_handler.cpp new file mode 100644 index 000000000..844f04c08 --- /dev/null +++ b/src/monitoring/monitoring_request_handler.cpp @@ -0,0 +1,43 @@ +#include "monitoring/monitoring_request_handler.hpp" + +#include "server/http/reply.hpp" +#include "server/http/request.hpp" + +#include "util/log.hpp" + +#include + +namespace osrm +{ +namespace server +{ + +void MonitoringRequestHandler::HandleRequest(const http::request &, http::reply ¤t_reply) +{ + std::stringstream out_stream; + out_stream << "osrm_routed_instance_info{"; + auto engine_info = service_handler->GetEngineInfo(); + for (auto record : engine_info) + { + out_stream << record.first << "=\"" << record.second << '"'; + out_stream << ','; + } + out_stream << "working_threads=\"" << working_threads << '"'; + out_stream << "} 1\n"; + + auto counters = service_handler->GetUsage(); + for (auto counter : counters) + { + out_stream << "http_requests_count{plugin=\"" << counter.first << "\"} " << counter.second + << "\n"; + } + + out_stream << "workers_busy " << service_handler->GetLoad() << "\n"; + + auto result = out_stream.str(); + current_reply.content.resize(result.size()); + std::copy(result.cbegin(), result.cend(), current_reply.content.begin()); +} + +} // namespace server +} // namespace osrm diff --git a/src/osrm/osrm.cpp b/src/osrm/osrm.cpp index 92a467919..56c4409ce 100644 --- a/src/osrm/osrm.cpp +++ b/src/osrm/osrm.cpp @@ -92,9 +92,6 @@ engine::Status OSRM::Tile(const engine::api::TileParameters ¶ms, return engine_->Tile(params, result); } -const engine::EngineInfo & OSRM::Info() const -{ - return engine_->GetInfo(); -} +const engine::EngineInfo &OSRM::Info() const { return engine_->GetInfo(); } } // ns osrm diff --git a/src/tools/routed.cpp b/src/tools/routed.cpp index ae19259a1..4fc6ff8b8 100644 --- a/src/tools/routed.cpp +++ b/src/tools/routed.cpp @@ -115,8 +115,7 @@ inline unsigned generateServerProgramOptions(const int argc, ("port,p", value(&ip_port)->default_value(5000), "TCP/IP port") // - ( - "metrics_port,P", + ("metrics_port,P", value(&ip_port_metrics)->default_value(0), "TCP/IP port for prometheus metrics exporter. Leave 0 if do not start server for " "metrics") // @@ -338,10 +337,8 @@ int main(int argc, const char *argv[]) try #else // Set console control handler to allow server to be stopped. console_ctrl_function = std::bind(&server::Server::Stop, routing_server); - console_ctrl_function_monitoring = - std::bind(&monitoring::Monitoring::Stop, monitoring_server); + console_ctrl_function_monitoring = std::bind(&server::Monitoring::Stop, monitoring_server); SetConsoleCtrlHandler(console_ctrl_handler, TRUE); - SetConsoleCtrlHandler(console_ctrl_handler_monitoring, TRUE); util::Log() << "running and waiting for requests"; monitoring_server->Run(); routing_server->Run();