[PATCH v1 0/6] Allow for multiple prometheus endpoints

[Vlad Lazar]

Previously, seastar collected all metrics (and metrics related metadata) in a
single object (metrics::impl::impl). The consequence of this is that there was
no way to have multiple metrics namespaces and expose each namespace through its
own prometheus endpoint. If an application published a large volume of metrics,
the user had to filter them by using by using the name request parameter of the
prometheus endpoint. The downside of this approach is that the user must have
knowledge of the metrics set and, even in that case, some queries cannot be
expressed in terms of substring matching (i.e. the existing filtering
implementation).

A more flexible approach is to allow applications to categorise metrics at the
time of registering them and expose the different metrics set on different
prometheus endpoints if required. This is the approach taken for this patch set.
The metrics registration api now allows applications to specify the internal
metrics implementation (think of it as a metrics bucket) via an integer handle.
Internally, whenever a shard needs to handle a metric related operation for a
handle it hasn't seen before, a new metrics::impl::impl object is created on the
fly. This leads to a simple api, as applications don't need to pre-register
metric implementation before adding groups.

Here is an example of how to add metric groups to a specific "bucket" and expose
each "bucket" on a prometheus endpoint. Metrics in the foo group will be exposed
on the default metric prometheus endpoint, while the baz group will be exposed
on the 'metrics_extra' endpoint.

```
auto extra_metrics_handle = 100;

ss::metrics::metric_groups metrics;
metrics.add_group(
"foo", {sm::make_gauge("bar", [] { return 1; }, sm::description("foobar metric")});

ss::metrics::metric_groups metrics2(extra_metrics_handle);
metrics2.add_group(
"baz", {sm::make_gauge("daz", [] { return 1; }, sm::description("bazdaz metric")});

ss::prometheus::config default;
default.prefix = "default metrics bucket";

ss::prometheus::config extra;
extra.prefix = "extra metrics bucket";
extra.handle = extra_metrics_handle;
extra.route = "metrics_extra";

seastar::prometheus::add_prometheus_routes(your_server, default).get();
seastar::prometheus::add_prometheus_routes(your_server, extra).get();
```

Vlad Lazar (6):
metrics: allow multiple metrics::impl instances
metrics: expose metric impl handle to internal api
metrics: expose metric impl handle to external api
metrics: Use handle for impl object
prometheus: support multiple metric impls
scollectd: select internal metrics implementation

include/seastar/core/metrics_api.hh | 25 ++++---
include/seastar/core/metrics_registration.hh | 11 +--
include/seastar/core/prometheus.hh | 4 +-
include/seastar/core/scollectd.hh | 10 +--
src/core/io_queue.cc | 2 +-
src/core/metrics.cc | 73 ++++++++++++--------
src/core/prometheus.cc | 12 ++--
src/core/reactor.cc | 8 ++-
src/core/scollectd.cc | 12 ++--
9 files changed, 95 insertions(+), 62 deletions(-)

--
2.36.1

[Vlad Lazar]

Prior to this patch seastar only exposes one global metrics::impl::impl
object which holds all metric related data for one application.

This patch changes the implementation details such that multiple
metrics::impl::impl objects can exist for any given application.
Said objects are stored into a map on each shard and created
dinamically whenever requested. A metrics::impl::impl is identified
by an integer handle that acts as the key for the storage map.

Implementation note: in order to avoid issues caused by the ordering
of static thread_local objects I had to declare the storage in
reactor.cc.

(cherry picked from commit 585a8af6a586e161bbfc93a087d06c9c30175ca0)
---
include/seastar/core/metrics_api.hh | 7 ++++++-
src/core/metrics.cc | 18 +++++++++++++-----
src/core/reactor.cc | 6 ++++++
3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/include/seastar/core/metrics_api.hh b/include/seastar/core/metrics_api.hh
index 6843df47a..c1f4bf605 100644
--- a/include/seastar/core/metrics_api.hh
+++ b/include/seastar/core/metrics_api.hh
@@ -37,6 +37,9 @@ namespace metrics {
namespace impl {

using labels_type = std::map<sstring, sstring>;
+
+int default_handle();
+
}
}
}
@@ -179,6 +182,8 @@ class metric_groups_impl : public metric_groups_def {
};

