KAFKA-19542: Consumer.close() does not remove all added sensors from Metrics #20631
+509
−423
Conversation
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
…Metrics Introduces AbstractConsumerMetricsManager as a shared base class for consumer metrics managers, consolidating metric and sensor registration and cleanup logic. Updates all relevant metrics manager classes to extend this base, reducing code duplication and improving maintainability.
Changed AbstractConsumerMetricsManager to use Set for sensors and added debug output in close(). Introduced AbstractConsumerMetricsManagerTest as a base test class for metrics cleanup verification. Updated AsyncConsumerMetricsTest and ConsumerRebalanceMetricsManagerTest to extend the new base test and implement required methods.
Moved sensor and metric management logic into AbstractConsumerMetricsManager, replacing SensorBuilder with an inner class. Updated FetchMetricsManager, ShareFetchMetricsManager, and related classes to extend AbstractConsumerMetricsManager and use its utilities. Ensured proper cleanup of metrics and sensors on close, added tests to verify metrics removal, and improved constructor usage with @SuppressWarnings. This change centralizes and simplifies metrics lifecycle management for Kafka consumers.
LiamClarkeNZ
approved these changes
Oct 4, 2025
|
|
||
| assertTrue(consumer.metrics().size() > 1, "The consumer should have created many metrics"); | ||
| consumer.close(CloseOptions.timeout(Duration.ZERO)); | ||
| assertTrue(consumer.metrics().size() <= 1, "The consumer should have removed all of its metrics"); |
There was a problem hiding this comment.
Curious about the > 1 and <= 1, reads like there's usually a metric just hanging around in there?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 change introduces
AbstractConsumerMetricsManageras a shared base class for consumer metrics managers, consolidating the steps of registering and cleanup logic.