cluster/partition: Reword description for kafka_request_bytes_total.#25499
Merged
BenPope merged 1 commit intoredpanda-data:devfrom Mar 25, 2025
Merged
Conversation
Collaborator
CI test resultstest results on build#63524
test results on build#63591
|
pgellert
previously approved these changes
Mar 24, 2025
src/v/cluster/partition_probe.cc
Outdated
Comment on lines
323
to
325
| sm::description( | ||
| "Total bytes read from or written to partitions for a topic " | ||
| "(fetched bytes may not all be returned to the client)."), |
Contributor
There was a problem hiding this comment.
nit (ignore unless you end up changing the wording based on docs feedback): you could extract the ss::description to a variable to show that they are shared+identical.
| @@ -309,7 +309,9 @@ void replicated_partition_probe::setup_public_metrics(const model::ntp& ntp) { | |||
| sm::make_total_bytes( | |||
| "request_bytes_total", | |||
Contributor
There was a problem hiding this comment.
This metric probably has the wrong name, the equivalent internal metric is cluster_partition_request_bytes_total.
I'm not following this part of the commit message. The change of reusing the description makes sense, but I am not following why the metric would have the wrong name. It seems reasonable to me, since it is the procuce+fetch total (with a label to allow splitting out either of them). Am I missing something here?
Member
Author
There was a problem hiding this comment.
This metric probably has the wrong name, the equivalent internal metric is cluster_partition_request_bytes_total.
I'm not following this part of the commit message. The change of reusing the description makes sense, but I am not following why the metric would have the wrong name. It seems reasonable to me, since it is the procuce+fetch total (with a label to allow splitting out either of them). Am I missing something here?
It's bytes written to and read from the partition, not necessarily what's returned to the user. It's confusing because it has kafka prefix, but actually measures disk activity. Kafka traffic is measured with redpanda_rpc_sent_bytes{redpanda_server="kafka"} and redpanda_rpc_received_bytes{redpanda_server="kafka"}
Contributor
There was a problem hiding this comment.
Maybe a slight rewording of the first sentence? Also maybe it would be clearer to lose the brackets and add another sentence?
Total number of bytes read from or written to the partitions of a topic. The total may include fetched bytes that are not returned to the client.
asimms41
previously approved these changes
Mar 24, 2025
Contributor
asimms41
left a comment
asimms41
left a comment
There was a problem hiding this comment.
Just a couple of suggestions.
This metric probably has the wrong name, the equivalent internal metric is cluster_partition_request_bytes_total, the metric is measuring disk activity, not kafka network traffic. Reword the description to make it clearer. The description is the same for both produce and consume because only the first one is returned on the prometheus endpoint. Signed-off-by: Ben Pope <ben@redpanda.com>
BenPope
force-pushed
the
core-9575/reword_kafka_request_bytes
branch
from
ce05fb4 to
97ecb4b
Compare
pgellert
approved these changes
Mar 24, 2025
Collaborator
Retry command for Build#63591please wait until all jobs are finished before running the slash command |
Collaborator
|
/backport v25.1.x |
Collaborator
|
/backport v24.3.x |
Collaborator
|
/backport v24.2.x |
Collaborator
|
/backport v24.1.x |
This was referenced Mar 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This metric probably has the wrong name, the equivalent internal metric is
cluster_partition_request_bytes_total, but it's been around a long time now.Reword the description to make it clearer.
The description is the same for both
produceandconsumebecause only the first one is returned on the Prometheus endpoint.Backports Required
Release Notes