Skip to content

KAFKA-13436: Omitted BrokerTopicMetrics metrics in the documentation#11473

Merged
mimaison merged 8 commits intoapache:trunkfrom
dongjinleekr:feature/KAFKA-13436
Jun 13, 2022
Merged

KAFKA-13436: Omitted BrokerTopicMetrics metrics in the documentation#11473
mimaison merged 8 commits intoapache:trunkfrom
dongjinleekr:feature/KAFKA-13436

Conversation

@dongjinleekr
Copy link
Copy Markdown
Contributor

I found this issue while working on KAFKA-13354.

As soon as this PR is approved, I will open a PR on the kafka-site also.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dongjinleekr
Copy link
Copy Markdown
Contributor Author

@splett2 Could you have a look while I am updating #11474?

Comment thread docs/ops.html Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like we're missing an explanation for what these metrics measure.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes let's try to include descriptions as above we recommend to graph and alert on these metrics

@dongjinleekr
Copy link
Copy Markdown
Contributor Author

@splett2 Here is the fix. Reviewing the documentation, I found that 1. the other 'BrokerTopicMetrics' metrics are omitting ,topic=([-.\w]+) at the end 2. also omitting the descriptions. I fixed all of them at once.

@dongjinleekr
Copy link
Copy Markdown
Contributor Author

As soon as this PR is merged, I will open a corresponding PR in kafka-site.

Comment thread docs/ops.html Outdated
@dongjinleekr
Copy link
Copy Markdown
Contributor Author

@dajac Here is the update; As you can see here, only Message conversion rate and Rejected byte rate have detailed descriptions now.

@dongjinleekr
Copy link
Copy Markdown
Contributor Author

@splett2 @mimaison Sorry for being late. Here is the fix.

Not only rebasing onto the latest trunk, I carefully inspected the implementation of BrokerTopicStats and did the following:

  • For the metrics provided on both of per-topic and all-topic basis (including newly documented four), the topic=([-.\w]+) tag and detailed descriptions are added, with an explanation on all-topic metrics.
  • For the metrics provided on an all-topic basis only, I added some simple descriptions on normal values for consistency.

@dongjinleekr dongjinleekr force-pushed the feature/KAFKA-13436 branch from dddea0d to d3c38f0 Compare June 6, 2022 00:03
@dongjinleekr
Copy link
Copy Markdown
Contributor Author

Rebased onto the latest trunk.

Copy link
Copy Markdown
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dongjinleekr for the PR. I left a few suggestions

Comment thread docs/ops.html Outdated
Comment thread docs/ops.html Outdated
Comment thread docs/ops.html Outdated
Comment thread docs/ops.html Outdated
…=([-.\w]+)' to the BrokerTopicMetrics metrics.
1. Add description on all-topic metrics; these metrics are provided on both of per-topic, all-topic basis. (see: BrokerTopicStats#topicStats, BrokerTopicStats#allTopicsStats)

  - MessagesInPerSec
  - BytesInPerSec
  - BytesOutPerSec
  - ReplicationBytesInPerSec
  - ReplicationBytesOutPerSec
  - TotalProduceRequestsPerSec
  - TotalFetchRequestsPerSec
  - FailedProduceRequestsPerSec
  - FailedFetchRequestsPerSec
  - ProduceMessageConversionsPerSec
  - FetchMessageConversionsPerSec
  - BytesRejectedPerSec

2. Remove the 'topic' attribute from all-topic only metrics:

  - NoKeyCompactedTopicRecordsPerSec
  - InvalidMagicNumberRecordsPerSec
  - InvalidMessageCrcRecordsPerSec
  - InvalidOffsetOrSequenceRecordsPerSec
  - ReassignmentBytesOutPerSec
  - ReassignmentBytesInPerSec
@dongjinleekr dongjinleekr force-pushed the feature/KAFKA-13436 branch from d3c38f0 to a517fbf Compare June 11, 2022 13:48
@dongjinleekr
Copy link
Copy Markdown
Contributor Author

@mimaison Here it is, with rebasing onto the latest trunk. 🙇

Copy link
Copy Markdown
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mimaison mimaison merged commit 89c0573 into apache:trunk Jun 13, 2022
ableegoldman added a commit to confluentinc/kafka that referenced this pull request Jun 14, 2022
CONFLUENT: Sync from apache/kafka trunk to confluentinc/kafka master (13 Jun 2022)

apache/trunk: (7 commits)
KAFKA-13891: reset generation when syncgroup failed with REBALANCE_IN…(apache#12140)
KAFKA-10000: Exactly-once source tasks (apache#11780)
KAFKA-13436: Omitted BrokerTopicMetrics metrics in the documentation (apache#11473)
MINOR: Use Exit.addShutdownHook instead of directly adding hooks to R…(apache#12283)
KAFKA-13846: Adding overloaded metricOrElseCreate method (apache#12121)
KAFKA-13935 Fix static usages of IBP in KRaft mode (apache#12250)
HOTFIX: null check keys of ProducerRecord when computing sizeInBytes (apache#12288)


Conflicts:
None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants