This is an automated email from the ASF dual-hosted git repository.
gehafearless 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 5ea60899a feat(new_metrics): build the http query string based on
metric filters (#1870)
5ea60899a is described below
commit 5ea60899aec815ff0384d005b2c556cae72d905a
Author: Dan Wang <[email protected]>
AuthorDate: Thu Jan 25 13:56:06 2024 +0800
feat(new_metrics): build the http query string based on metric filters
(#1870)
Pegasus shell needs to fetch metrics by http client. However, building query
string is not simple. You have to write each field name yourself, and
assemble
all field names/values into a valid query string.
To simplify this operation, we could allow metric filters to support
exporting
the query string. Users could just set conditions they need to metric
filters,
then get the query string just by a function conveniently.
---
src/http/http_server.cpp | 26 ++++++++---
src/http/http_server.h | 8 +++-
src/utils/CMakeLists.txt | 5 ++-
src/utils/metrics.cpp | 36 ++++++++++++++-
src/utils/metrics.h | 11 ++++-
src/utils/strings.cpp | 11 +++++
src/utils/strings.h | 4 ++
src/utils/test/metrics_test.cpp | 97 +++++++++++++++++++++++++++++++++++++++++
src/utils/test/utils.cpp | 38 +++++++++++++++-
9 files changed, 221 insertions(+), 15 deletions(-)
diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp
index aa628dac9..94de34bb7 100644
--- a/src/http/http_server.cpp
+++ b/src/http/http_server.cpp
@@ -57,6 +57,16 @@ error_s update_config(const http_request &req)
return update_flag(iter->first, iter->second);
}
+// If sub_path is 'app/duplication', the built path would be
'<root_path>/app/duplication'.
+std::string build_rel_path(const std::string &root_path, const std::string
&sub_path)
+{
+ std::string rel_path(root_path);
+ if (!rel_path.empty()) {
+ rel_path += '/';
+ }
+ return rel_path += sub_path;
+}
+
} // anonymous namespace
/*extern*/ http_call ®ister_http_call(std::string full_path)
@@ -73,18 +83,20 @@ error_s update_config(const http_request &req)
http_call_registry::instance().remove(full_path);
}
-void http_service::register_handler(std::string sub_path, http_callback cb,
std::string help)
+std::string http_service::get_rel_path(const std::string &sub_path) const
{
- CHECK(!sub_path.empty(), "");
+ return build_rel_path(path(), sub_path);
+}
+
+void http_service::register_handler(std::string sub_path, http_callback cb,
std::string help) const
+{
+ CHECK_FALSE(sub_path.empty());
if (!FLAGS_enable_http_server) {
return;
}
+
auto call = std::make_unique<http_call>();
- call->path = this->path();
- if (!call->path.empty()) {
- call->path += '/';
- }
- call->path += sub_path;
+ call->path = get_rel_path(sub_path);
call->callback = std::move(cb);
call->help = std::move(help);
http_call_registry::instance().add(std::move(call));
diff --git a/src/http/http_server.h b/src/http/http_server.h
index f6cb1628d..71fd8bde2 100644
--- a/src/http/http_server.h
+++ b/src/http/http_server.h
@@ -89,11 +89,17 @@ struct http_call
class http_service
{
public:
+ http_service() noexcept = default;
virtual ~http_service() = default;
virtual std::string path() const = 0;
- void register_handler(std::string sub_path, http_callback cb, std::string
help);
+ void register_handler(std::string sub_path, http_callback cb, 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.
+ std::string get_rel_path(const std::string &sub_path) const;
};
class http_server_base : public http_service
diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt
index 1c1c7d0c4..337ac2591 100644
--- a/src/utils/CMakeLists.txt
+++ b/src/utils/CMakeLists.txt
@@ -36,14 +36,15 @@ set(MY_PROJ_LIBS
rocksdb
lz4
zstd
- snappy)
+ snappy
+ absl::strings)
# Extra files that will be installed
set(MY_BINPLACES "")
if (APPLE)
dsn_add_static_library()
- target_link_libraries(${MY_PROJ_NAME} PRIVATE dsn_http
dsn_replication_common)
+ target_link_libraries(${MY_PROJ_NAME} PRIVATE dsn_http
dsn_replication_common absl::strings)
else()
dsn_add_shared_library()
target_link_libraries(${MY_PROJ_NAME} PRIVATE dsn_replication_common)
diff --git a/src/utils/metrics.cpp b/src/utils/metrics.cpp
index 2cd8e70d0..681b6555c 100644
--- a/src/utils/metrics.cpp
+++ b/src/utils/metrics.cpp
@@ -18,6 +18,7 @@
#include "utils/metrics.h"
#include <absl/strings/string_view.h>
+#include <boost/algorithm/string/join.hpp>
#include <boost/asio/basic_deadline_timer.hpp>
#include <boost/date_time/posix_time/posix_time_duration.hpp>
#include <boost/system/error_code.hpp>
@@ -230,6 +231,32 @@ void metric_filters::extract_entity_metrics(const
metric_entity::metric_map &can
}
}
+std::string metric_filters::to_query_string() const
+{
+#define COMBINE_FIELD_PAIR(name, container)
\
+ do {
\
+ if (container.empty()) {
\
+ break;
\
+ }
\
+
\
+ std::string pair(#name);
\
+ pair += '=';
\
+ pair += boost::join(container, ",");
\
+ fields.push_back(std::move(pair));
\
+ } while (0)
+
+ std::vector<std::string> fields;
+ COMBINE_FIELD_PAIR(with_metric_fields, with_metric_fields);
+ COMBINE_FIELD_PAIR(types, entity_types);
+ COMBINE_FIELD_PAIR(ids, entity_ids);
+ COMBINE_FIELD_PAIR(attributes, entity_attrs);
+ COMBINE_FIELD_PAIR(metrics, entity_metrics);
+
+#undef COMBINE_FIELD_PAIR
+
+ return boost::join(fields, "&");
+}
+
metric_entity_ptr metric_entity_prototype::instantiate(const std::string &id,
const
metric_entity::attr_map &attrs) const
{
@@ -245,14 +272,19 @@ metric_entity_prototype::metric_entity_prototype(const
char *name) : _name(name)
metric_entity_prototype::~metric_entity_prototype() {}
+const std::string metrics_http_service::kMetricsRootPath("");
+const std::string metrics_http_service::kMetricsQuerySubPath("metrics");
+const std::string
+ metrics_http_service::kMetricsQueryPath('/' +
metrics_http_service::kMetricsQuerySubPath);
+
metrics_http_service::metrics_http_service(metric_registry *registry) :
_registry(registry)
{
- register_handler("metrics",
+ register_handler(kMetricsQuerySubPath,
std::bind(&metrics_http_service::get_metrics_handler,
this,
std::placeholders::_1,
std::placeholders::_2),
- "ip:port/metrics");
+ fmt::format("ip:port{}", kMetricsQueryPath));
}
namespace {
diff --git a/src/utils/metrics.h b/src/utils/metrics.h
index abd7d5ef2..47ab3a720 100644
--- a/src/utils/metrics.h
+++ b/src/utils/metrics.h
@@ -509,6 +509,11 @@ struct metric_filters
void extract_entity_metrics(const metric_entity::metric_map &candidates,
metric_entity::metric_map &target_metrics)
const;
+ // Build the http query string based on metric filters. This is useful
when an http request
+ // is performed for metrics query: firstly, set metric filters with what
you want; then,
+ // get query string by this function conveniently and put it into the http
request.
+ std::string to_query_string() const;
+
// `with_metric_fields` includes all the metric fields that are wanted by
client. If it
// is empty, there will be no restriction: in other words, all fields
owned by the metric
// will be put in the response.
@@ -563,12 +568,16 @@ class metric_registry; // IWYU pragma: keep
class metrics_http_service : public http_server_base
{
public:
+ static const std::string kMetricsRootPath;
+ static const std::string kMetricsQuerySubPath;
+ static const std::string kMetricsQueryPath;
+
explicit metrics_http_service(metric_registry *registry);
~metrics_http_service() = default;
// There is only one API now whose URI is "/metrics", thus just make
// this URI as sub path while leaving the root path empty.
- std::string path() const override { return ""; }
+ std::string path() const override { return kMetricsRootPath; }
private:
friend void test_get_metrics_handler(const http_request &req,
http_response &resp);
diff --git a/src/utils/strings.cpp b/src/utils/strings.cpp
index 32e810ebd..c7c48a9c3 100644
--- a/src/utils/strings.cpp
+++ b/src/utils/strings.cpp
@@ -24,6 +24,7 @@
* THE SOFTWARE.
*/
+#include <absl/strings/ascii.h>
#include <openssl/md5.h>
#include <stdio.h>
#include <strings.h>
@@ -424,5 +425,15 @@ std::string find_string_prefix(const std::string &input,
char separator)
}
return input.substr(0, current);
}
+
+bool has_space(const std::string &str)
+{
+ // Use absl::ascii_isspace() instead of std::isspace(), which could not be
used as
+ // the predicate directly, since it might be implemented as a macro, and
its parameter
+ // must be declared as unsigned. Thus, to use std::isspace(), we have to
wrap it into
+ // a lambda expression.
+ return std::any_of(str.begin(), str.end(), absl::ascii_isspace);
+}
+
} // namespace utils
} // namespace dsn
diff --git a/src/utils/strings.h b/src/utils/strings.h
index 1759856f0..85d4a75a9 100644
--- a/src/utils/strings.h
+++ b/src/utils/strings.h
@@ -121,5 +121,9 @@ std::string string_md5(const char *buffer, unsigned int
length);
// splits the "input" string by the only character "separator" to get the
string prefix.
// if there is no prefix or the first character is "separator", it will return
"".
std::string find_string_prefix(const std::string &input, char separator);
+
+// Decide if there are some space characters in the given string, such as ' ',
'\r', '\n' or '\t'.
+bool has_space(const std::string &str);
+
} // namespace utils
} // namespace dsn
diff --git a/src/utils/test/metrics_test.cpp b/src/utils/test/metrics_test.cpp
index d1d33bf1c..cbacc6a7b 100644
--- a/src/utils/test/metrics_test.cpp
+++ b/src/utils/test/metrics_test.cpp
@@ -2887,6 +2887,103 @@ TEST(metrics_test, http_get_metrics)
}
}
+struct metric_filters_query_string_case
+{
+ metric_filters::metric_fields_type with_metric_fields;
+ metric_filters::entity_types_type entity_types;
+ metric_filters::entity_ids_type entity_ids;
+ metric_filters::entity_attrs_type entity_attrs;
+ metric_filters::entity_metrics_type entity_metrics;
+ size_t expected_fields;
+};
+
+class MetricFiltersQueryStringTest : public
testing::TestWithParam<metric_filters_query_string_case>
+{
+};
+
+const std::vector<metric_filters_query_string_case>
metric_filters_query_string_tests = {
+ // Empty query string.
+ {{}, {}, {}, {}, {}, 0},
+ // Some fields were missing in the query string.
+ {{"name", "value"}, {"replica"}, {}, {}, {"rdb_total_sst_files",
"rdb_total_sst_size_mb"}, 3},
+ // All fields were present.
+ {{"name", "value"},
+ {"replica"},
+ {"replica5.2"},
+ {"table_id", "partition_id"},
+ {"rdb_total_sst_files", "rdb_total_sst_size_mb"},
+ 5},
+};
+
+TEST_P(MetricFiltersQueryStringTest, BuildQueryString)
+{
+ const auto &query_string_case = GetParam();
+
+ metric_filters filters;
+
+#define COPY_CONTAINER(field) filters.field = query_string_case.field
+
+ // Copy the data of the case to the tested metric filters.
+ COPY_CONTAINER(with_metric_fields);
+ COPY_CONTAINER(entity_types);
+ COPY_CONTAINER(entity_ids);
+ COPY_CONTAINER(entity_attrs);
+ COPY_CONTAINER(entity_metrics);
+
+#undef COPY_CONTAINER
+
+ // Build the http query string based on the tested metric filters.
+ const auto &query_string = filters.to_query_string();
+ std::cout << "query string: " << query_string << std::endl;
+
+ // There should not be any space character in the query string.
+ ASSERT_FALSE(utils::has_space(query_string));
+
+ // Extract each field name/value pair from the query string and check the
number of
+ // the fields.
+ std::vector<std::string> fields;
+ utils::split_args(query_string.c_str(), fields, '&');
+ ASSERT_EQ(query_string_case.expected_fields, fields.size());
+
+ size_t i = 0;
+
+#define CHECK_FIELD(name, type, field)
\
+ do {
\
+ if (query_string_case.field.empty()) {
\
+ break;
\
+ }
\
+
\
+ ASSERT_LT(i, query_string_case.expected_fields);
\
+
\
+ std::vector<std::string> kvs;
\
+ utils::split_args(fields[i].c_str(), kvs, '=');
\
+ ASSERT_EQ(2, kvs.size());
\
+ ASSERT_STREQ(#name, kvs[0].c_str());
\
+
\
+ type actual_field;
\
+ utils::split_args(kvs[1].c_str(), actual_field, ',');
\
+ ASSERT_EQ(query_string_case.field, actual_field);
\
+ ++i;
\
+ } while (0)
+
+ // Check each field name/value extracted from the query string is exactly
equal to the
+ // original metric filters.
+ CHECK_FIELD(with_metric_fields, metric_filters::metric_fields_type,
with_metric_fields);
+ CHECK_FIELD(types, metric_filters::entity_types_type, entity_types);
+ CHECK_FIELD(ids, metric_filters::entity_ids_type, entity_ids);
+ CHECK_FIELD(attributes, metric_filters::entity_attrs_type, entity_attrs);
+ CHECK_FIELD(metrics, metric_filters::entity_metrics_type, entity_metrics);
+
+#undef CHECK_FIELD
+
+ // All of the fields should have been checked.
+ ASSERT_EQ(query_string_case.expected_fields, i);
+}
+
+INSTANTIATE_TEST_SUITE_P(MetricsTest,
+ MetricFiltersQueryStringTest,
+ testing::ValuesIn(metric_filters_query_string_tests));
+
using surviving_metrics_case = std::tuple<std::string, bool, bool, bool, bool>;
class MetricsRetirementTest : public
testing::TestWithParam<surviving_metrics_case>
diff --git a/src/utils/test/utils.cpp b/src/utils/test/utils.cpp
index 311b04c51..cd5708750 100644
--- a/src/utils/test/utils.cpp
+++ b/src/utils/test/utils.cpp
@@ -45,8 +45,8 @@
#include "utils/strings.h"
#include "utils/utils.h"
-using namespace ::dsn;
-using namespace ::dsn::utils;
+namespace dsn {
+namespace utils {
TEST(core, get_last_component)
{
@@ -568,3 +568,37 @@ TEST(core, get_intersection)
ASSERT_EQ(intersection.size(), 1);
ASSERT_EQ(*intersection.begin(), 3);
}
+
+struct has_space_case
+{
+ std::string str;
+ bool expected_has_space;
+};
+
+class HasSpaceTest : public testing::TestWithParam<has_space_case>
+{
+};
+
+TEST_P(HasSpaceTest, HasSpace)
+{
+ const auto &space_case = GetParam();
+ EXPECT_EQ(space_case.expected_has_space, has_space(space_case.str));
+}
+
+const std::vector<has_space_case> has_space_tests = {
+ {"abc xyz", true},
+ {" abcxyz", true},
+ {"abcxyz ", true},
+ {"abc xyz", true},
+ {"abc\r\nxyz", true},
+ {"abc\r\nxyz", true},
+ {"abc\txyz", true},
+ {"abc\txyz", true},
+ {"abcxyz", false},
+ {"abc_xyz", false},
+};
+
+INSTANTIATE_TEST_SUITE_P(StringTest, HasSpaceTest,
testing::ValuesIn(has_space_tests));
+
+} // namespace utils
+} // namespace dsn
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
