Minimal primary metrics endpoint #5165

VladLazar · 2022-06-20T17:43:30Z

Cover letter

Currently, Redpanda exposes its full set of metrics in Prometheus format on the /metrics endpoint.
A large number of metrics are exposed on this endpoint (e.g. 6275 for a broker with no user topics
added), many of which are only useful to someone with context regarding the inner working of
Redpanda.

To address the, issues with the /metrics endpoint, this set of patches adds a new endpoint
lower in cardinality which is meant for external and internal users that don't require such a
high level of granularity.

A list of the metrics that have been added to the new endpoint (exposed on /public_metrics)
can be found below.

Partition Level Metrics:

redpanda_kafka_max_offset:
- Description: latest committed offset for the partition (i.e. the
  offset of the last message safely persisted on most replicas)
- Labels: namespace, topic, partition, shard
- Aggregation: shard
redpanda_kafka_under_replicated_replicas:
- Description: the number of under replicated replicas for the
  partition
- Labels: namespace, topic, partition, shard
- Aggregation: shard

Topic Level Metrics:

redpanda_kafka_request_bytes_total:
- Description: total number of bytes consumed/produced per topic
- Labels: namespace, topic, partition,
  request ("produce" | "consume")
- Aggregation: shard, partition
redpanda_kafka_replicas:
- Description: number of configured replicas per topic
- Labels: namespace, topic, partition
- Aggregation: shard, partition

Broker Level Metrics:

redpanda_kafka_request_latency_seconds:
- Description: latency of produce/consume requests per broker
  (measures from the moment a request is initiated on the partition to
  the moment when the response is fulfilled)
- Labels: shard
- Aggregation: shard

For a few desirable metrics, the decision was made to obtain them
by aggregating in PromQL in order to reduce the amount of work performed on scrapes:

redpanda_partitions_underreplicated - cluster level
redpanda_kafka_write_bytes - broker level
redpanda_kafka_read_bytes - broker level
redpanda_topic_partitions - topic level

force push:

