This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new e7b9b831e refactor(HTTP): Improve the help description of HTTP APIs
(#1959)
e7b9b831e is described below
commit e7b9b831e4b999236e2ebced370090321b5dbf28
Author: Yingchun Lai <[email protected]>
AuthorDate: Mon Apr 8 15:23:39 2024 +0800
refactor(HTTP): Improve the help description of HTTP APIs (#1959)
This patch improves the help description of HTTP APIs.
Before this patch, the path and parameters are output. The path is duplicate
because we can obtain all the paths directly (e.g. `curl 127.0.0.1:34601/`).
After this patch, the help description is more readable and we don't have to
maintain the website docs, e.g.
```
$ curl 127.0.0.1:34601/
{
"/": "List all supported calls.",
"/config": "name=<config_name>. Gets the details of a specified config.
Only the configs which are registered by DSN_DEFINE_xxx macro can be queried.",
"/configs": "List all configs. Only the configs which are registered by
DSN_DEFINE_xxx macro can be queried.",
"/meta/app": "name=<app_name>[&detail]. Query app info.",
"/meta/app/duplication": "name=<app_name>. Query app duplication info.",
"/meta/app/query_bulk_load": "name=<app_name>. Query app bulk load
info.",
"/meta/app/start_bulk_load": "A JSON format of start_bulk_load_request
structure. Start bulk load on an app.",
"/meta/app/start_compaction": "A JSON format of manual_compaction_info
structure. Start compaction for an app.",
"/meta/app/usage_scenario": "A JSON format of usage_scenario_info
structure. Update usage scenario of an app.",
"/meta/app_envs": "name=<app_name>. Query app environments.",
"/meta/apps": "[detail]. List all apps in the cluster.",
"/meta/backup_policy": "name=<app_name1>&name=<app_name2>. Query backup
policy by policy names.",
"/meta/cluster": "Query the cluster info.",
"/meta/nodes": "[detail]. Query the replica servers info.",
"/metrics":
"[with_metric_fields=field1,field2,...][&types=type1,type2,...][&ids=id1,id2,...][&attributes=attr1,value1,attr2,value2,...][&metrics=metric1,metric2,...][&detail=true|false]Query
the node metrics.",
"/pprof/cmdline": "Query the process' cmdline.",
"/pprof/growth": "Query the stack traces that caused growth in the
address space size.",
"/pprof/heap": "[seconds=<heap_profile_seconds>]. Query a sample of
live objects and the stack traces that allocated these objects (an environment
variable TCMALLOC_SAMPLE_PARAMETER should set to a positive value, such as
524288), or the current heap profiling information if 'seconds' parameter is
specified.",
"/pprof/profile": "[seconds=<cpu_profile_seconds>]. Query the CPU
profile. 'seconds' is 60 if not specified.",
"/pprof/symbol": "[symbol_address]. Query the process' symbols. Return
the symbol count of the process if using GET, return the symbol of the
'symbol_address' if using POST.",
"/recentStartTime": "Get the server start time.",
"/updateConfig": "<key>=<new_value>. Update the config to the new
value.",
"/version": "Get the server version."
}
```
or
```
$ curl 127.0.0.1:34801/
{
"/": "List all supported calls.",
"/config": "name=<config_name>. Gets the details of a specified config.
Only the configs which are registered by DSN_DEFINE_xxx macro can be queried.",
"/configs": "List all configs. Only the configs which are registered by
DSN_DEFINE_xxx macro can be queried.",
"/metrics":
"[with_metric_fields=field1,field2,...][&types=type1,type2,...][&ids=id1,id2,...][&attributes=attr1,value1,attr2,value2,...][&metrics=metric1,metric2,...][&detail=true|false]Query
the node metrics.",
"/pprof/cmdline": "Query the process' cmdline.",
"/pprof/growth": "Query the stack traces that caused growth in the
address space size.",
"/pprof/heap": "[seconds=<heap_profile_seconds>]. Query a sample of
live objects and the stack traces that allocated these objects (an environment
variable TCMALLOC_SAMPLE_PARAMETER should set to a positive value, such as
524288), or the current heap profiling information if 'seconds' parameter is
specified.",
"/pprof/profile": "[seconds=<cpu_profile_seconds>]. Query the CPU
profile. 'seconds' is 60 if not specified.",
"/pprof/symbol": "[symbol_address]. Query the process' symbols. Return
the symbol count of the process if using GET, return the symbol of the
'symbol_address' if using POST.",
"/recentStartTime": "Get the server start time.",
"/replica/data_version": "app_id=<app_id>. Query the data version of an
app.",
"/replica/duplication": "appid=<appid>. Query the duplication status of
an app.",
"/replica/manual_compaction": "app_id=<app_id>. Query the manual
compaction status of an app.",
"/updateConfig": "<key>=<new_value>. Update the config to the new
value.",
"/version": "Get the server version."
}
```
---
src/http/builtin_http_calls.cpp | 14 +++++++++-----
src/http/config_http_service.cpp | 6 ++++++
src/http/http_server.cpp | 11 +++++++++--
src/http/http_server.h | 16 ++++++++++++----
src/http/pprof_http_service.cpp | 10 +++++-----
src/http/pprof_http_service.h | 17 ++++++++++++-----
src/http/test/http_server_test.cpp | 9 +++++----
src/http/test/main.cpp | 4 ++--
src/meta/meta_http_service.cpp | 9 ++++++++-
src/meta/meta_http_service.h | 32 +++++++++++++++++++++-----------
src/replica/replica_http_service.h | 9 ++++++---
src/utils/flags.cpp | 2 +-
src/utils/metrics.cpp | 5 ++++-
13 files changed, 100 insertions(+), 44 deletions(-)
diff --git a/src/http/builtin_http_calls.cpp b/src/http/builtin_http_calls.cpp
index 6cdb87159..d42c95d68 100644
--- a/src/http/builtin_http_calls.cpp
+++ b/src/http/builtin_http_calls.cpp
@@ -80,27 +80,31 @@ namespace dsn {
register_http_call("")
.with_callback(
[](const http_request &req, http_response &resp) {
get_help_handler(req, resp); })
- .with_help("Lists all supported calls");
+ .with_help("List all supported calls.");
register_http_call("version")
.with_callback(
[](const http_request &req, http_response &resp) {
get_version_handler(req, resp); })
- .with_help("Gets the server version.");
+ .with_help("Get the server version.");
register_http_call("recentStartTime")
.with_callback([](const http_request &req, http_response &resp) {
get_recent_start_time_handler(req, resp);
})
- .with_help("Gets the server start time.");
+ .with_help("Get the server start time.");
register_http_call("config")
.with_callback([](const http_request &req, http_response &resp) {
get_config(req, resp); })
- .with_help("get the details of a specified config");
+ .with_help("name=<config_name>",
+ "Gets the details of a specified config. Only the configs "
+ "which are registered by DSN_DEFINE_xxx macro can be "
+ "queried.");
register_http_call("configs")
.with_callback(
[](const http_request &req, http_response &resp) {
list_all_configs(req, resp); })
- .with_help("list all configs");
+ .with_help("List all configs. Only the configs which are registered by
DSN_DEFINE_xxx "
+ "macro can be queried.");
}
} // namespace dsn
diff --git a/src/http/config_http_service.cpp b/src/http/config_http_service.cpp
index 68fd8be36..c13db7a5d 100644
--- a/src/http/config_http_service.cpp
+++ b/src/http/config_http_service.cpp
@@ -48,6 +48,12 @@ void get_config(const http_request &req, http_response &resp)
}
}
+ if (config_name.empty()) {
+ resp.status_code = http_status_code::kBadRequest;
+ resp.body = "name shouldn't be empty";
+ return;
+ }
+
auto res = get_flag_str(config_name);
if (res.is_ok()) {
resp.body = res.get_value();
diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp
index d8e38cf53..cbd214bba 100644
--- a/src/http/http_server.cpp
+++ b/src/http/http_server.cpp
@@ -89,6 +89,14 @@ std::string http_service::get_rel_path(const std::string
&sub_path) const
}
void http_service::register_handler(std::string sub_path, http_callback cb,
std::string help) const
+{
+ register_handler(std::move(sub_path), std::move(cb), "", std::move(help));
+}
+
+void http_service::register_handler(std::string sub_path,
+ http_callback cb,
+ std::string parameters,
+ std::string help) const
{
CHECK_FALSE(sub_path.empty());
if (!FLAGS_enable_http_server) {
@@ -97,8 +105,7 @@ void http_service::register_handler(std::string sub_path,
http_callback cb, std:
auto call = std::make_unique<http_call>();
call->path = get_rel_path(sub_path);
- call->callback = std::move(cb);
- call->help = std::move(help);
+ call->with_callback(std::move(cb)).with_help(parameters, help);
http_call_registry::instance().add(std::move(call));
}
diff --git a/src/http/http_server.h b/src/http/http_server.h
index 0f600ec4f..a9cf668b2 100644
--- a/src/http/http_server.h
+++ b/src/http/http_server.h
@@ -25,6 +25,7 @@
#include <unordered_map>
#include <utility>
+#include "fmt/core.h"
#include "http/http_method.h"
#include "http/http_status_code.h"
#include "runtime/task/task_code.h"
@@ -76,9 +77,10 @@ struct http_call
callback = std::move(cb);
return *this;
}
- http_call &with_help(std::string hp)
+ http_call &with_help(std::string hp) { return with_help("",
std::move(hp)); }
+ http_call &with_help(std::string parameters, std::string hp)
{
- help = std::move(hp);
+ help = fmt::format("{}{}{}", parameters, parameters.empty() ? "" : ".
", hp);
return *this;
}
};
@@ -96,6 +98,11 @@ public:
void register_handler(std::string sub_path, http_callback cb, std::string
help) const;
+ void register_handler(std::string sub_path,
+ http_callback cb,
+ std::string parameters,
+ std::string help) const;
+
private:
// If sub_path is 'app/duplication', the built path would be
'<root_path>/app/duplication',
// where path() would be called as root_path.
@@ -114,7 +121,8 @@ public:
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/updateConfig?<key>=<value>");
+ "<key>=<new_value>",
+ "Update the config to the new value.");
});
}
@@ -134,7 +142,7 @@ protected:
// this,
// std::placeholders::_1,
// std::placeholders::_2))
-// .with_help("Gets the app information")
+// .with_help("Gets the app information.")
// .add_argument("app_name", HTTP_ARG_STRING);
// ```
extern http_call ®ister_http_call(std::string full_path);
diff --git a/src/http/pprof_http_service.cpp b/src/http/pprof_http_service.cpp
index 8d9757fe9..d7246ae69 100644
--- a/src/http/pprof_http_service.cpp
+++ b/src/http/pprof_http_service.cpp
@@ -481,12 +481,12 @@ void pprof_http_service::growth_handler(const
http_request &req, http_response &
// //
// == ip:port/pprof/profile == //
// //
-static bool get_cpu_profile(std::string &result, useconds_t seconds)
+static bool get_cpu_profile(std::string &result, useconds_t micro_seconds)
{
const char *file_name = "cpu.prof";
ProfilerStart(file_name);
- usleep(seconds);
+ usleep(micro_seconds);
ProfilerStop();
std::ifstream in(file_name);
@@ -514,7 +514,7 @@ void pprof_http_service::profile_handler(const http_request
&req, http_response
return;
}
- useconds_t seconds = 60000000;
+ useconds_t micro_seconds = 60000000;
std::string req_url = req.full_url.to_string();
size_t len = req.full_url.length();
@@ -526,7 +526,7 @@ void pprof_http_service::profile_handler(const http_request
&req, http_response
std::string key(kv_sp.field(), kv_sp.length());
if (kv_sp != NULL && key == "seconds" && ++kv_sp != NULL) {
char *end_ptr;
- seconds = strtoul(kv_sp.field(), &end_ptr, 10) * 1000000;
+ micro_seconds = strtoul(kv_sp.field(), &end_ptr, 10) * 1000000;
break;
}
param_sp++;
@@ -535,7 +535,7 @@ void pprof_http_service::profile_handler(const http_request
&req, http_response
resp.status_code = http_status_code::kOk;
- get_cpu_profile(resp.body, seconds);
+ get_cpu_profile(resp.body, micro_seconds);
_in_pprof_action.store(false);
}
diff --git a/src/http/pprof_http_service.h b/src/http/pprof_http_service.h
index c10f3c719..56659d0bf 100644
--- a/src/http/pprof_http_service.h
+++ b/src/http/pprof_http_service.h
@@ -37,31 +37,38 @@ public:
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/pprof/heap");
+ "[seconds=<heap_profile_seconds>]",
+ "Query a sample of live objects and the stack traces
that allocated these "
+ "objects (an environment variable
TCMALLOC_SAMPLE_PARAMETER should set to "
+ "a positive value, such as 524288), or the current
heap profiling "
+ "information if 'seconds' parameter is specified.");
register_handler("symbol",
std::bind(&pprof_http_service::symbol_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/pprof/symbol");
+ "[symbol_address]",
+ "Query the process' symbols. Return the symbol count
of the process if "
+ "using GET, return the symbol of the 'symbol_address'
if using POST.");
register_handler("cmdline",
std::bind(&pprof_http_service::cmdline_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/pprof/cmdline");
+ "Query the process' cmdline.");
register_handler("growth",
std::bind(&pprof_http_service::growth_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/pprof/growth");
+ "Query the stack traces that caused growth in the
address space size.");
register_handler("profile",
std::bind(&pprof_http_service::profile_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/pprof/profile");
+ "[seconds=<cpu_profile_seconds>]",
+ "Query the CPU profile. 'seconds' is 60 if not
specified.");
}
std::string path() const override { return "pprof"; }
diff --git a/src/http/test/http_server_test.cpp
b/src/http/test/http_server_test.cpp
index 01d854b07..dc7f0bce7 100644
--- a/src/http/test/http_server_test.cpp
+++ b/src/http/test/http_server_test.cpp
@@ -99,22 +99,23 @@ TEST(bultin_http_calls_test, get_help)
register_http_call("")
.with_callback(
[](const http_request &req, http_response &resp) {
get_help_handler(req, resp); })
- .with_help("ip:port/");
+ .with_help("Empty test");
http_request req;
http_response resp;
get_help_handler(req, resp);
ASSERT_EQ(resp.status_code, http_status_code::kOk);
- ASSERT_EQ(resp.body, "{\"/\":\"ip:port/\"}\n");
+ ASSERT_EQ(resp.body, "{\"/\":\"Empty test\"}\n");
register_http_call("recentStartTime")
.with_callback([](const http_request &req, http_response &resp) {
get_recent_start_time_handler(req, resp);
})
- .with_help("ip:port/recentStartTime");
+ .with_help("Gets recentStartTime test");
get_help_handler(req, resp);
- ASSERT_EQ(resp.body,
"{\"/\":\"ip:port/\",\"/recentStartTime\":\"ip:port/recentStartTime\"}\n");
+ ASSERT_EQ(resp.body,
+ "{\"/\":\"Empty test\",\"/recentStartTime\":\"Gets
recentStartTime test\"}\n");
// Remove all http calls, especially `recentStartTime`.
for (const auto &call : http_call_registry::instance().list_all_calls()) {
diff --git a/src/http/test/main.cpp b/src/http/test/main.cpp
index bde70c9d0..7f950b99b 100644
--- a/src/http/test/main.cpp
+++ b/src/http/test/main.cpp
@@ -46,14 +46,14 @@ public:
dsn::http_method::GET,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/test/get");
+ "Test GET.");
register_handler("post",
std::bind(&test_http_service::method_handler,
this,
dsn::http_method::POST,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/test/post");
+ "Test POST.");
}
~test_http_service() = default;
diff --git a/src/meta/meta_http_service.cpp b/src/meta/meta_http_service.cpp
index ba5bc9b7e..4ae299bb8 100644
--- a/src/meta/meta_http_service.cpp
+++ b/src/meta/meta_http_service.cpp
@@ -90,6 +90,12 @@ void meta_http_service::get_app_handler(const http_request
&req, http_response &
if (!redirect_if_not_primary(req, resp))
return;
+ if (app_name.empty()) {
+ resp.status_code = http_status_code::kBadRequest;
+ resp.body = "app name shouldn't be empty";
+ return;
+ }
+
query_cfg_request request;
query_cfg_response response;
@@ -674,6 +680,7 @@ void meta_http_service::start_bulk_load_handler(const
http_request &req, http_re
resp.status_code = http_status_code::kBadRequest;
return;
}
+ // TODO(yingchun): Also deal the 'ingest_behind' parameter.
auto rpc_req = std::make_unique<start_bulk_load_request>(request);
start_bulk_load_rpc rpc(std::move(rpc_req), LPC_META_CALLBACK);
@@ -799,7 +806,7 @@ void meta_http_service::update_scenario_handler(const
http_request &req, http_re
return;
}
- // validate paramters
+ // validate parameters
usage_scenario_info info;
bool ret = json::json_forwarder<usage_scenario_info>::decode(req.body,
info);
if (!ret) {
diff --git a/src/meta/meta_http_service.h b/src/meta/meta_http_service.h
index 3c4d1c0a3..e603a4517 100644
--- a/src/meta/meta_http_service.h
+++ b/src/meta/meta_http_service.h
@@ -67,70 +67,80 @@ public:
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/app?app_name=temp");
+ "name=<app_name>[&detail]",
+ "Query app info.");
register_handler("app/duplication",
std::bind(&meta_http_service::query_duplication_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/app/duplication?name=<app_name>");
+ "name=<app_name>",
+ "Query app duplication info.");
register_handler("apps",
std::bind(&meta_http_service::list_app_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/apps");
+ "[detail]",
+ "List all apps in the cluster.");
register_handler("nodes",
std::bind(&meta_http_service::list_node_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/nodes");
+ "[detail]",
+ "Query the replica servers info.");
register_handler("cluster",
std::bind(&meta_http_service::get_cluster_info_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/cluster");
+ "Query the cluster info.");
register_handler("app_envs",
std::bind(&meta_http_service::get_app_envs_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/app_envs?name=temp");
+ "name=<app_name>",
+ "Query app environments.");
register_handler("backup_policy",
std::bind(&meta_http_service::query_backup_policy_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/backup_policy");
+ "name=<app_name1>&name=<app_name2>",
+ "Query backup policy by policy names.");
// request body should be start_bulk_load_request
register_handler("app/start_bulk_load",
std::bind(&meta_http_service::start_bulk_load_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/start_bulk_load");
+ "A JSON format of start_bulk_load_request structure",
+ "Start bulk load on an app.");
register_handler("app/query_bulk_load",
std::bind(&meta_http_service::query_bulk_load_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/query_bulk_load?name=temp");
+ "name=<app_name>",
+ "Query app bulk load info.");
// request body should be manual_compaction_info
register_handler("app/start_compaction",
std::bind(&meta_http_service::start_compaction_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/start_compaction");
+ "A JSON format of manual_compaction_info structure",
+ "Start compaction for an app.");
// request body should be usage_scenario_info
register_handler("app/usage_scenario",
std::bind(&meta_http_service::update_scenario_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/meta/app/usage_scenario");
+ "A JSON format of usage_scenario_info structure",
+ "Update usage scenario of an app.");
}
std::string path() const override { return "meta"; }
diff --git a/src/replica/replica_http_service.h
b/src/replica/replica_http_service.h
index 8fed6c6da..84f652d81 100644
--- a/src/replica/replica_http_service.h
+++ b/src/replica/replica_http_service.h
@@ -37,19 +37,22 @@ public:
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/replica/duplication?appid=<appid>");
+ "appid=<appid>",
+ "Query the duplication status of an app.");
register_handler("data_version",
std::bind(&replica_http_service::query_app_data_version_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/replica/data_version?app_id=<app_id>");
+ "app_id=<app_id>",
+ "Query the data version of an app.");
register_handler("manual_compaction",
std::bind(&replica_http_service::query_manual_compaction_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/replica/manual_compaction?app_id=<app_id>");
+ "app_id=<app_id>",
+ "Query the manual compaction status of an app.");
}
~replica_http_service()
diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp
index b49526280..0eaf13806 100644
--- a/src/utils/flags.cpp
+++ b/src/utils/flags.cpp
@@ -322,7 +322,7 @@ public:
{
const auto iter = _flags.find(name);
if (iter == _flags.end()) {
- return error_s::make(ERR_OBJECT_NOT_FOUND, fmt::format("{} is not
found", name));
+ return error_s::make(ERR_OBJECT_NOT_FOUND, fmt::format("'{}' is
not found", name));
}
return iter->second.to_json();
diff --git a/src/utils/metrics.cpp b/src/utils/metrics.cpp
index d7ca3b503..1677584be 100644
--- a/src/utils/metrics.cpp
+++ b/src/utils/metrics.cpp
@@ -285,7 +285,10 @@ metrics_http_service::metrics_http_service(metric_registry
*registry) : _registr
this,
std::placeholders::_1,
std::placeholders::_2),
- fmt::format("ip:port{}", kMetricsQueryPath));
+
"[with_metric_fields=field1,field2,...][&types=type1,type2,...][&ids=id1,id2,."
+
"..][&attributes=attr1,value1,attr2,value2,...][&metrics=metric1,metric2,...]["
+ "&detail=true|false]"
+ "Query the node metrics.");
}
namespace {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
