Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export number of members and assigned partitions for each topic in a consumer group #106

Merged
merged 2 commits into from
Sep 20, 2021

Conversation

amuraru
Copy link
Contributor

@amuraru amuraru commented Jul 31, 2021

Added consumer_group_topic_members, consumer_group_topic_assigned_partitions and consumer_group_empty_members metrics.

Fixes #104

@amuraru amuraru force-pushed the issue-104 branch 7 times, most recently from 46f428c to 37283c5 Compare August 1, 2021 23:00
Copy link
Contributor

@weeco weeco left a comment

Choose a reason for hiding this comment

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

Hey Adi,
generally LGTM only a few minor things that improve readability, naming etc. I edited them just in GitHub - so the suggested changes may require further changes (e.g. changing the variable names in other places where it's been used etc).

Thanks 🚀

prometheus/collect_consumer_groups.go Outdated Show resolved Hide resolved
prometheus/collect_consumer_groups.go Outdated Show resolved Hide resolved
prometheus/collect_consumer_groups.go Outdated Show resolved Hide resolved
prometheus/collect_consumer_groups.go Outdated Show resolved Hide resolved
Comment on lines 90 to 91
// number of members with no assignment in a stable consumer group
if membersWithEmptyAssignment > 0 && group.State == "Stable" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering whether we actually want that condition again (or just report it as zero everytime where this is case - regardless of the current group state). I assume that you have a certain usecase in mind which is why you added these metrics in the first place. Do you care to explain them so that I can understand the idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right - superseded by #108

prometheus/exporter.go Outdated Show resolved Hide resolved
@amuraru amuraru force-pushed the issue-104 branch 2 times, most recently from 89de365 to 249d16b Compare August 3, 2021 08:33
@amuraru
Copy link
Contributor Author

amuraru commented Sep 1, 2021

@weeco kind reminder to review this PR. thanks :)

prometheus/collect_consumer_groups.go Outdated Show resolved Hide resolved
prometheus/exporter.go Outdated Show resolved Hide resolved
prometheus/exporter.go Outdated Show resolved Hide resolved
prometheus/exporter.go Outdated Show resolved Hide resolved
@amuraru
Copy link
Contributor Author

amuraru commented Sep 20, 2021

thanks for the review @weeco - would you please check the last version?

…consumer group

Added consumer_group_empty_members, consumer_group_topic_members and 
consumer_group_topic_assigned_partitions metrics.

Fixes redpanda-data#104
@weeco weeco merged commit dff9857 into redpanda-data:master Sep 20, 2021
@amuraru amuraru deleted the issue-104 branch October 23, 2021 21:51
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.

Export number of members and assigned partitions for each topic in a consumer group
2 participants