From 80b765542ff1b97165e5426235d72fb3bcd042d7 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Fri, 12 Jun 2020 18:04:42 +0800 Subject: [PATCH 1/7] ISSUES-7572 support config default http handler --- src/Server/HTTPHandlerFactory.cpp | 72 ++++++++++--------------- src/Server/HTTPHandlerFactory.h | 10 ++++ src/Server/PrometheusRequestHandler.cpp | 14 +++++ src/Server/ReplicasStatusHandler.cpp | 8 +++ src/Server/StaticRequestHandler.cpp | 16 ++++++ 5 files changed, 77 insertions(+), 43 deletions(-) diff --git a/src/Server/HTTPHandlerFactory.cpp b/src/Server/HTTPHandlerFactory.cpp index 2f00aa0aa72..ec75656a9a8 100644 --- a/src/Server/HTTPHandlerFactory.cpp +++ b/src/Server/HTTPHandlerFactory.cpp @@ -1,9 +1,5 @@ #include "HTTPHandlerFactory.h" -#include -#include -#include -#include #include #include "HTTPHandler.h" @@ -68,7 +64,8 @@ HTTPRequestHandlerFactoryMain::TThis * HTTPRequestHandlerFactoryMain::addHandler return this; } -static inline auto createHandlersFactoryFromConfig(IServer & server, const std::string & name, const String & prefix) +static inline auto createHandlersFactoryFromConfig( + IServer & server, const std::string & name, const String & prefix, AsynchronousMetrics & async_metrics) { auto main_handler_factory = std::make_unique(name); @@ -82,7 +79,17 @@ static inline auto createHandlersFactoryFromConfig(IServer & server, const std:: const auto & handler_type = server.config().getString(prefix + "." + key + ".handler.type", ""); - if (handler_type == "static") + if (handler_type == "root") + addRootHandlerFactory(*main_handler_factory, server); + else if (handler_type == "ping") + addPingHandlerFactory(*main_handler_factory, server); + else if (handler_type == "defaults") + addDefaultHandlersFactory(*main_handler_factory, server, async_metrics); + else if (handler_type == "prometheus") + addPrometheusHandlerFactory(*main_handler_factory, server, async_metrics); + else if (handler_type == "replicas_status") + addReplicasStatusHandlerFactory(*main_handler_factory, server); + else if (handler_type == "static") main_handler_factory->addHandler(createStaticHandlerFactory(server, prefix + "." + key)); else if (handler_type == "dynamic_query_handler") main_handler_factory->addHandler(createDynamicHandlerFactory(server, prefix + "." + key)); @@ -99,44 +106,23 @@ static inline auto createHandlersFactoryFromConfig(IServer & server, const std:: return main_handler_factory.release(); } -static const auto ping_response_expression = "Ok.\n"; -static const auto root_response_expression = "config://http_server_default_response"; - static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory( IServer & server, const std::string & name, AsynchronousMetrics & async_metrics) { if (server.config().has("http_handlers")) - return createHandlersFactoryFromConfig(server, name, "http_handlers"); + return createHandlersFactoryFromConfig(server, name, "http_handlers", async_metrics); else { auto factory = std::make_unique(name); - auto root_handler = std::make_unique>(server, root_response_expression); - root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); - factory->addHandler(root_handler.release()); - - auto ping_handler = std::make_unique>(server, ping_response_expression); - ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); - factory->addHandler(ping_handler.release()); - - auto replicas_status_handler = std::make_unique>(server); - replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); - factory->addHandler(replicas_status_handler.release()); + addRootHandlerFactory(*factory, server); + addPingHandlerFactory(*factory, server); + addReplicasStatusHandlerFactory(*factory, server); + addPrometheusHandlerFactory(*factory, server, async_metrics); auto query_handler = std::make_unique>(server, "query"); query_handler->allowPostAndGetParamsRequest(); factory->addHandler(query_handler.release()); - - /// We check that prometheus handler will be served on current (default) port. - /// Otherwise it will be created separately, see below. - if (server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) - { - auto prometheus_handler = std::make_unique>( - server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics)); - prometheus_handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); - factory->addHandler(prometheus_handler.release()); - } - return factory.release(); } } @@ -145,17 +131,9 @@ static inline Poco::Net::HTTPRequestHandlerFactory * createInterserverHTTPHandle { auto factory = std::make_unique(name); - auto root_handler = std::make_unique>(server, root_response_expression); - root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); - factory->addHandler(root_handler.release()); - - auto ping_handler = std::make_unique>(server, ping_response_expression); - ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); - factory->addHandler(ping_handler.release()); - - auto replicas_status_handler = std::make_unique>(server); - replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); - factory->addHandler(replicas_status_handler.release()); + addRootHandlerFactory(*factory, server); + addPingHandlerFactory(*factory, server); + addReplicasStatusHandlerFactory(*factory, server); auto main_handler = std::make_unique>(server); main_handler->allowPostAndGetParamsRequest(); @@ -183,4 +161,12 @@ Poco::Net::HTTPRequestHandlerFactory * createHandlerFactory(IServer & server, As throw Exception("LOGICAL ERROR: Unknown HTTP handler factory name.", ErrorCodes::LOGICAL_ERROR); } +void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics) +{ + addRootHandlerFactory(factory, server); + addPingHandlerFactory(factory, server); + addReplicasStatusHandlerFactory(factory, server); + addPrometheusHandlerFactory(factory, server, async_metrics); +} + } diff --git a/src/Server/HTTPHandlerFactory.h b/src/Server/HTTPHandlerFactory.h index 273e337813e..ac3a7451338 100644 --- a/src/Server/HTTPHandlerFactory.h +++ b/src/Server/HTTPHandlerFactory.h @@ -103,6 +103,16 @@ private: std::function creator; }; +void addRootHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); + +void addPingHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); + +void addReplicasStatusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); + +void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics); + +void addPrometheusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics); + Poco::Net::HTTPRequestHandlerFactory * createStaticHandlerFactory(IServer & server, const std::string & config_prefix); Poco::Net::HTTPRequestHandlerFactory * createDynamicHandlerFactory(IServer & server, const std::string & config_prefix); diff --git a/src/Server/PrometheusRequestHandler.cpp b/src/Server/PrometheusRequestHandler.cpp index 43f39e36de8..0f5df54b002 100644 --- a/src/Server/PrometheusRequestHandler.cpp +++ b/src/Server/PrometheusRequestHandler.cpp @@ -12,6 +12,7 @@ #include #include +#include namespace DB @@ -40,4 +41,17 @@ void PrometheusRequestHandler::handleRequest( } } +void addPrometheusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics) +{ + /// We check that prometheus handler will be served on current (default) port. + /// Otherwise it will be created separately, see below. + if (server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) + { + auto prometheus_handler = std::make_unique>( + server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics)); + prometheus_handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); + factory.addHandler(prometheus_handler.release()); + } +} + } diff --git a/src/Server/ReplicasStatusHandler.cpp b/src/Server/ReplicasStatusHandler.cpp index 57c97b0e4e0..9b3e00cc069 100644 --- a/src/Server/ReplicasStatusHandler.cpp +++ b/src/Server/ReplicasStatusHandler.cpp @@ -7,8 +7,10 @@ #include #include +#include #include #include +#include namespace DB @@ -104,5 +106,11 @@ void ReplicasStatusHandler::handleRequest(Poco::Net::HTTPServerRequest & request } } +void addReplicasStatusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) +{ + auto replicas_status_handler = std::make_unique>(server); + replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); + factory->addHandler(replicas_status_handler.release()); +} } diff --git a/src/Server/StaticRequestHandler.cpp b/src/Server/StaticRequestHandler.cpp index 22f32e6a0e7..255e3cab5af 100644 --- a/src/Server/StaticRequestHandler.cpp +++ b/src/Server/StaticRequestHandler.cpp @@ -155,6 +155,22 @@ StaticRequestHandler::StaticRequestHandler(IServer & server_, const String & exp { } +void addRootHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) +{ + static const auto root_response_expression = "config://http_server_default_response"; + + auto root_handler = std::make_unique>(server, root_response_expression); + root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); + factory.addHandler(root_handler.release()); +} + +void addPingHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) +{ + auto ping_handler = std::make_unique>(server, "Ok.\n"); + ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); + factory.addHandler(ping_handler.release()); +} + Poco::Net::HTTPRequestHandlerFactory * createStaticHandlerFactory(IServer & server, const std::string & config_prefix) { int status = server.config().getInt(config_prefix + ".handler.status", 200); From 1c55aa03334f4c011c5c587468641b873ffcd83d Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Fri, 12 Jun 2020 18:19:03 +0800 Subject: [PATCH 2/7] ISSUES-7572 add integration test --- .../test_http_handlers_config/test.py | 20 +++++++++++++++++++ .../test_custom_defaults_handlers/config.xml | 10 ++++++++++ .../test_defaults_handlers/config.xml | 9 +++++++++ 3 files changed, 39 insertions(+) create mode 100644 tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml create mode 100644 tests/integration/test_http_handlers_config/test_defaults_handlers/config.xml diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index 31d40bd8a1d..a38bd3ff343 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -113,3 +113,23 @@ def test_relative_path_static_handler(): assert 'text/html; charset=UTF-8' == cluster.instance.http_request('test_get_relative_path_static_handler', method='GET', headers={'XXX': 'xxx'}).headers['Content-Type'] assert 'Relative Path File\n' == cluster.instance.http_request('test_get_relative_path_static_handler', method='GET', headers={'XXX': 'xxx'}).content +def test_defaults_http_handlers(): + with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "defaults_handlers", "test_defaults_handlers")) as cluster: + assert 200 == cluster.instance.http_request('', method='GET').status_code + assert 'Default server response' == cluster.instance.http_request('', method='GET').content + + assert 200 == cluster.instance.http_request('ping', method='GET').status_code + assert 'Ok\n' == cluster.instance.http_request('ping', method='GET').content + + assert 200 == cluster.instance.http_request('replicas_status', method='GET').status_code + assert 'Ok\n' == cluster.instance.http_request('replicas_status', method='GET').content + +def test_custom_defaults_http_handlers(): + with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "custom_defaults_handlers", "test_custom_defaults_handlers")) as cluster: + assert 200 == cluster.instance.http_request('', method='GET').status_code + assert 'Default server response' == cluster.instance.http_request('', method='GET').content + + assert 200 == cluster.instance.http_request('ping', method='GET').status_code + assert 'Ok\n' == cluster.instance.http_request('ping', method='GET').content + + assert 404 == cluster.instance.http_request('replicas_status', method='GET').status_code diff --git a/tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml b/tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml new file mode 100644 index 00000000000..54008c2c4b8 --- /dev/null +++ b/tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml @@ -0,0 +1,10 @@ + + + + Default server response + + + + + + diff --git a/tests/integration/test_http_handlers_config/test_defaults_handlers/config.xml b/tests/integration/test_http_handlers_config/test_defaults_handlers/config.xml new file mode 100644 index 00000000000..fd280e05cf4 --- /dev/null +++ b/tests/integration/test_http_handlers_config/test_defaults_handlers/config.xml @@ -0,0 +1,9 @@ + + + + Default server response + + + + + From 2c439afc015b15d9babb632b423991aeea0f397f Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Fri, 12 Jun 2020 19:17:34 +0800 Subject: [PATCH 3/7] ISSUES-7572 fix build failure --- src/Server/ReplicasStatusHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Server/ReplicasStatusHandler.cpp b/src/Server/ReplicasStatusHandler.cpp index 9b3e00cc069..d6bbfdbd090 100644 --- a/src/Server/ReplicasStatusHandler.cpp +++ b/src/Server/ReplicasStatusHandler.cpp @@ -110,7 +110,7 @@ void addReplicasStatusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IS { auto replicas_status_handler = std::make_unique>(server); replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); - factory->addHandler(replicas_status_handler.release()); + factory.addHandler(replicas_status_handler.release()); } } From 8d9b770da4a74a16c405f90df669bf1d34135fc5 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Sat, 13 Jun 2020 00:15:02 +0800 Subject: [PATCH 4/7] ISSUES-7572 fix defaults config level & add replicas_status and prometheus handler --- src/Server/HTTPHandlerFactory.cpp | 93 +++++++++++-------- src/Server/HTTPHandlerFactory.h | 14 +-- src/Server/PrometheusRequestHandler.cpp | 15 +-- src/Server/ReplicasStatusHandler.cpp | 6 +- src/Server/StaticRequestHandler.cpp | 16 ---- .../test_http_handlers_config/test.py | 26 ++++-- .../test_custom_defaults_handlers/config.xml | 10 -- .../test_prometheus_handler/config.xml | 17 ++++ .../test_replicas_status_handler/config.xml | 12 +++ 9 files changed, 112 insertions(+), 97 deletions(-) delete mode 100644 tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml create mode 100644 tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml create mode 100644 tests/integration/test_http_handlers_config/test_replicas_status_handler/config.xml diff --git a/src/Server/HTTPHandlerFactory.cpp b/src/Server/HTTPHandlerFactory.cpp index ec75656a9a8..6459b0aab3b 100644 --- a/src/Server/HTTPHandlerFactory.cpp +++ b/src/Server/HTTPHandlerFactory.cpp @@ -74,55 +74,51 @@ static inline auto createHandlersFactoryFromConfig( for (const auto & key : keys) { - if (!startsWith(key, "rule")) - throw Exception("Unknown element in config: " + prefix + "." + key + ", must be 'rule'", ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + if (key == "defaults") + addDefaultHandlersFactory(*main_handler_factory, server, &async_metrics); + else if (startsWith(key, "rule")) + { + const auto & handler_type = server.config().getString(prefix + "." + key + ".handler.type", ""); - const auto & handler_type = server.config().getString(prefix + "." + key + ".handler.type", ""); + if (handler_type.empty()) + throw Exception("Handler type in config is not specified here: " + prefix + "." + key + ".handler.type", + ErrorCodes::INVALID_CONFIG_PARAMETER); - if (handler_type == "root") - addRootHandlerFactory(*main_handler_factory, server); - else if (handler_type == "ping") - addPingHandlerFactory(*main_handler_factory, server); - else if (handler_type == "defaults") - addDefaultHandlersFactory(*main_handler_factory, server, async_metrics); - else if (handler_type == "prometheus") - addPrometheusHandlerFactory(*main_handler_factory, server, async_metrics); - else if (handler_type == "replicas_status") - addReplicasStatusHandlerFactory(*main_handler_factory, server); - else if (handler_type == "static") - main_handler_factory->addHandler(createStaticHandlerFactory(server, prefix + "." + key)); - else if (handler_type == "dynamic_query_handler") - main_handler_factory->addHandler(createDynamicHandlerFactory(server, prefix + "." + key)); - else if (handler_type == "predefined_query_handler") - main_handler_factory->addHandler(createPredefinedHandlerFactory(server, prefix + "." + key)); - else if (handler_type.empty()) - throw Exception("Handler type in config is not specified here: " + - prefix + "." + key + ".handler.type", ErrorCodes::INVALID_CONFIG_PARAMETER); + if (handler_type == "static") + main_handler_factory->addHandler(createStaticHandlerFactory(server, prefix + "." + key)); + else if (handler_type == "dynamic_query_handler") + main_handler_factory->addHandler(createDynamicHandlerFactory(server, prefix + "." + key)); + else if (handler_type == "predefined_query_handler") + main_handler_factory->addHandler(createPredefinedHandlerFactory(server, prefix + "." + key)); + else if (handler_type == "prometheus") + main_handler_factory->addHandler(createPrometheusHandlerFactory(server, async_metrics, prefix + "." + key)); + else if (handler_type == "replicas_status") + main_handler_factory->addHandler(createReplicasStatusHandlerFactory(server, prefix + "." + key)); + else + throw Exception("Unknown handler type '" + handler_type + "' in config here: " + prefix + "." + key + ".handler.type", + ErrorCodes::INVALID_CONFIG_PARAMETER); + } else - throw Exception("Unknown handler type '" + handler_type +"' in config here: " + - prefix + "." + key + ".handler.type",ErrorCodes::INVALID_CONFIG_PARAMETER); + throw Exception("Unknown element in config: " + prefix + "." + key + ", must be 'rule' or 'defaults'", + ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); } return main_handler_factory.release(); } -static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory( - IServer & server, const std::string & name, AsynchronousMetrics & async_metrics) +static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory(IServer & server, const std::string & name, AsynchronousMetrics & async_metrics) { if (server.config().has("http_handlers")) return createHandlersFactoryFromConfig(server, name, "http_handlers", async_metrics); else { auto factory = std::make_unique(name); - - addRootHandlerFactory(*factory, server); - addPingHandlerFactory(*factory, server); - addReplicasStatusHandlerFactory(*factory, server); - addPrometheusHandlerFactory(*factory, server, async_metrics); + addDefaultHandlersFactory(*factory, server, &async_metrics); auto query_handler = std::make_unique>(server, "query"); query_handler->allowPostAndGetParamsRequest(); factory->addHandler(query_handler.release()); + return factory.release(); } } @@ -130,10 +126,7 @@ static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory( static inline Poco::Net::HTTPRequestHandlerFactory * createInterserverHTTPHandlerFactory(IServer & server, const std::string & name) { auto factory = std::make_unique(name); - - addRootHandlerFactory(*factory, server); - addPingHandlerFactory(*factory, server); - addReplicasStatusHandlerFactory(*factory, server); + addDefaultHandlersFactory(*factory, server, nullptr); auto main_handler = std::make_unique>(server); main_handler->allowPostAndGetParamsRequest(); @@ -161,12 +154,32 @@ Poco::Net::HTTPRequestHandlerFactory * createHandlerFactory(IServer & server, As throw Exception("LOGICAL ERROR: Unknown HTTP handler factory name.", ErrorCodes::LOGICAL_ERROR); } -void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics) +static const auto ping_response_expression = "Ok.\n"; +static const auto root_response_expression = "config://http_server_default_response"; + +void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics * async_metrics) { - addRootHandlerFactory(factory, server); - addPingHandlerFactory(factory, server); - addReplicasStatusHandlerFactory(factory, server); - addPrometheusHandlerFactory(factory, server, async_metrics); + auto root_handler = std::make_unique>(server, root_response_expression); + root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); + factory.addHandler(root_handler.release()); + + auto ping_handler = std::make_unique>(server, ping_response_expression); + ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); + factory.addHandler(ping_handler.release()); + + auto replicas_status_handler = std::make_unique>(server); + replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); + factory.addHandler(replicas_status_handler.release()); + + /// We check that prometheus handler will be served on current (default) port. + /// Otherwise it will be created separately, see below. + if (async_metrics && server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) + { + auto prometheus_handler = std::make_unique>( + server, PrometheusMetricsWriter(server.config(), "prometheus", *async_metrics)); + prometheus_handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); + factory.addHandler(prometheus_handler.release()); + } } } diff --git a/src/Server/HTTPHandlerFactory.h b/src/Server/HTTPHandlerFactory.h index ac3a7451338..8e21a13ba18 100644 --- a/src/Server/HTTPHandlerFactory.h +++ b/src/Server/HTTPHandlerFactory.h @@ -103,15 +103,7 @@ private: std::function creator; }; -void addRootHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); - -void addPingHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); - -void addReplicasStatusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); - -void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics); - -void addPrometheusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics); +void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics * async_metrics); Poco::Net::HTTPRequestHandlerFactory * createStaticHandlerFactory(IServer & server, const std::string & config_prefix); @@ -119,6 +111,10 @@ Poco::Net::HTTPRequestHandlerFactory * createDynamicHandlerFactory(IServer & ser Poco::Net::HTTPRequestHandlerFactory * createPredefinedHandlerFactory(IServer & server, const std::string & config_prefix); +Poco::Net::HTTPRequestHandlerFactory * createReplicasStatusHandlerFactory(IServer & server, const std::string & config_prefix); + +Poco::Net::HTTPRequestHandlerFactory * createPrometheusHandlerFactory(IServer & server, AsynchronousMetrics & async_metrics, const std::string & config_prefix); + Poco::Net::HTTPRequestHandlerFactory * createHandlerFactory(IServer & server, AsynchronousMetrics & async_metrics, const std::string & name); } diff --git a/src/Server/PrometheusRequestHandler.cpp b/src/Server/PrometheusRequestHandler.cpp index 0f5df54b002..60deec9b289 100644 --- a/src/Server/PrometheusRequestHandler.cpp +++ b/src/Server/PrometheusRequestHandler.cpp @@ -12,7 +12,7 @@ #include #include -#include +#include namespace DB @@ -41,17 +41,10 @@ void PrometheusRequestHandler::handleRequest( } } -void addPrometheusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics) +Poco::Net::HTTPRequestHandlerFactory * createPrometheusHandlerFactory(IServer & server, AsynchronousMetrics & async_metrics, const std::string & config_prefix) { - /// We check that prometheus handler will be served on current (default) port. - /// Otherwise it will be created separately, see below. - if (server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) - { - auto prometheus_handler = std::make_unique>( - server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics)); - prometheus_handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); - factory.addHandler(prometheus_handler.release()); - } + return addFiltersFromConfig(new HandlingRuleHTTPHandlerFactory( + server, PrometheusMetricsWriter(server.config(), config_prefix + ".handler", async_metrics)), server.config(), config_prefix); } } diff --git a/src/Server/ReplicasStatusHandler.cpp b/src/Server/ReplicasStatusHandler.cpp index d6bbfdbd090..3606da23ab5 100644 --- a/src/Server/ReplicasStatusHandler.cpp +++ b/src/Server/ReplicasStatusHandler.cpp @@ -106,11 +106,9 @@ void ReplicasStatusHandler::handleRequest(Poco::Net::HTTPServerRequest & request } } -void addReplicasStatusHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) +Poco::Net::HTTPRequestHandlerFactory * createReplicasStatusHandlerFactory(IServer & server, const std::string & config_prefix) { - auto replicas_status_handler = std::make_unique>(server); - replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); - factory.addHandler(replicas_status_handler.release()); + return addFiltersFromConfig(new HandlingRuleHTTPHandlerFactory(server), server.config(), config_prefix); } } diff --git a/src/Server/StaticRequestHandler.cpp b/src/Server/StaticRequestHandler.cpp index 255e3cab5af..22f32e6a0e7 100644 --- a/src/Server/StaticRequestHandler.cpp +++ b/src/Server/StaticRequestHandler.cpp @@ -155,22 +155,6 @@ StaticRequestHandler::StaticRequestHandler(IServer & server_, const String & exp { } -void addRootHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) -{ - static const auto root_response_expression = "config://http_server_default_response"; - - auto root_handler = std::make_unique>(server, root_response_expression); - root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); - factory.addHandler(root_handler.release()); -} - -void addPingHandlerFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) -{ - auto ping_handler = std::make_unique>(server, "Ok.\n"); - ping_handler->attachStrictPath("/ping")->allowGetAndHeadRequest(); - factory.addHandler(ping_handler.release()); -} - Poco::Net::HTTPRequestHandlerFactory * createStaticHandlerFactory(IServer & server, const std::string & config_prefix) { int status = server.config().getInt(config_prefix + ".handler.status", 200); diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index a38bd3ff343..c18c22acbb2 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -124,12 +124,24 @@ def test_defaults_http_handlers(): assert 200 == cluster.instance.http_request('replicas_status', method='GET').status_code assert 'Ok\n' == cluster.instance.http_request('replicas_status', method='GET').content -def test_custom_defaults_http_handlers(): - with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "custom_defaults_handlers", "test_custom_defaults_handlers")) as cluster: - assert 200 == cluster.instance.http_request('', method='GET').status_code - assert 'Default server response' == cluster.instance.http_request('', method='GET').content +def test_prometheus_handler(): + with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "prometheus_handler", "test_prometheus_handler")) as cluster: + assert 404 == cluster.instance.http_request('', method='GET', headers={'XXX': 'xxx'}).status_code - assert 200 == cluster.instance.http_request('ping', method='GET').status_code - assert 'Ok\n' == cluster.instance.http_request('ping', method='GET').content + assert 404 == cluster.instance.http_request('test_prometheus', method='GET', headers={'XXX': 'bad'}).status_code - assert 404 == cluster.instance.http_request('replicas_status', method='GET').status_code + assert 404 == cluster.instance.http_request('test_prometheus', method='POST', headers={'XXX': 'xxx'}).status_code + + assert 200 == cluster.instance.http_request('test_prometheus', method='GET', headers={'XXX': 'xxx'}).status_code + assert 'ClickHouseProfileEvents_Query' in cluster.instance.http_request('test_prometheus', method='GET', headers={'XXX': 'xxx'}).content + +def test_replicas_status_handler(): + with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "replicas_status_handler", "test_replicas_status_handler")) as cluster: + assert 404 == cluster.instance.http_request('', method='GET', headers={'XXX': 'xxx'}).status_code + + assert 404 == cluster.instance.http_request('test_replicas_status', method='GET', headers={'XXX': 'bad'}).status_code + + assert 404 == cluster.instance.http_request('test_replicas_status', method='POST', headers={'XXX': 'xxx'}).status_code + + assert 200 == cluster.instance.http_request('test_replicas_status', method='GET', headers={'XXX': 'xxx'}).status_code + assert 'Ok\n' == cluster.instance.http_request('test_replicas_status', method='GET', headers={'XXX': 'xxx'}).content diff --git a/tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml b/tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml deleted file mode 100644 index 54008c2c4b8..00000000000 --- a/tests/integration/test_http_handlers_config/test_custom_defaults_handlers/config.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - Default server response - - - - - - diff --git a/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml b/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml new file mode 100644 index 00000000000..7c80649cee2 --- /dev/null +++ b/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml @@ -0,0 +1,17 @@ + + + + + + GET + xxx + /test_prometheus + + replicas_status + true + true + true + + + + diff --git a/tests/integration/test_http_handlers_config/test_replicas_status_handler/config.xml b/tests/integration/test_http_handlers_config/test_replicas_status_handler/config.xml new file mode 100644 index 00000000000..21f7d3b0fc8 --- /dev/null +++ b/tests/integration/test_http_handlers_config/test_replicas_status_handler/config.xml @@ -0,0 +1,12 @@ + + + + + + GET + xxx + /test_replicas_status + replicas_status + + + From de96296e019887f925c998bef97e42bafaa964e8 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Sat, 13 Jun 2020 10:17:02 +0800 Subject: [PATCH 5/7] ISSUES-7572 fix build failure --- src/Server/ReplicasStatusHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Server/ReplicasStatusHandler.cpp b/src/Server/ReplicasStatusHandler.cpp index 3606da23ab5..5ead756ee1e 100644 --- a/src/Server/ReplicasStatusHandler.cpp +++ b/src/Server/ReplicasStatusHandler.cpp @@ -11,6 +11,7 @@ #include #include #include +#include namespace DB From e1317ef8ae6acc7ac40c3b6985bbd97828880dd2 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Sun, 14 Jun 2020 09:44:05 +0800 Subject: [PATCH 6/7] ISSUES-7572 fix test failure --- tests/integration/test_http_handlers_config/test.py | 6 +++--- .../test_prometheus_handler/config.xml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index c18c22acbb2..b31913ba962 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -119,10 +119,10 @@ def test_defaults_http_handlers(): assert 'Default server response' == cluster.instance.http_request('', method='GET').content assert 200 == cluster.instance.http_request('ping', method='GET').status_code - assert 'Ok\n' == cluster.instance.http_request('ping', method='GET').content + assert 'Ok.\n' == cluster.instance.http_request('ping', method='GET').content assert 200 == cluster.instance.http_request('replicas_status', method='GET').status_code - assert 'Ok\n' == cluster.instance.http_request('replicas_status', method='GET').content + assert 'Ok.\n' == cluster.instance.http_request('replicas_status', method='GET').content def test_prometheus_handler(): with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "prometheus_handler", "test_prometheus_handler")) as cluster: @@ -144,4 +144,4 @@ def test_replicas_status_handler(): assert 404 == cluster.instance.http_request('test_replicas_status', method='POST', headers={'XXX': 'xxx'}).status_code assert 200 == cluster.instance.http_request('test_replicas_status', method='GET', headers={'XXX': 'xxx'}).status_code - assert 'Ok\n' == cluster.instance.http_request('test_replicas_status', method='GET', headers={'XXX': 'xxx'}).content + assert 'Ok.\n' == cluster.instance.http_request('test_replicas_status', method='GET', headers={'XXX': 'xxx'}).content diff --git a/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml b/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml index 7c80649cee2..8ace97a66dc 100644 --- a/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml +++ b/tests/integration/test_http_handlers_config/test_prometheus_handler/config.xml @@ -7,7 +7,7 @@ xxx /test_prometheus - replicas_status + prometheus true true true From def0158638b82c6d2d38ceb80daec6f74b992a15 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Mon, 15 Jun 2020 14:33:44 +0300 Subject: [PATCH 7/7] configure query handler as default --- src/Server/HTTPHandlerFactory.cpp | 31 ++++++++++++------- src/Server/HTTPHandlerFactory.h | 2 -- .../test_http_handlers_config/test.py | 3 ++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/Server/HTTPHandlerFactory.cpp b/src/Server/HTTPHandlerFactory.cpp index 6459b0aab3b..f34852054d1 100644 --- a/src/Server/HTTPHandlerFactory.cpp +++ b/src/Server/HTTPHandlerFactory.cpp @@ -20,6 +20,9 @@ namespace ErrorCodes extern const int INVALID_CONFIG_PARAMETER; } +static void addCommonDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server); +static void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics); + HTTPRequestHandlerFactoryMain::HTTPRequestHandlerFactoryMain(const std::string & name_) : log(&Poco::Logger::get(name_)), name(name_) { @@ -75,7 +78,7 @@ static inline auto createHandlersFactoryFromConfig( for (const auto & key : keys) { if (key == "defaults") - addDefaultHandlersFactory(*main_handler_factory, server, &async_metrics); + addDefaultHandlersFactory(*main_handler_factory, server, async_metrics); else if (startsWith(key, "rule")) { const auto & handler_type = server.config().getString(prefix + "." + key + ".handler.type", ""); @@ -113,12 +116,7 @@ static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory(IS else { auto factory = std::make_unique(name); - addDefaultHandlersFactory(*factory, server, &async_metrics); - - auto query_handler = std::make_unique>(server, "query"); - query_handler->allowPostAndGetParamsRequest(); - factory->addHandler(query_handler.release()); - + addDefaultHandlersFactory(*factory, server, async_metrics); return factory.release(); } } @@ -126,7 +124,7 @@ static inline Poco::Net::HTTPRequestHandlerFactory * createHTTPHandlerFactory(IS static inline Poco::Net::HTTPRequestHandlerFactory * createInterserverHTTPHandlerFactory(IServer & server, const std::string & name) { auto factory = std::make_unique(name); - addDefaultHandlersFactory(*factory, server, nullptr); + addCommonDefaultHandlersFactory(*factory, server); auto main_handler = std::make_unique>(server); main_handler->allowPostAndGetParamsRequest(); @@ -157,7 +155,7 @@ Poco::Net::HTTPRequestHandlerFactory * createHandlerFactory(IServer & server, As static const auto ping_response_expression = "Ok.\n"; static const auto root_response_expression = "config://http_server_default_response"; -void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics * async_metrics) +void addCommonDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server) { auto root_handler = std::make_unique>(server, root_response_expression); root_handler->attachStrictPath("/")->allowGetAndHeadRequest(); @@ -170,13 +168,22 @@ void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer auto replicas_status_handler = std::make_unique>(server); replicas_status_handler->attachNonStrictPath("/replicas_status")->allowGetAndHeadRequest(); factory.addHandler(replicas_status_handler.release()); +} + +void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics & async_metrics) +{ + addCommonDefaultHandlersFactory(factory, server); + + auto query_handler = std::make_unique>(server, "query"); + query_handler->allowPostAndGetParamsRequest(); + factory.addHandler(query_handler.release()); /// We check that prometheus handler will be served on current (default) port. - /// Otherwise it will be created separately, see below. - if (async_metrics && server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) + /// Otherwise it will be created separately, see createHandlerFactory(...). + if (server.config().has("prometheus") && server.config().getInt("prometheus.port", 0) == 0) { auto prometheus_handler = std::make_unique>( - server, PrometheusMetricsWriter(server.config(), "prometheus", *async_metrics)); + server, PrometheusMetricsWriter(server.config(), "prometheus", async_metrics)); prometheus_handler->attachStrictPath(server.config().getString("prometheus.endpoint", "/metrics"))->allowGetAndHeadRequest(); factory.addHandler(prometheus_handler.release()); } diff --git a/src/Server/HTTPHandlerFactory.h b/src/Server/HTTPHandlerFactory.h index 8e21a13ba18..3e8313172eb 100644 --- a/src/Server/HTTPHandlerFactory.h +++ b/src/Server/HTTPHandlerFactory.h @@ -103,8 +103,6 @@ private: std::function creator; }; -void addDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IServer & server, AsynchronousMetrics * async_metrics); - Poco::Net::HTTPRequestHandlerFactory * createStaticHandlerFactory(IServer & server, const std::string & config_prefix); Poco::Net::HTTPRequestHandlerFactory * createDynamicHandlerFactory(IServer & server, const std::string & config_prefix); diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index b31913ba962..b15cd1fdb89 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -124,6 +124,9 @@ def test_defaults_http_handlers(): assert 200 == cluster.instance.http_request('replicas_status', method='GET').status_code assert 'Ok.\n' == cluster.instance.http_request('replicas_status', method='GET').content + assert 200 == cluster.instance.http_request('?query=SELECT+1', method='GET').status_code + assert '1\n' == cluster.instance.http_request('?query=SELECT+1', method='GET').content + def test_prometheus_handler(): with contextlib.closing(SimpleCluster(ClickHouseCluster(__file__), "prometheus_handler", "test_prometheus_handler")) as cluster: assert 404 == cluster.instance.http_request('', method='GET', headers={'XXX': 'xxx'}).status_code