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

Feat/consumer offset no consumers #172

Merged
merged 30 commits into from
Jul 1, 2022

Conversation

paologallinaharbur
Copy link
Member

@paologallinaharbur paologallinaharbur commented May 25, 2022

Add consumerGroup offsets and lag for consumer groups with inactive consumers

Makefile Outdated Show resolved Hide resolved
@alvarocabanas alvarocabanas force-pushed the feat/consumerOffsetNoConsumers branch from 9eb35ef to 252744a Compare June 21, 2022 15:47
@alvarocabanas alvarocabanas marked this pull request as ready for review June 29, 2022 09:58
@sigilioso sigilioso requested a review from a team June 29, 2022 13:39
Copy link
Contributor

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

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

I think the changes have good shape, great job so far! I left some comments, basically typos and some suggestion to try to make it easier to follow.

src/args/args.go Outdated Show resolved Hide resolved
src/args/args.go Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect.go Outdated Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect.go Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect.go Outdated Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments! I think it is easier to follow now. I've left a couple of additional comments and questions.

src/consumeroffset/cgroup_metrics_aggregator.go Outdated Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect.go Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect.go Show resolved Hide resolved
src/consumeroffset/kafka_offset_collect.go Show resolved Hide resolved
@alvarocabanas alvarocabanas force-pushed the feat/consumerOffsetNoConsumers branch from 001a78f to 1f792b3 Compare July 1, 2022 07:53
Copy link
Contributor

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

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

Great job! 👍

@alvarocabanas alvarocabanas merged commit 87c6c55 into master Jul 1, 2022
@alvarocabanas alvarocabanas deleted the feat/consumerOffsetNoConsumers branch July 1, 2022 10:06
@alvarocabanas alvarocabanas changed the title Feat/consumer offset no consumers Feat/consumer offset no consumers - issue https://github.com/newrelic/nri-kafka/issues/160 Jul 12, 2022
@alvarocabanas alvarocabanas changed the title Feat/consumer offset no consumers - issue https://github.com/newrelic/nri-kafka/issues/160 Feat/consumer offset no consumers Jul 12, 2022
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.

3 participants