class impl;
+using metric_implementations = std::unordered_map<int, ::seastar::shared_ptr<impl>>;
+metric_implementations& get_metric_implementations();

class registered_metric {
metric_info _info;
@@ -358,7 +363,7 @@ using values_reference = shared_ptr<values_copy>;

foreign_ptr<values_reference> get_values();

-shared_ptr<impl> get_local_impl();
+shared_ptr<impl> get_local_impl(int handle = default_handle());

void unregister_metric(const metric_id & id);

diff --git a/src/core/metrics.cc b/src/core/metrics.cc
index 98a14874a..9a21d6882 100644
--- a/src/core/metrics.cc
+++ b/src/core/metrics.cc
@@ -226,13 +226,17 @@ bool metric_id::operator==(
return as_tuple() == id2.as_tuple();
}

-// Unfortunately, metrics_impl can not be shared because it
-// need to be available before the first users (reactor) will call it
+shared_ptr<impl> get_local_impl(int handle) {
+ auto& impls = get_metric_implementations();
+ auto [it, inserted] = impls.try_emplace(handle);

-shared_ptr<impl> get_local_impl() {
- static thread_local auto the_impl = ::seastar::make_shared<impl>();
- return the_impl;
+ if (inserted) {
+ it->second = ::seastar::make_shared<impl>();
+ }
+
+ return it->second;
}
+
void impl::remove_registration(const metric_id& id) {
auto i = get_value_map().find(id.full_name());
if (i != get_value_map().end()) {
@@ -349,6 +353,10 @@ void impl::add_registration(const metric_id& id, const metric_type& type, metric
dirty();
}

+int default_handle() {
+ return 0;
+}
+
}

const bool metric_disabled = false;
diff --git a/src/core/reactor.cc b/src/core/reactor.cc
index 6f5683d75..733f1754b 100644
--- a/src/core/reactor.cc
+++ b/src/core/reactor.cc
@@ -3544,6 +3544,12 @@ smp_options::smp_options(program_options::option_group* parent_group)
{
}

+thread_local metrics::impl::metric_implementations metric_impls;
+
+metrics::impl::metric_implementations& metrics::impl::get_metric_implementations() {
+ return metric_impls;
+}
+
thread_local scollectd::impl scollectd_impl;

scollectd::impl & scollectd::get_impl() {
--
2.36.1

[Vlad Lazar]

This patch extends the metrics internal apis to use a specific
metrics::impl::impl object identified by its integer handle.

(cherry picked from commit 6ee4af7)
---
include/seastar/core/metrics_api.hh | 18 ++++++-----
include/seastar/core/metrics_registration.hh | 1 +
src/core/metrics.cc | 32 +++++++++++---------
3 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/include/seastar/core/metrics_api.hh b/include/seastar/core/metrics_api.hh
index c1f4bf605..b9dff10ed 100644
--- a/include/seastar/core/metrics_api.hh
+++ b/include/seastar/core/metrics_api.hh
@@ -170,9 +170,10 @@ struct metric_info {
using metrics_registration = std::vector<metric_id>;

class metric_groups_impl : public metric_groups_def {
+ int _handle;
metrics_registration _registration;
public:
- metric_groups_impl() = default;
+ explicit metric_groups_impl(int handle = default_handle());
~metric_groups_impl();
metric_groups_impl(const metric_groups_impl&) = delete;
metric_groups_impl(metric_groups_impl&&) = default;
@@ -190,7 +191,7 @@ class registered_metric {
metric_function _f;
shared_ptr<impl> _impl;
public:
- registered_metric(metric_id id, metric_function f, bool enabled=true);
+ registered_metric(metric_id id, metric_function f, bool enabled=true, int handle=default_handle());
virtual ~registered_metric() {}
virtual metric_value operator()() const {
return _f();
@@ -335,7 +336,7 @@ class impl {
return _value_map;
}

- void add_registration(const metric_id& id, const metric_type& type, metric_function f, const description& d, bool enabled);
+ void add_registration(const metric_id& id, const metric_type& type, metric_function f, const description& d, bool enabled, int handle = default_handle());
void remove_registration(const metric_id& id);
future<> stop() {
return make_ready_future<>();
@@ -358,14 +359,15 @@ class impl {
}
};

-const value_map& get_value_map();
+const value_map& get_value_map(int handle = default_handle());

using values_reference = shared_ptr<values_copy>;

-foreign_ptr<values_reference> get_values();
+foreign_ptr<values_reference> get_values(int handle = default_handle());

shared_ptr<impl> get_local_impl(int handle = default_handle());

-void unregister_metric(const metric_id & id);
+
+void unregister_metric(const metric_id & id, int handle = default_handle());

/*!
* \brief initialize metric group
@@ -373,7 +375,7 @@ void unregister_metric(const metric_id & id);
* Create a metric_group_def.
* No need to use it directly.
*/
-std::unique_ptr<metric_groups_def> create_metric_groups();
+std::unique_ptr<metric_groups_def> create_metric_groups(int handle = default_handle());

}

@@ -390,7 +392,7 @@ struct options : public program_options::option_group {
/*!
* \brief set the metrics configuration
*/
-future<> configure(const options& opts);
+future<> configure(const options& opts, int handle = default_handle());

}
}
diff --git a/include/seastar/core/metrics_registration.hh b/include/seastar/core/metrics_registration.hh
index 6f57b708b..e26949475 100644
--- a/include/seastar/core/metrics_registration.hh
+++ b/include/seastar/core/metrics_registration.hh
@@ -51,6 +51,7 @@ namespace seastar {
namespace metrics {

namespace impl {
+int default_handle();
class metric_groups_def;
struct metric_definition_impl;
class metric_groups_impl;
diff --git a/src/core/metrics.cc b/src/core/metrics.cc
index 9a21d6882..793b36b9d 100644
--- a/src/core/metrics.cc
+++ b/src/core/metrics.cc
@@ -118,8 +118,8 @@ bool label_instance::operator!=(const label_instance& id2) const {
label shard_label("shard");
namespace impl {

-registered_metric::registered_metric(metric_id id, metric_function f, bool enabled) :
- _f(f), _impl(get_local_impl()) {
+registered_metric::registered_metric(metric_id id, metric_function f, bool enabled, int handle) :
+ _f(f), _impl(get_local_impl(handle)) {
_info.enabled = enabled;
_info.id = id;
}
@@ -172,13 +172,15 @@ metric_definition_impl& metric_definition_impl::set_type(const sstring& type_nam
return *this;
}

-std::unique_ptr<metric_groups_def> create_metric_groups() {
- return std::make_unique<metric_groups_impl>();
+std::unique_ptr<metric_groups_def> create_metric_groups(int handle) {
+ return std::make_unique<metric_groups_impl>(handle);
}

+metric_groups_impl::metric_groups_impl(int handle) : _handle(handle) {}
+
metric_groups_impl::~metric_groups_impl() {
for (const auto& i : _registration) {
- unregister_metric(i);
+ unregister_metric(i, _handle);
}
}

@@ -186,7 +188,7 @@ metric_groups_impl& metric_groups_impl::add_metric(group_name_type name, const m

metric_id id(name, md._impl->name, md._impl->labels);

- get_local_impl()->add_registration(id, md._impl->type, md._impl->f, md._impl->d, md._impl->enabled);
+ get_local_impl(_handle)->add_registration(id, md._impl->type, md._impl->f, md._impl->d, md._impl->enabled, _handle);

_registration.push_back(id);
return *this;
@@ -252,20 +254,20 @@ void impl::remove_registration(const metric_id& id) {
}
}

-void unregister_metric(const metric_id & id) {
- get_local_impl()->remove_registration(id);
+void unregister_metric(const metric_id & id, int handle) {
+ get_local_impl(handle)->remove_registration(id);
}

-const value_map& get_value_map() {
- return get_local_impl()->get_value_map();
+const value_map& get_value_map(int handle) {
+ return get_local_impl(handle)->get_value_map();
}

-foreign_ptr<values_reference> get_values() {
+foreign_ptr<values_reference> get_values(int handle) {
shared_ptr<values_copy> res_ref = ::seastar::make_shared<values_copy>();
auto& res = *(res_ref.get());
auto& mv = res.values;
- res.metadata = get_local_impl()->metadata();
- auto & functions = get_local_impl()->functions();
+ res.metadata = get_local_impl(handle)->metadata();
+ auto & functions = get_local_impl(handle)->functions();
mv.reserve(functions.size());
for (auto&& i : functions) {
value_vector values;
@@ -331,8 +333,8 @@ std::vector<std::vector<metric_function>>& impl::functions() {
return _current_metrics;
}

-void impl::add_registration(const metric_id& id, const metric_type& type, metric_function f, const description& d, bool enabled) {
- auto rm = ::seastar::make_shared<registered_metric>(id, f, enabled);
+void impl::add_registration(const metric_id& id, const metric_type& type, metric_function f, const description& d, bool enabled, int handle) {
+ auto rm = ::seastar::make_shared<registered_metric>(id, f, enabled, handle);
sstring name = id.full_name();
if (_value_map.find(name) != _value_map.end()) {
auto& metric = _value_map[name];
--
2.36.1

[Vlad Lazar]

This patch extends the metrics user facing apis to use a specific

metrics::impl::impl object identified by its integer handle.

Note that the constructor of 'metric_groups' is marked explicit
in this patch and updates two call sites where the constructor was used
implicitly.
---
include/seastar/core/metrics_registration.hh | 10 ++++++----
src/core/io_queue.cc | 2 +-
src/core/metrics.cc | 20 +++++++++++---------
src/core/reactor.cc | 2 +-
4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/include/seastar/core/metrics_registration.hh b/include/seastar/core/metrics_registration.hh
index e26949475..78649058b 100644
--- a/include/seastar/core/metrics_registration.hh
+++ b/include/seastar/core/metrics_registration.hh
@@ -57,6 +57,8 @@ struct metric_definition_impl;
class metric_groups_impl;
}

+int default_handle();
+
using group_name_type = sstring; /*!< A group of logically related metrics */
class metric_groups;

@@ -90,7 +92,7 @@ class metric_group_definition {
class metric_groups {
std::unique_ptr<impl::metric_groups_def> _impl;
public:
- metric_groups() noexcept;
+ explicit metric_groups(int handle = default_handle()) noexcept;
metric_groups(metric_groups&&) = default;
virtual ~metric_groups();
metric_groups& operator=(metric_groups&&) = default;
@@ -99,7 +101,7 @@ class metric_groups {
*
* combine the constructor with the add_group functionality.
*/
- metric_groups(std::initializer_list<metric_group_definition> mg);
+ metric_groups(std::initializer_list<metric_group_definition> mg, int handle = default_handle());

/*!
* \brief Add metrics belonging to the same group.
@@ -155,7 +157,7 @@ class metric_groups {
*/
class metric_group : public metric_groups {
public:
- metric_group() noexcept;
+ explicit metric_group(int handle = default_handle()) noexcept;
metric_group(const metric_group&) = delete;
metric_group(metric_group&&) = default;
virtual ~metric_group();
@@ -166,7 +168,7 @@ class metric_group : public metric_groups {
*
*
*/
- metric_group(const group_name_type& name, std::initializer_list<metric_definition> l);
+ metric_group(const group_name_type& name, std::initializer_list<metric_definition> l, int handle = default_handle());
};


diff --git a/src/core/io_queue.cc b/src/core/io_queue.cc
index b1a97b478..b961208b9 100644
--- a/src/core/io_queue.cc
+++ b/src/core/io_queue.cc
@@ -650,7 +650,7 @@ void io_queue::register_stats(sstring name, priority_class_data& pc) {
}

new_metrics.add_group("io_queue", std::move(metrics));
- pc.metric_groups = std::exchange(new_metrics, {});
+ pc.metric_groups = std::exchange(new_metrics, sm::metric_groups{});
}

io_queue::priority_class_data& io_queue::find_or_create_class(const io_priority_class& pc) {
diff --git a/src/core/metrics.cc b/src/core/metrics.cc
index 793b36b9d..194ccf167 100644
--- a/src/core/metrics.cc
+++ b/src/core/metrics.cc
@@ -30,16 +30,20 @@
namespace seastar {
namespace metrics {

+int default_handle() {
+ return impl::default_handle();
+};
+
double_registration::double_registration(std::string what): std::runtime_error(what) {}

-metric_groups::metric_groups() noexcept : _impl(impl::create_metric_groups()) {
+metric_groups::metric_groups(int handle) noexcept : _impl(impl::create_metric_groups(handle)) {
}

void metric_groups::clear() {
_impl = impl::create_metric_groups();
}

-metric_groups::metric_groups(std::initializer_list<metric_group_definition> mg) : _impl(impl::create_metric_groups()) {
+metric_groups::metric_groups(std::initializer_list<metric_group_definition> mg, int handle) : _impl(impl::create_metric_groups(handle)) {
for (auto&& i : mg) {
add_group(i.name, i.metrics);
}
@@ -52,10 +56,9 @@ metric_groups& metric_groups::add_group(const group_name_type& name, const std::
_impl->add_group(name, l);
return *this;
}
-metric_group::metric_group() noexcept = default;
+metric_group::metric_group(int handle) noexcept : metric_groups(handle) {}
metric_group::~metric_group() = default;
-metric_group::metric_group(const group_name_type& name, std::initializer_list<metric_definition> l) {
- add_group(name, l);
+metric_group::metric_group(const group_name_type& name, std::initializer_list<metric_definition> l, int handle) : metric_groups({metric_group_definition(name, l)}, handle) {
}

metric_group_definition::metric_group_definition(const group_name_type& name, std::initializer_list<metric_definition> l) : name(name), metrics(l) {
@@ -101,15 +104,14 @@ options::options(program_options::option_group* parent_group)
{
}

-future<> configure(const options& opts) {
+future<> configure(const options& opts, int handle) {
impl::config c;
c.hostname = opts.metrics_hostname.get_value();
- return smp::invoke_on_all([c] {
- impl::get_local_impl()->set_config(c);
+ return smp::invoke_on_all([c, handle] {
+ impl::get_local_impl(handle)->set_config(c);
});
}

-

bool label_instance::operator!=(const label_instance& id2) const {

auto& id1 = *this;
return !(id1 == id2);
diff --git a/src/core/reactor.cc b/src/core/reactor.cc
index 733f1754b..04cb76f10 100644
--- a/src/core/reactor.cc
+++ b/src/core/reactor.cc
@@ -866,7 +866,7 @@ reactor::task_queue::register_stats() {
}, sm::description("Total amount in milliseconds we were in violation of the task quota"),
{group_label}),
});
- _metrics = std::exchange(new_metrics, {});
+ _metrics = std::exchange(new_metrics, sm::metric_groups{});
}

void
--
2.36.1

[Vlad Lazar]

This patch removes two subsequent calls to `get_local_impl` and reuses
the returned handle in that scope.
---
src/core/metrics.cc | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/core/metrics.cc b/src/core/metrics.cc
index 194ccf167..5bb224cf9 100644
--- a/src/core/metrics.cc
+++ b/src/core/metrics.cc
@@ -268,8 +268,11 @@ foreign_ptr<values_reference> get_values(int handle) {

shared_ptr<values_copy> res_ref = ::seastar::make_shared<values_copy>();
auto& res = *(res_ref.get());
auto& mv = res.values;

- res.metadata = get_local_impl(handle)->metadata();
- auto & functions = get_local_impl(handle)->functions();
+
+ auto impl = get_local_impl(handle);
+ res.metadata = impl->metadata();
+ auto & functions = impl->functions();
+

mv.reserve(functions.size());
for (auto&& i : functions) {
value_vector values;

--
2.36.1

[Vlad Lazar]

This patch extends the user facing prometheus apis allowing the user to
specify the internal metrics implementation to be used through a handle.
Additionally, 'add_prometheus_routes' now takes an argument that
specifies the route on which to advertise the metrics. This enables
different metrics "namespaces" to be served by different endpoints in
isolation.

(cherry picked from commit 6189522fd8247894fae1bd664a7305484cd22724)
---
include/seastar/core/prometheus.hh | 4 +++-
src/core/prometheus.cc | 12 ++++++------
2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/seastar/core/prometheus.hh b/include/seastar/core/prometheus.hh
index 187af568f..44d78f94c 100644
--- a/include/seastar/core/prometheus.hh
+++ b/include/seastar/core/prometheus.hh
@@ -37,11 +37,13 @@ struct config {
sstring hostname; //!< hostname is deprecated, use label instead
std::optional<metrics::label_instance> label; //!< A label that will be added to all metrics, we advice not to use it and set it on the prometheus server
sstring prefix = "seastar"; //!< a prefix that will be added to metric names
+ int handle = metrics::default_handle(); //!< Handle that specifies which metric implementation to query
+ sstring route = "/metrics"; //!< Name of the route on which to expose the metrics
};

future<> start(httpd::http_server_control& http_server, config ctx);

-/// \defgroup add_prometheus_routes adds a /metrics endpoint that returns prometheus metrics
+/// \defgroup add_prometheus_routes adds a specified endpoint (defaults to /metrics) that returns prometheus metrics
/// in txt format
/// @{
future<> add_prometheus_routes(distributed<http_server>& server, config ctx);
diff --git a/src/core/prometheus.cc b/src/core/prometheus.cc
index 937eec757..f2f6deb30 100644
--- a/src/core/prometheus.cc
+++ b/src/core/prometheus.cc
@@ -213,11 +213,11 @@ class metrics_families_per_shard {
/** @} */
};

-static future<> get_map_value(metrics_families_per_shard& vec) {
+static future<> get_map_value(metrics_families_per_shard& vec, int handle) {
vec.resize(smp::count);
- return parallel_for_each(boost::irange(0u, smp::count), [&vec] (auto cpu) {
- return smp::submit_to(cpu, [] {
- return mi::get_values();
+ return parallel_for_each(boost::irange(0u, smp::count), [handle, &vec] (auto cpu) {
+ return smp::submit_to(cpu, [handle] {
+ return mi::get_values(handle);
}).then([&vec, cpu] (auto res) {
vec[cpu] = std::move(res);
});
@@ -574,7 +574,7 @@ class metrics_handler : public handler_base {
rep->write_body("txt", [this, metric_family_name, prefix] (output_stream<char>&& s) {
return do_with(metrics_families_per_shard(), output_stream<char>(std::move(s)),
[this, prefix, &metric_family_name] (metrics_families_per_shard& families, output_stream<char>& s) mutable {
- return get_map_value(families).then([&s, &families, this, prefix, &metric_family_name]() mutable {
+ return get_map_value(families, _ctx.handle).then([&s, &families, this, prefix, &metric_family_name]() mutable {
return do_with(get_range(families, metric_family_name, prefix),
[&s, this](metric_family_range& m) {
return write_text_representation(s, _ctx, m);
@@ -591,7 +591,7 @@ class metrics_handler : public handler_base {


future<> add_prometheus_routes(http_server& server, config ctx) {
- server._routes.put(GET, "/metrics", new metrics_handler(ctx));
+ server._routes.put(GET, ctx.route, new metrics_handler(ctx));
return make_ready_future<>();
}

--
2.36.1

[Vlad Lazar]

This patch extends the scollectd apis with the ability to select the
internal metrics implementation to be used by providing a handle.

(cherry picked from commit d4331d148af54213890cf1d2fe05e8df05cdd504)
---
include/seastar/core/scollectd.hh | 10 +++++-----
src/core/scollectd.cc | 12 ++++++------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/seastar/core/scollectd.hh b/include/seastar/core/scollectd.hh
index 27c076690..9f677d273 100644
--- a/include/seastar/core/scollectd.hh
+++ b/include/seastar/core/scollectd.hh
@@ -371,7 +371,7 @@ struct options : public program_options::option_group {
/// \endcond
};

-void configure(const options&);
+void configure(const options&, int handle = seastar::metrics::default_handle());
void remove_polled_metric(const type_instance_id &);

class plugin_instance_metrics;
@@ -390,8 +390,8 @@ class plugin_instance_metrics;
*/
struct registration {
registration() = default;
- registration(const type_instance_id& id);
- registration(type_instance_id&& id);
+ registration(const type_instance_id& id, int handle = seastar::metrics::default_handle());
+ registration(type_instance_id&& id, int handle = seastar::metrics::default_handle());
registration(const registration&) = delete;
registration(registration&&) = default;
~registration();
@@ -798,8 +798,8 @@ seastar::metrics::impl::metric_id to_metrics_id(const type_instance_id & id);
*/
template<typename Arg>
[[deprecated("Use the metrics layer")]] static type_instance_id add_polled_metric(const type_instance_id & id, description d,
- Arg&& arg, bool enabled = true) {
- seastar::metrics::impl::get_local_impl()->add_registration(to_metrics_id(id), arg.type, seastar::metrics::impl::make_function(arg.value, arg.type), d, enabled);
+ Arg&& arg, bool enabled = true, int handle = seastar::metrics::default_handle()) {
+ seastar::metrics::impl::get_local_impl(handle)->add_registration(to_metrics_id(id), arg.type, seastar::metrics::impl::make_function(arg.value, arg.type), d, enabled);
return id;
}
/*!
diff --git a/src/core/scollectd.cc b/src/core/scollectd.cc
index b06785a07..c0227833d 100644
--- a/src/core/scollectd.cc
+++ b/src/core/scollectd.cc
@@ -76,12 +76,12 @@ registration::~registration() {
unregister();
}

-registration::registration(const type_instance_id& id)
-: _id(id), _impl(seastar::metrics::impl::get_local_impl()) {
+registration::registration(const type_instance_id& id, int handle)
+: _id(id), _impl(seastar::metrics::impl::get_local_impl(handle)) {
}

-registration::registration(type_instance_id&& id)
-: _id(std::move(id)), _impl(seastar::metrics::impl::get_local_impl()) {
+registration::registration(type_instance_id&& id, int handle)
+: _id(std::move(id)), _impl(seastar::metrics::impl::get_local_impl(handle)) {
}

seastar::metrics::impl::metric_id to_metrics_id(const type_instance_id & id) {
@@ -524,7 +524,7 @@ future<> send_metric(const type_instance_id & id,
return get_impl().send_metric(id, values);
}

-void configure(const options& opts) {
+void configure(const options& opts, int handle) {
bool enable = opts.collectd.get_value();
if (!enable) {
return;
@@ -533,7 +533,7 @@ void configure(const options& opts) {
auto period = std::chrono::milliseconds(opts.collectd_poll_period.get_value());

auto host = (opts.collectd_hostname.get_value() == "")
- ? seastar::metrics::impl::get_local_impl()->get_config().hostname
+ ? seastar::metrics::impl::get_local_impl(handle)->get_config().hostname
: sstring(opts.collectd_hostname.get_value());

// Now create send loops on each cpu
--
2.36.1

[Vlad Lazar]

^ Gentle ping :)

[Avi Kivity]

Amnon, I think he meant you.

--
You received this message because you are subscribed to the Google Groups "seastar-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to seastar-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/2e6d962c-22e6-4818-b67e-ea46c20eeb2en%40googlegroups.com.

[Amnon Heiman]

Now, I think so too.

I'll take a look

[Vlad Lazar]

Gentle reminder to take a look when you get a chance. If the motivation behind the patchset is not clear, then I can elaborate. Just let me know.

[Amnon Heiman]

I'm not sure I understand why doing anything specific with scollectd, why not using the metrics layer?

To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/5baee83b-5b8a-4ba0-b9f3-61bb1d4a0af5n%40googlegroups.com.

[Vlad Lazar]

That's just the last commit in the set. It was used as the subject because I replied to it I guess.

[Amnon Heiman]

Way back in 2018 I had the series: [PATCH seastar v7 0/3] Support multiple Prometheus server

have you seen that discussion?

[Vlad Lazar]

No, I hadn't seen it. Thanks for pointing it out. This patch seems pretty similar to the one in 2018.

A few differences that I could spot at first glance:

* The 2018 patch set did the selection of the "container" at the metric level (by extending the API of make_gauge and friends).

In my patches this is done at the "metric_groups" level.

* The 2018 patch set uses a string as a handle for the container instead of an integer.

Otherwise, they look pretty similar. Do you maybe remember why that didn't get merged in? I didn't see any blockers raised in the comments.

--
You received this message because you are subscribed to a topic in the Google Groups "seastar-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/seastar-dev/xtZnIU14u5c/unsubscribe.
To unsubscribe from this group and all its topics, send an email to seastar-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/abfc9666-684b-4a79-9fb9-b99e301cd319n%40googlegroups.com.

[Amnon Heiman]

There were 7 different implementations in that discussion, one of them used enum which is similiar to an int, but more meaningfull.

making the decision on the family level is not a huge limitation.

I think that adding a label is the best approach and add the ability to the already exising prometheus filtering capability

to filter by that label,

This is also be in sync with an effort I wanted to do that is data-dog related, to mark to external system, what is more important.

Back then, Avi didn't like that, but maybe he'll be more persuable today, if we'll pick this approach we can make this a super simple change

(we can even start with adding such a label independent to the metrics level implementation).

Note that the following the prometheus metrics reduction effort that I'm doing, the total number of metrics will be reduce drastically.

[Vlad Lazar]

If I understand this correctly, you're suggesting to add a label to each metric that should be published on a different Prometheus endpoint

and then use that label to filter the metrics depending on which endpoint is queried. I don't see why that wouldn't work. I find it a bit weird

to dispatch based on a label, but maybe I just need to get used to the idea.

You received this message because you are subscribed to the Google Groups "seastar-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to seastar-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/28b3b931-06b2-47bf-a3bf-f923c595ce31n%40googlegroups.com.

[Amnon Heiman]

I suggest to have a single endpoing but uses query parameters to get different values from that endpoint.

We already do that with name

so lets say the labels will be "level"

http://localhost:9180/metrics?level=verbose

would return something else then

http://localhost:9180/metrics?level=info

In general, I think we shouldn't have more than one prometheus API but make that one API more flexible.