From 0ecded6f43559b3e1bc02fa1524a77d3ad27450e Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 17:41:48 +0800 Subject: [PATCH 1/7] apps/metrics_tester: fix typo in conf-example.yaml as we fallback to creating a counter mtric, this does not change the behavior of the test. Signed-off-by: Kefu Chai --- apps/metrics_tester/conf-example.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/metrics_tester/conf-example.yaml b/apps/metrics_tester/conf-example.yaml index 8dc44342d83..0de628ec303 100644 --- a/apps/metrics_tester/conf-example.yaml +++ b/apps/metrics_tester/conf-example.yaml @@ -5,6 +5,6 @@ metrics: - name: gag1 type: gauge values: [5] -- name: caunt1 - type: caunter +- name: count1 + type: counter values: [7] From e9d8ef43c67ec42176fbd4100015a87c4c9507ef Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 18:19:25 +0800 Subject: [PATCH 2/7] apps/metrics_tester: always start exporter the whole purpose of this tester is to serve as a prometheus exporter. so let's always start it. Signed-off-by: Kefu Chai --- apps/metrics_tester/metrics_tester.cc | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/apps/metrics_tester/metrics_tester.cc b/apps/metrics_tester/metrics_tester.cc index cdbef296f44..4a34d2faccd 100644 --- a/apps/metrics_tester/metrics_tester.cc +++ b/apps/metrics_tester/metrics_tester.cc @@ -136,17 +136,15 @@ int main(int ac, char** av) { }); }).get(); - if (port) { - prometheus_server.start("prometheus").get(); + prometheus_server.start("prometheus").get(); - prometheus::config pctx; - pctx.allow_protobuf = true; - prometheus::start(prometheus_server, pctx).get(); - prometheus_server.listen(socket_address{listen, port}).handle_exception([] (auto ep) { - return make_exception_future<>(ep); - }).get(); - stop_signal.wait().get(); - } + prometheus::config pctx; + pctx.allow_protobuf = true; + prometheus::start(prometheus_server, pctx).get(); + prometheus_server.listen(socket_address{listen, port}).handle_exception([] (auto ep) { + return make_exception_future<>(ep); + }).get(); + stop_signal.wait().get(); }); }); } From ceb9d5284f74c7f22371bd5c4b31d7ad221cef48 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 18:22:29 +0800 Subject: [PATCH 3/7] apps/metrics_tester: stop server properly otherwise the sharded_server would abort when the server shuts down ``` metrics_tester: /home/kefu/dev/seastar/include/seastar/core/sharded.hh:565: seastar::sharded::~sharded() [Service = seastar::ht tpd::http_server]: Assertion `_instances.empty()' failed. Aborting. ``` Signed-off-by: Kefu Chai --- apps/metrics_tester/metrics_tester.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/metrics_tester/metrics_tester.cc b/apps/metrics_tester/metrics_tester.cc index 4a34d2faccd..ec4f0cb6521 100644 --- a/apps/metrics_tester/metrics_tester.cc +++ b/apps/metrics_tester/metrics_tester.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include "../lib/stop_signal.hh" #include using namespace seastar; @@ -137,6 +138,9 @@ int main(int ac, char** av) { }).get(); prometheus_server.start("prometheus").get(); + auto stop_server = defer([&] () noexcept { + prometheus_server.stop().get(); + }); prometheus::config pctx; pctx.allow_protobuf = true; From 3088c3836c2fa22339b6f85435aa4b198b846c3b Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 18:26:24 +0800 Subject: [PATCH 4/7] apps/metrics_tester: support "labels" in conf.yaml instead of hardwiring to {"private": "1"}, let's support "labels" in conf.yaml, so that we can add multiple metrics with different labels in the configuration file. Signed-off-by: Kefu Chai --- apps/metrics_tester/conf-example.yaml | 6 ++++++ apps/metrics_tester/metrics_tester.cc | 20 ++++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/apps/metrics_tester/conf-example.yaml b/apps/metrics_tester/conf-example.yaml index 0de628ec303..c4e47746535 100644 --- a/apps/metrics_tester/conf-example.yaml +++ b/apps/metrics_tester/conf-example.yaml @@ -2,9 +2,15 @@ metrics: - name: hist1 type: histogram values: [1000,2000,3000] + labels: + private: "1" - name: gag1 type: gauge values: [5] + labels: + private: "1" - name: count1 type: counter + labels: + private: "1" values: [7] diff --git a/apps/metrics_tester/metrics_tester.cc b/apps/metrics_tester/metrics_tester.cc index ec4f0cb6521..dc305e9ba40 100644 --- a/apps/metrics_tester/metrics_tester.cc +++ b/apps/metrics_tester/metrics_tester.cc @@ -27,6 +27,8 @@ #include #include #include "../lib/stop_signal.hh" +#include +#include #include using namespace seastar; using namespace std::chrono_literals; @@ -37,6 +39,7 @@ struct metric_def { sstring name; sstring type; std::vector values; + std::vector labels; }; struct config { @@ -55,6 +58,12 @@ struct convert { if (node["values"]) { cfg.values = node["values"].as>(); } + if (node["labels"]) { + const auto labels = node["labels"].as>(); + for (auto& [key, value]: labels) { + cfg.labels.emplace_back(key, value); + } + } return true; } }; @@ -76,21 +85,21 @@ std::function make_histogram_fun(const return [histogram]() {return histogram;}; } -sm::impl::metric_definition_impl make_metrics_definition(const metric_def& jc, sm::label_instance private_label) { +sm::impl::metric_definition_impl make_metrics_definition(const metric_def& jc) { if (jc.type == "histogram") { sm::internal::time_estimated_histogram histogram; for (const auto& v : jc.values) { histogram.add_micro(v); } return sm::make_histogram(jc.name, [histogram]() {return histogram.to_metrics_histogram();}, - sm::description(jc.name),{private_label} ); + sm::description(jc.name), jc.labels ); } if (jc.type == "gauge") { return sm::make_gauge(jc.name, [val=jc.values[0]] { return val; }, - sm::description(jc.name),{private_label} ); + sm::description(jc.name), jc.labels ); } return sm::make_counter(jc.name, [val=jc.values[0]] { return val; }, - sm::description(jc.name),{private_label} ); + sm::description(jc.name), jc.labels ); } int main(int ac, char** av) { @@ -111,7 +120,6 @@ int main(int ac, char** av) { seastar_apps_lib::stop_signal stop_signal; auto& opts = app.configuration(); auto& listen = opts["listen"].as(); - auto private_label = sm::label_instance("private", "1"); auto& port = opts["port"].as(); auto& conf = opts["conf"].as(); @@ -120,7 +128,7 @@ int main(int ac, char** av) { for (auto&& jc : cfg.metrics) { _metrics.add_group("test_group", { - make_metrics_definition(jc, private_label) + make_metrics_definition(jc) }); } smp::invoke_on_all([] { From 04edaaec2984b560dd8c99644910a122254c594d Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 19:22:10 +0800 Subject: [PATCH 5/7] apps/metrics_tester: keep metrics with "private" labels before this change, only the metrics with labels of {"private": "1"} are preserved. but if we want to test the metrics aggregation by name, would be more convenient if we can preserve metrics with the same name but with different labels. so, in this change, we relax the criteria so that all metrics with non-empty "private" labels are preserved. Signed-off-by: Kefu Chai --- apps/metrics_tester/metrics_tester.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/metrics_tester/metrics_tester.cc b/apps/metrics_tester/metrics_tester.cc index dc305e9ba40..84fef695c94 100644 --- a/apps/metrics_tester/metrics_tester.cc +++ b/apps/metrics_tester/metrics_tester.cc @@ -138,7 +138,7 @@ int main(int ac, char** av) { rl[0].action = metrics::relabel_config::relabel_action::drop; rl[1].source_labels = {"private"}; - rl[1].expr = "1"; + rl[1].expr = ".*"; rl[1].action = metrics::relabel_config::relabel_action::keep; return metrics::set_relabel_configs(rl).then([](metrics::metric_relabeling_result) { return; From 00a448e1368fe6df3652e0cb0962be12f1bbaa52 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 19:47:29 +0800 Subject: [PATCH 6/7] tests: move apps/metrics_tester to tests/unit so that we can reuse metrics_tester for unit test -- we will add tests exercising the query parameters supported by the exporter httpd server, like `__name__` and `__aggregate__`. Signed-off-by: Kefu Chai --- apps/CMakeLists.txt | 1 - apps/metrics_tester/CMakeLists.txt | 27 ------------------- tests/unit/CMakeLists.txt | 7 +++++ .../unit}/conf-example.yaml | 0 .../unit}/metrics_tester.cc | 2 +- .../unit}/test_metrics.py | 0 6 files changed, 8 insertions(+), 29 deletions(-) delete mode 100644 apps/metrics_tester/CMakeLists.txt rename {apps/metrics_tester => tests/unit}/conf-example.yaml (100%) rename {apps/metrics_tester => tests/unit}/metrics_tester.cc (99%) rename {apps/metrics_tester => tests/unit}/test_metrics.py (100%) diff --git a/apps/CMakeLists.txt b/apps/CMakeLists.txt index 333d72d9c4e..a4f34812cea 100644 --- a/apps/CMakeLists.txt +++ b/apps/CMakeLists.txt @@ -51,7 +51,6 @@ endmacro () add_subdirectory (httpd) add_subdirectory (io_tester) add_subdirectory (rpc_tester) -add_subdirectory(metrics_tester) add_subdirectory (iotune) add_subdirectory (memcached) add_subdirectory (seawreck) diff --git a/apps/metrics_tester/CMakeLists.txt b/apps/metrics_tester/CMakeLists.txt deleted file mode 100644 index 4883e083e1e..00000000000 --- a/apps/metrics_tester/CMakeLists.txt +++ /dev/null @@ -1,27 +0,0 @@ -# -# This file is open source software, licensed to you under the terms -# of the Apache License, Version 2.0 (the "License"). See the NOTICE file -# distributed with this work for additional information regarding copyright -# ownership. You may not use this file except in compliance with the License. -# -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# - -# -# Copyright (C) 2024 Scylladb, Ltd. -# - -seastar_add_app (metrics_tester - SOURCES metrics_tester.cc) - -target_link_libraries (app_metrics_tester - PRIVATE yaml-cpp::yaml-cpp) diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index e74df8d8ce9..b85d701eba6 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -757,3 +757,10 @@ add_test ( set_tests_properties (Seastar.unit.json2code PROPERTIES TIMEOUT ${Seastar_TEST_TIMEOUT}) + +add_executable (metrics_tester + metrics_tester.cc) +target_link_libraries (metrics_tester + PRIVATE + seastar_private + yaml-cpp::yaml-cpp) diff --git a/apps/metrics_tester/conf-example.yaml b/tests/unit/conf-example.yaml similarity index 100% rename from apps/metrics_tester/conf-example.yaml rename to tests/unit/conf-example.yaml diff --git a/apps/metrics_tester/metrics_tester.cc b/tests/unit/metrics_tester.cc similarity index 99% rename from apps/metrics_tester/metrics_tester.cc rename to tests/unit/metrics_tester.cc index 84fef695c94..b1a7ed87ac7 100644 --- a/apps/metrics_tester/metrics_tester.cc +++ b/tests/unit/metrics_tester.cc @@ -26,7 +26,7 @@ #include #include #include -#include "../lib/stop_signal.hh" +#include "../../apps/lib/stop_signal.hh" #include #include #include diff --git a/apps/metrics_tester/test_metrics.py b/tests/unit/test_metrics.py similarity index 100% rename from apps/metrics_tester/test_metrics.py rename to tests/unit/test_metrics.py From 606e022b4e297274126a262470586ff95f87a581 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 12 May 2024 20:44:40 +0800 Subject: [PATCH 7/7] tests: add test for prometheus exporter Fixes #2233 Signed-off-by: Kefu Chai --- tests/unit/CMakeLists.txt | 15 +++ tests/unit/conf-example.yaml | 14 ++ tests/unit/metrics_tester.cc | 31 +++++ tests/unit/prometheus_test.py | 234 ++++++++++++++++++++++++++++++++++ 4 files changed, 294 insertions(+) create mode 100755 tests/unit/prometheus_test.py diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index b85d701eba6..8184b861fa1 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -764,3 +764,18 @@ target_link_libraries (metrics_tester PRIVATE seastar_private yaml-cpp::yaml-cpp) + +add_dependencies (unit_tests metrics_tester) +add_custom_target (test_unit_prometheus_run + COMMAND ${CMAKE_COMMAND} -E env ${Seastar_TEST_ENVIRONMENT} + ${CMAKE_CURRENT_SOURCE_DIR}/prometheus_test.py + --exporter $ + --config ${CMAKE_CURRENT_SOURCE_DIR}/conf-example.yaml + USES_TERMINAL) +add_dependencies (test_unit_prometheus_run metrics_tester) +add_test ( + NAME Seastar.unit.prometheus + COMMAND ${CMAKE_COMMAND} --build ${Seastar_BINARY_DIR} --target test_unit_prometheus_run) +set_tests_properties (Seastar.unit.prometheus + PROPERTIES + TIMEOUT ${Seastar_TEST_TIMEOUT}) diff --git a/tests/unit/conf-example.yaml b/tests/unit/conf-example.yaml index c4e47746535..c37f0977a2a 100644 --- a/tests/unit/conf-example.yaml +++ b/tests/unit/conf-example.yaml @@ -14,3 +14,17 @@ metrics: labels: private: "1" values: [7] +- name: counter_1 + type: counter + labels: + private: "2" + values: [1] +- name: counter_1 + type: counter + labels: + private: "3" + values: [2] +metric_family_config: +- name: test_group_counter_1 + aggregate_labels: + - private diff --git a/tests/unit/metrics_tester.cc b/tests/unit/metrics_tester.cc index b1a7ed87ac7..5c932ff4395 100644 --- a/tests/unit/metrics_tester.cc +++ b/tests/unit/metrics_tester.cc @@ -44,7 +44,9 @@ struct metric_def { struct config { std::vector metrics; + std::vector metric_family_config; }; + namespace YAML { template<> struct convert { @@ -67,15 +69,36 @@ struct convert { return true; } }; + +template<> +struct convert { + static bool decode(const Node& node, sm::metric_family_config& cfg) { + if (node["name"]) { + cfg.name = node["name"].as(); + } + if (node["regex_name"]) { + cfg.regex_name = node["regex_name"].as(); + } + if (node["aggregate_labels"]) { + cfg.aggregate_labels = node["aggregate_labels"].as>(); + } + return true; + } +}; + template<> struct convert { static bool decode(const Node& node, config& cfg) { if (node["metrics"]) { cfg.metrics = node["metrics"].as>(); } + if (node["metric_family_config"]) { + cfg.metric_family_config = node["metric_family_config"].as>(); + } return true; } }; + } std::function make_histogram_fun(const metric_def& c) { sm::internal::time_estimated_histogram histogram; @@ -145,6 +168,10 @@ int main(int ac, char** av) { }); }).get(); + if (!cfg.metric_family_config.empty()) { + sm::set_metric_family_configs(cfg.metric_family_config); + } + prometheus_server.start("prometheus").get(); auto stop_server = defer([&] () noexcept { prometheus_server.stop().get(); @@ -156,6 +183,10 @@ int main(int ac, char** av) { prometheus_server.listen(socket_address{listen, port}).handle_exception([] (auto ep) { return make_exception_future<>(ep); }).get(); + + fmt::print("{}\n", port); + fflush(stdout); + stop_signal.wait().get(); }); }); diff --git a/tests/unit/prometheus_test.py b/tests/unit/prometheus_test.py new file mode 100755 index 00000000000..00953f2002f --- /dev/null +++ b/tests/unit/prometheus_test.py @@ -0,0 +1,234 @@ +#!/usr/bin/env python3 +# +# This file is open source software, licensed to you under the terms +# of the Apache License, Version 2.0 (the "License"). See the NOTICE file +# distributed with this work for additional information regarding copyright +# ownership. You may not use this file except in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# +# Copyright (C) 2024 Scylladb, Ltd. +# + +import argparse +import re +import subprocess +import sys +import unittest +import urllib.request +import urllib.parse + +from typing import Optional +from collections import namedtuple + + +class Exposition: + def __init__(self, + name: str, + type_: str, + value: str, + labels: Optional[dict[str, str]] = None) -> None: + self.name = name + if type_ == 'counter': + self.value = float(value) + elif type_ == 'gauge': + self.value = float(value) + else: + # we don't verify histogram or summary yet + self.value = None + self.labels = labels + + +class Metrics: + prefix = 'seastar' + group = 'test_group' + # parse lines like: + # rest_api_scheduler_queue_length{group="main",shard="0"} 0.000000 + # where: + # - "rest_api" is the prometheus prefix + # - "scheduler" is the metric group name + # - "queue_length" is the name of the metric + # - the kv pairs in "{}" are labels" + # - "0.000000" is the value of the metric + # this format is compatible with + # https://github.com/prometheus/docs/blob/main/content/docs/instrumenting/exposition_formats.md + # NOTE: scylla does not include timestamp in the exported metrics + pattern = re.compile(r'''(?P\w+) # rest_api_scheduler_queue_length + \{(?P[^\}]*)\} # {group="main",shard="0"} + \s+ # + (?P[^\s]+) # 0.000000''', re.X) + + def __init__(self, lines: list[str]) -> None: + self.lines: list[str] = lines + + @classmethod + def full_name(cls, name: str) -> str: + '''return the full name of a metrics + ''' + return f'{cls.group}_{name}' + + @staticmethod + def _parse_labels(s: str) -> dict[str, str]: + return dict(name_value.split('=', 1) for name_value in s.split(',')) + + def get(self, + name: Optional[str] = None, + labels: Optional[dict[str, str]] = None) -> list[Exposition]: + '''Return all expositions matching the given name and labels + ''' + full_name = None + if name is not None: + full_name = f'{self.prefix}_{self.group}_{name}' + results: list[Exposition] = [] + metric_type = None + + for line in self.lines: + if not line: + continue + if line.startswith('# HELP'): + continue + if line.startswith('# TYPE'): + _, _, metric_name, type_ = line.split() + if full_name is None or metric_name == full_name: + metric_type = type_ + continue + matched = self.pattern.match(line) + assert matched, f'malformed metric line: {line}' + + metric_name = matched.group('metric_name') + if full_name and metric_name != full_name: + continue + + metric_labels = self._parse_labels(matched.group('labels')) + if labels is not None and metric_labels != labels: + continue + + metric_value = matched.group('value') + results.append(Exposition(metric_name, + metric_type, + metric_value, + metric_labels)) + return results + + def get_help(self, name: str) -> Optional[str]: + full_name = f'{self.prefix}_{self.group}_{name}' + header = f'# HELP {full_name}' + for line in self.lines: + if line.startswith(header): + tokens = line.split(maxsplit=3) + return tokens[-1] + return None + + +class TestPrometheus(unittest.TestCase): + exporter_path = None + exporter_process = None + exporter_config = None + port = 10001 + + @classmethod + def setUpClass(cls) -> None: + args = [cls.exporter_path, + '--port', f'{cls.port}', + '--conf', cls.exporter_config, + '--smp=2'] + cls.exporter_process = subprocess.Popen(args, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + bufsize=0, text=True) + # wait until the server is ready for serve + cls.exporter_process.stdout.readline() + + @classmethod + def tearDownClass(cls) -> None: + cls.exporter_process.terminate() + + @classmethod + def _get_metrics(cls, + name: Optional[str] = None, + labels: Optional[dict[str, str]] = None, + with_help: bool = True, + aggregate: bool = True) -> Metrics: + query: dict[str, str] = {} + if name is not None: + query['__name__'] = name + if labels is not None: + query.update(labels) + if not with_help: + query['__help__'] = 'false' + if not aggregate: + query['__aggregate__'] = 'false' + params = urllib.parse.urlencode(query) + host = 'localhost' + url = f'http://{host}:{cls.port}/metrics?{params}' + with urllib.request.urlopen(url) as f: + body = f.read().decode('utf-8') + return Metrics(body.rstrip().split('\n')) + + def test_filtering_by_label(self) -> None: + TestCase = namedtuple('TestCase', ['label', 'regex', 'found']) + label = 'private' + tests = [ + TestCase(label=label, regex='dne', found=0), + TestCase(label=label, regex='404', found=0), + TestCase(label=label, regex='2', found=1), + # aggregated + TestCase(label=label, regex='2|3', found=1), + ] + for test in tests: + with self.subTest(regex=test.regex, found=test.found): + metrics = self._get_metrics(labels={test.label: test.regex}) + self.assertEqual(len(metrics.get()), test.found) + + def test_aggregated(self) -> None: + name = 'counter_1' + # see also rest_api_httpd.cc::aggregate_by_name + TestCase = namedtuple('TestCase', ['aggregate', 'expected_values']) + tests = [ + TestCase(aggregate=False, expected_values=[1, 2]), + TestCase(aggregate=True, expected_values=[3]) + ] + for test in tests: + with self.subTest(aggregate=test.aggregate, + values=test.expected_values): + metrics = self._get_metrics(Metrics.full_name(name), aggregate=test.aggregate) + expositions = metrics.get(name) + actual_values = sorted(e.value for e in expositions) + self.assertEqual(actual_values, test.expected_values) + + def test_help(self) -> None: + name = 'counter_1' + tests = [True, False] + for with_help in tests: + with self.subTest(with_help=with_help): + metrics = self._get_metrics(Metrics.full_name(name), with_help=with_help) + msg = metrics.get_help(name) + if with_help: + self.assertIsNotNone(msg) + else: + self.assertIsNone(msg) + + +if __name__ == '__main__': + parser = argparse.ArgumentParser() + parser.add_argument('--exporter', + required=True, + help='Path to the exporter executable') + parser.add_argument('--config', + required=True, + help='Path to the metrics definition file') + opts, remaining = parser.parse_known_args() + remaining.insert(0, sys.argv[0]) + TestPrometheus.exporter_path = opts.exporter + TestPrometheus.exporter_config = opts.config + unittest.main(argv=remaining)