use sm::label for specifying the aggregation labels (required due to change in seastar Add metrics aggregation capabilities seastar#26)

force push:

update the metric creation calls to match the changes in seastar (PR)
optimise metric reporting by using for_each_leader and by introducing utility function
rename the new metrics endpoint to /public_metrics
abstract common metric utilities in the ssx/metrics.h header
tweak commit messages

force-push

Git rebase with intermediate clang-format

force push:

Elaborate metric descriptions

force push:

Made the disable_metrics config option disable only the /metrics endpoint metrics
Added a new config option, disable_public_metrics to disable registering metrics on
the /public_metrics endpoint.
Made pandaproxy publish its metrics under the rest_proxy name for the public_metrics
endpoint. Nothing changes for the metrics endpoint.
Nested the metrics utils in ssx in a namespace: ssx::metrics.
Added test for cluster level metrics

force push:

Rebase to fix conflicts

force push:

Reformat python with yapf

force push:

Disable public metrics for redpanda fixtures used in cluster tests.

force push:

Changed the added tests to check the metrics delta instead of reported values. Also addressed a few nits.

force push:

Rebase on dev

force push:

Fixed test failure caused by throwing on offset translation failure.

force push:

squashed stray commit

Release notes

Features

Redpanda now has a new metrics endpoint, /public_metrics, that exposes a small set of
well curated metrics. See the documentation for the full list of published metrics. The disable_public_metrics
configuration option disables registering metrics for the /public_metrics endpoint. The older disable_metrics
configuration option skips registration of metrics for the /metrics endpoint.

src/v/redpanda/admin_server.cc

src/v/pandaproxy/probe.cc

BenPope · 2022-06-22T12:20:25Z

src/v/pandaproxy/server.cc

@@ -137,7 +139,7 @@ void server::route(server::route_t r) {
    // NOTE: this pointer will be owned by data member _routes of
    // ss::httpd:server. seastar didn't use any unique ptr to express that.
    auto* handler = new handler_adaptor(
-      _pending_reqs, _ctx, std::move(r.handler), r.path_desc);
+      _pending_reqs, _ctx, std::move(r.handler), r.path_desc, _server_name);


For the rest proxy we get "pandaproxy", It might be preferable to make it "rest_proxy". I'm not sure of the implication of renaming the server - I think it also affects a field in the rpc metrics, which may or may not be desired.

By changing the server name, the older metrics endpoint would refer to pandaproxy under both names (pandaproxy and rest_proxy). This is because the server object metrics use the server name as a label. I think that would be confusing.

I feel like I'm missing some context here in order to make a decision. In general, do we want to rename pandaproxy to rest_proxy?

Yes, that's the case. Once upon a time Pandaproxy was it's own thing, and needed branding, but now it's a part of Redpanda proper, it's better to have it be named what it does.

But _server_name is piped through to the old metrics, which we shouldn't change, so I think we'll need a new name for the new metrics, whilst keeping the old name for the old metrics.

src/v/cluster/partition_probe.cc

src/v/kafka/latency_probe.h

src/v/cluster/controller_probe.cc

src/v/redpanda/admin_server.cc

src/v/pandaproxy/probe.cc

jcsp · 2022-06-22T19:56:12Z

For a few desirable metrics, the decision was made to obtain them
by aggregating in PromQL in order to reduce the amount of work performed on scrapes:

I didn't quite parse that: does it mean that e.g. redpanda_kafka_write_bytes is not rolled up to a per-node value (if not, what are its labels?)?

BenPope · 2022-06-22T20:12:32Z

For a few desirable metrics, the decision was made to obtain them
by aggregating in PromQL in order to reduce the amount of work performed on scrapes:

I didn't quite parse that: does it mean that e.g. redpanda_kafka_write_bytes is not rolled up to a per-node value (if not, what are its labels?)?

Let's take redpanda_kafka_request_bytes_total which has labels:

request (either produce or consume)
namespace
topic
partition
shard

And internally aggregates:

partition
shard

The Prometheus scrape will add something like instance_id for the broker.

Sometimes the data doesn't exist at the broker level

To get topic bytes written, the query should be
sum without(instance_id)(redpanda_kafka_request_bytes_total{request="produce"})

Sometimes there's no point in adding a broker level metric that increases cardinality

To get the throughput published to the broker:
sum without(namespace,topic,partition)(rate(redpanda_kafka_request_bytes_total{request="produce"}))

emaxerrno · 2022-06-22T21:20:14Z

@VladLazar can we change the endpoint to /user-metrics or /public-metrics or /user-facing-metrics ... doing a /metrics2 gives no meaning to how the endpoint is to be used by end users.

emaxerrno · 2022-06-22T21:22:22Z

@VladLazar can you give guidance as to what is the end-result of reduction size/cardinality/etc what is the measurable improvement. how to think about this for cloud for example - i.e.: how many bytes does this save. or how much is cardinality reduced. some high level guidance is helpful.

src/v/pandaproxy/probe.cc

VladLazar · 2022-06-23T16:15:16Z

@VladLazar can you give guidance as to what is the end-result of reduction size/cardinality/etc what is the measurable improvement. how to think about this for cloud for example - i.e.: how many bytes does this save. or how much is cardinality reduced. some high level guidance is helpful.

Sure, the metric cardinality reduction happens in a separate PR. This PR just adds the new metric endpoint. There's a bit of detail in the aggregation PR cover letter, but in brief I've seen cardinality be reduced by several orders of magnitude (somewhere between 4 and 6 times). Most of the aggregations that were introduced are done by shard, which means that the magnitude of the reduction increases with the number of shards and decreases for the other dimensions (e.g. topics, partitions).

BenPope · 2022-06-27T21:25:48Z

Can you add some release notes, and move the force push comments above them.

src/v/redpanda/admin_server.cc

BenPope · 2022-06-27T22:01:08Z

I wonder if it's worth having another configuration parameter, we have enable_metrics which covers both endpoints, I think it's worth having enable_public_metrics (or whatever we call it), so that the original metrics can be disabled independently of these new ones.

VladLazar · 2022-06-28T09:55:53Z

I wonder if it's worth having another configuration parameter, we have enable_metrics which covers both endpoints, I think it's worth having enable_public_metrics (or whatever we call it), so that the original metrics can be disabled independently of these new ones.

The only metrics related config we have is disable_metrics, which disables all metrics on /public_metrics and most metrics (minus the seastar ones) on /metrics. We can add something along the lines of disable_default_metrics (naming is hard) which would only disable the metrics on /metrics. In what scenario would we use it though? We can't set it to true while the current dashboards are still in use. If we reach a point when they're not useful any more we might as well remove the old enpoint altogether.

BenPope · 2022-06-28T10:02:59Z

The only metrics related config we have is disable_metrics, which disables all metrics on /public_metrics and most metrics (minus the seastar ones) on /metrics. We can add something along the lines of disable_default_metrics (naming is hard) which would only disable the metrics on /metrics. In what scenario would we use it though? We can't set it to true while the current dashboards are still in use. If we reach a point when they're not useful any more we might as well remove the old enpoint altogether.

Oh yeah, it's the other way around, disable vs enable. Under the assumption that disable_metrics is in use somewhere, it feel fitting that we may want to have similar functionality for the new endpoint, perhaps, disable_public_metrics, such that disable_metrics only disables the internal/existing metrics.

We can leave it to another PR if it's an open question.

VladLazar · 2022-06-28T10:17:00Z

The only metrics related config we have is disable_metrics, which disables all metrics on /public_metrics and most metrics (minus the seastar ones) on /metrics. We can add something along the lines of disable_default_metrics (naming is hard) which would only disable the metrics on /metrics. In what scenario would we use it though? We can't set it to true while the current dashboards are still in use. If we reach a point when they're not useful any more we might as well remove the old enpoint altogether.

Oh yeah, it's the other way around, disable vs enable. Under the assumption that disable_metrics is in use somewhere, it feel fitting that we may want to have similar functionality for the new endpoint, perhaps, disable_public_metrics, such that disable_metrics only disables the internal/existing metrics.

We can leave it to another PR if it's an open question.

For me the question is whether disable_metrics should disable metrics on both endpoints (that's what the name implies to me).

BenPope · 2022-06-28T13:04:46Z

Can you do a pass of the descriptions, so that they make sense as documentation. E.g.: redpanda_schema_registry_request_latency_seconds just says "Request latency" (which is entirely my fault).

src/v/ssx/metrics.h

BenPope · 2022-06-30T10:57:47Z

src/v/pandaproxy/server.cc

@@ -137,7 +139,7 @@ void server::route(server::route_t r) {
    // NOTE: this pointer will be owned by data member _routes of
    // ss::httpd:server. seastar didn't use any unique ptr to express that.
    auto* handler = new handler_adaptor(
-      _pending_reqs, _ctx, std::move(r.handler), r.path_desc);
+      _pending_reqs, _ctx, std::move(r.handler), r.path_desc, _server_name);


Yes, that's the case. Once upon a time Pandaproxy was it's own thing, and needed branding, but now it's a part of Redpanda proper, it's better to have it be named what it does.

But _server_name is piped through to the old metrics, which we shouldn't change, so I think we'll need a new name for the new metrics, whilst keeping the old name for the old metrics.

src/v/cluster/partition_probe.cc

src/v/kafka/latency_probe.h

src/v/cluster/controller_probe.cc

VladLazar · 2022-06-30T14:33:00Z

force push:

Made the disable_metrics config option disable only the /metrics endpoint metrics
Added a new config option, disable_public_metrics to disable registering metrics on
the /public_metrics endpoint.
Made pandaproxy publish its metrics under the rest_proxy name for the public_metrics
endpoint. Nothing changes for the metrics endpoint. Nested the metrics utils in ssx in a
namespace: ssx::metrics.
Added test for cluster level metrics
Addressed all other pending comments

jcsp · 2022-07-01T15:29:01Z

No outstanding comments from me! Delighted to have this landing as our new home for higher level metrics + I think the code for only outputting cluster stats from the leader is neat. I think others had more recent comments than I did, so will leave it for them to ✔️ it formally.

BenPope

Looking great

src/v/cluster/partition_probe.cc

src/v/kafka/latency_probe.h

src/v/cluster/controller_probe.cc

tests/rptest/tests/cluster_metrics_test.py

src/v/cluster/controller_probe.cc

BenPope

LGTM, assuming tests pass

Signed-off-by: Ben Pope <ben@redpanda.com>

This patch adds a new 'metrics.h' header that exposes some utilites for interacting with the seastar metrics subsystem: * public_metrics_handle: the seastar internal implementation handle for metrics exposed on the '/public_metrics' endpoint * report_default_histogram: converts an HDR histogram to a seastar histogram with pre-configured defaults

Expose another prometheus endpoint on route '/public_metrics'. This endpoint should include a limited set of metrics, aggregated by default that's useful for external and internal users who lack context into the innerworkings of the project. Signed-off-by: Ben Pope <ben@redpanda.com>

This patch adds a configuration option that allows users to disable the public metric endpoints. It also updates the description of both 'disable_metrics' and 'disable_public_metric' to make it clear which metrics endpoint will be affected.

This patch disables the public metrics for the redpanda fixture used in the cluster unit tests. These simulated nodes use the same internal metrics implementation which means that normal metric registration will cause 'double_registration' errors to be thrown unless metrics are disabled.

This patch adds request latency metrics for pandaproxy and the schema registry. * repdanda_rest_proxy_request_latency_seconds * redpanda_schema_registry_request_latency_seconds * Description: latency of requests processed by the respective server which includes the time spent waiting for resources to become available * Labels: shard, operation * Aggregation: shard, operation This patch updates the server constructor to accept the name of the metrics group under which to publish metrics for the new endpoint. The server name, which is already an argument to the constructor, cannot be used for this because we wish to publish 'pandaproxy' metrics under the 'rest_proxy' name.

This patch adds partition and topic level endpoint to the new prometheus endpoint (exposed at "/public_metrics"). The following metrics were added: Partition Level Metrics: * redpanda_kafka_max_offset: * Description: latest committed offset for the partition (i.e. the offset of the last message safely persisted on most replicas) * Labels: namespace, topic, partition, shard * Aggregation: shard * redpanda_kafka_under_replicated_replicas: * Description: the number of under replicated replicas for the partition * Labels: namespace, topic, partition, shard * Aggregation: shard Topic Level Metrics: * redpanda_kafka_request_bytes_total: * Description: total number of bytes consumed/produced per topic * Labels: namespace, topic, partition, request ("produce" | "consume") * Aggregation: shard, partition * redpanda_kafka_replicas: * Description: number of configured replicas per topic * Labels: namespace, topic, partition * Aggregation: shard, partition Broker Level Metrics: * redpanda_kafka_request_latency_seconds: * Description: latency of produce/consume requests per broker (measures from the moment a request is initiated on the partition to the moment when the response is fulfilled) * Labels: shard * Aggregation: shard

This patch introduces a new probe that which is embedded in the controller object. Only one instance is created per broker and it matches the lifetime of the controller object. These metrics are reported only from the controller leader (one node per cluster). The following cluster level metrics were added: * redpanda_cluster_brokers: * Description: number of configured brokers in the cluster * Label: shard * Aggreation: shard * redpanda_cluster_topics: * Description: number of user created topics in the cluster * Label: shard * Aggreation: shard * redpanda_cluster_partitions: * Description: number of partitions managed by the cluster (note that replicas are not counted as partitions and that the partitions of the controller topic are included) * Label: shard * Aggregation: shard * redpanda_cluster_unavailable_partitions: * Description: number of unavailable partitions in the cluster (i.e. partitions that lack quorum among replicants) * Label: shard * Aggregation: shard

This patch extends the metric querying interface in the redpanda service to allow for fetching metrics from the newly added public endpoint (/public_metrics).

This patch extends the MetricsCheck utility by exposing the metrics endpoint to scrape the metrics from in the interface. As a result, the class can be used to test the metrics exposed by new "/public_metrics" endpoint.

This patch adds tests for the cluster level metrics exposed by the new metrics endpoint.

BenPope

Awesome!

dotnwat

nice stuff

github-actions bot added the area/redpanda label Jun 20, 2022

VladLazar force-pushed the primary-metrics-endpoint branch from c459892 to 61fa813 Compare June 21, 2022 13:21

VladLazar marked this pull request as ready for review June 21, 2022 13:27

VladLazar requested review from BenPope, dotnwat, mmaslankaprv, NyaliaLui, ztlpn and VadimPlh as code owners June 21, 2022 13:27

BenPope reviewed Jun 22, 2022

View reviewed changes

BenPope requested a review from jcsp June 22, 2022 12:53

NyaliaLui reviewed Jun 22, 2022

View reviewed changes

src/v/redpanda/admin_server.cc Show resolved Hide resolved

src/v/pandaproxy/probe.cc Show resolved Hide resolved

NyaliaLui mentioned this pull request Jun 22, 2022

Metrics request bytes #4982

Closed

NyaliaLui reviewed Jun 23, 2022

View reviewed changes

src/v/pandaproxy/probe.cc Outdated Show resolved Hide resolved

BenPope mentioned this pull request Jun 24, 2022

Aggregate metrics to reduce cardinality #5166

Merged

VladLazar force-pushed the primary-metrics-endpoint branch from 61fa813 to 59dc009 Compare June 27, 2022 13:09

VladLazar requested a review from rystsov as a code owner June 27, 2022 13:09

BenPope force-pushed the primary-metrics-endpoint branch from bb55bf4 to 2ae501f Compare June 27, 2022 21:21

BenPope reviewed Jun 27, 2022

View reviewed changes

src/v/redpanda/admin_server.cc Show resolved Hide resolved

BenPope reviewed Jun 30, 2022

View reviewed changes

VladLazar force-pushed the primary-metrics-endpoint branch 2 times, most recently from 6f9d222 to 76d8545 Compare June 30, 2022 14:29

VladLazar force-pushed the primary-metrics-endpoint branch 2 times, most recently from 2748eb7 to 005794d Compare July 1, 2022 14:36

BenPope reviewed Jul 4, 2022

View reviewed changes

VladLazar force-pushed the primary-metrics-endpoint branch from 005794d to 910ea77 Compare July 4, 2022 10:07

VladLazar requested a review from BenPope July 4, 2022 10:50

BenPope previously approved these changes Jul 4, 2022

View reviewed changes

BenPope and others added 6 commits July 4, 2022 15:46

metrics: Allow histogram_logform to be configurable

7248bf1

Signed-off-by: Ben Pope <ben@redpanda.com>

VladLazar dismissed BenPope’s stale review via 66e8f57 July 4, 2022 16:47

VladLazar force-pushed the primary-metrics-endpoint branch 2 times, most recently from 66e8f57 to c71b09a Compare July 5, 2022 12:04

Vlad Lazar added 5 commits July 5, 2022 14:37

tests: expose public endpoint metrics

06acd35

This patch extends the metric querying interface in the redpanda service to allow for fetching metrics from the newly added public endpoint (/public_metrics).

tests: allow MetricsCheck to choose endpoint

8d13fae

This patch extends the MetricsCheck utility by exposing the metrics endpoint to scrape the metrics from in the interface. As a result, the class can be used to test the metrics exposed by new "/public_metrics" endpoint.

tests: add rp tests for cluster level metrics

334d5c5

This patch adds tests for the cluster level metrics exposed by the new metrics endpoint.

VladLazar force-pushed the primary-metrics-endpoint branch from c71b09a to 334d5c5 Compare July 5, 2022 13:38

BenPope approved these changes Jul 5, 2022

View reviewed changes

VladLazar merged commit 8c5c4a7 into redpanda-data:dev Jul 5, 2022

VladLazar mentioned this pull request Jul 6, 2022

Bump seastar version for OSS builds #5359

Merged

dotnwat reviewed Jul 25, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minimal primary metrics endpoint #5165

Minimal primary metrics endpoint #5165

VladLazar commented Jun 20, 2022 •

edited

Loading

BenPope Jun 22, 2022

VladLazar Jun 27, 2022

BenPope Jun 30, 2022

jcsp commented Jun 22, 2022

BenPope commented Jun 22, 2022 •

edited

Loading

emaxerrno commented Jun 22, 2022

emaxerrno commented Jun 22, 2022

VladLazar commented Jun 23, 2022

BenPope commented Jun 27, 2022

BenPope commented Jun 27, 2022

VladLazar commented Jun 28, 2022

BenPope commented Jun 28, 2022

VladLazar commented Jun 28, 2022

BenPope commented Jun 28, 2022

BenPope Jun 30, 2022

VladLazar commented Jun 30, 2022

jcsp commented Jul 1, 2022

BenPope left a comment

BenPope left a comment

BenPope left a comment

dotnwat left a comment

Minimal primary metrics endpoint #5165

Minimal primary metrics endpoint #5165

Conversation

VladLazar commented Jun 20, 2022 • edited Loading

Cover letter

Partition Level Metrics:

Topic Level Metrics:

Broker Level Metrics:

Release notes

Features

BenPope Jun 22, 2022

Choose a reason for hiding this comment

VladLazar Jun 27, 2022

Choose a reason for hiding this comment

BenPope Jun 30, 2022

Choose a reason for hiding this comment

jcsp commented Jun 22, 2022

BenPope commented Jun 22, 2022 • edited Loading

Sometimes the data doesn't exist at the broker level

Sometimes there's no point in adding a broker level metric that increases cardinality

emaxerrno commented Jun 22, 2022

emaxerrno commented Jun 22, 2022

VladLazar commented Jun 23, 2022

BenPope commented Jun 27, 2022

BenPope commented Jun 27, 2022

VladLazar commented Jun 28, 2022

BenPope commented Jun 28, 2022

VladLazar commented Jun 28, 2022

BenPope commented Jun 28, 2022

BenPope Jun 30, 2022

Choose a reason for hiding this comment

VladLazar commented Jun 30, 2022

jcsp commented Jul 1, 2022

BenPope left a comment

Choose a reason for hiding this comment

BenPope left a comment

Choose a reason for hiding this comment

BenPope left a comment

Choose a reason for hiding this comment

dotnwat left a comment

Choose a reason for hiding this comment

VladLazar commented Jun 20, 2022 •

edited

Loading

BenPope commented Jun 22, 2022 •

edited

Loading