Skip to content

Conversation

dhruvCW
Copy link

@dhruvCW dhruvCW commented Jul 30, 2025

@Copilot Copilot AI review requested due to automatic review settings July 30, 2025 12:48
@dhruvCW dhruvCW requested a review from a team as a code owner July 30, 2025 12:48
Copilot

This comment was marked as outdated.

@dhruvCW dhruvCW force-pushed the prometheus_metrics branch from b7d42a6 to d3fbea9 Compare July 30, 2025 13:01
@dhruvCW dhruvCW force-pushed the prometheus_metrics branch from d3fbea9 to 4a30e94 Compare July 30, 2025 13:16
@dhruvCW dhruvCW requested a review from Copilot July 30, 2025 13:23
@dhruvCW dhruvCW self-assigned this Jul 30, 2025
Copilot

This comment was marked as outdated.

@dhruvCW dhruvCW force-pushed the prometheus_metrics branch from e30bb49 to ef07ea9 Compare July 30, 2025 13:25
@dhruvCW dhruvCW requested a review from Copilot July 30, 2025 13:26
Copilot

This comment was marked as outdated.

@dhruvCW dhruvCW requested a review from Copilot July 30, 2025 13:33
Copilot

This comment was marked as outdated.

@dhruvCW dhruvCW requested a review from Copilot July 30, 2025 13:39
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Prometheus metric reporting support for the ruby-kafka library by implementing a comprehensive monitoring system. The implementation includes metrics tracking for various Kafka operations including producer, consumer, and connection activities.

  • Implements Prometheus metrics collection for Kafka operations using ActiveSupport subscribers
  • Adds comprehensive test coverage for all monitored Kafka events and metrics
  • Extends consumer batch processing to include last message create time for time lag calculations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
lib/kafka/prometheus.rb Core implementation of Prometheus metrics collection with subscriber classes for different Kafka components
spec/prometheus_spec.rb Comprehensive test suite covering all Prometheus metrics and event scenarios
lib/kafka/consumer.rb Minor enhancement to include last message create time in batch processing payload
Gemfile Adds prometheus-client gem dependency for development/test environments

Copy link

@nithyanandankm nithyanandankm left a comment

Choose a reason for hiding this comment

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

Overall looks good, nice one!

Is there an easy way to generate some metrics in staging to make sure that it works as expected?

@fcbr
Copy link
Member

fcbr commented Jul 31, 2025

why are you only adding metrics to the consumer?

@dhruvCW
Copy link
Author

dhruvCW commented Jul 31, 2025

Overall looks good, nice one!

Is there an easy way to generate some metrics in staging to make sure that it works as expected?

I would have to create a pre-release gem which isn't the easiest for this gem. Since this is an opt-in feature I am not sure if it would break anything. LMK if I am missing anything

@dhruvCW
Copy link
Author

dhruvCW commented Jul 31, 2025

why are you only adding metrics to the consumer?

Um metrics are for both producers and consumers. The consumer was missing a data point so I added that.

Copy link
Member

@fcbr fcbr left a comment

Choose a reason for hiding this comment

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

LGTM but I strongly recommend manually publishing a pre-release version and testing this before merging this.

You can just use gem release and use semver to mark it as -alpha and it should work fine. I've done this in the past - here's some reference docs. You need a PAT with the proper permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants