-
Notifications
You must be signed in to change notification settings - Fork 23
Stats callback in promisified api #399
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
Conversation
There was a problem hiding this 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 enables statistics callback support in the promisified API for all client types (Producer, Consumer, and Admin). Previously, stats callbacks were only available in the non-promisified API. The implementation moves background polling setup from Producer to the base Connection class, allowing all client types to receive statistics events.
Key Changes:
- Refactored
SetPollInBackgroundfrom Producer-specific to Connection base class - Added stats callback configuration and event handling for Producer, Consumer, and Admin clients
- Added comprehensive test coverage for stats callbacks across all three client types
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/connection.h | Added SetPollInBackground method declaration and m_is_background_polling field to base Connection class |
| src/connection.cc | Implemented SetPollInBackground method in Connection class with background polling queue management |
| src/producer.h | Changed to use inherited SetPollInBackground from Connection class |
| src/producer.cc | Removed Producer-specific SetPollInBackground implementation and m_is_background_polling field |
| src/admin.cc | Added automatic background polling activation when statistics interval is configured |
| lib/kafkajs/_producer.js | Added stats callback extraction, storage, and event listener registration |
| lib/kafkajs/_consumer.js | Added stats callback extraction, storage, and event listener registration |
| lib/kafkajs/_admin.js | Added stats callback extraction, storage, and event listener registration |
| test/promisified/producer/statsCallback.spec.js | Added test verifying Producer receives stats callbacks |
| test/promisified/consumer/statsCallback.spec.js | Added test verifying Consumer receives stats callbacks |
| test/promisified/admin/statsCallback.spec.js | Added test verifying Admin receives stats callbacks |
| CHANGELOG.md | Documented new statistics callback feature for promisified API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
22990aa to
fc56b7e
Compare
PratRanj07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @emasab . LGTM
Statistics callback is available when using the promisified API with all client types.
Note for the reviewers:
SetPollInBackgroundis moved to the Connection base class, false by default and it's public only on the Producer where it's possible to chose if polling externally. For the AdminClient it's enabled only if thestatistics.interval.msis set as the main reply queue wasn't polled.Checklist
statistics.interval.mstogether with thestats_cband receive the statistics