-
Notifications
You must be signed in to change notification settings - Fork 23
Add highwatermark offset to eachBatch callback #317
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 adds support for exposing the high watermark offset and lag information in the eachBatch callback, including updating tests and documentation.
- Updates consumer batch payload computation to include highWatermark, offsetLag, and offsetLagLow.
- Enhances tests to verify the correct reporting of these values for multiple partitions.
- Updates examples and the CHANGELOG to reflect these changes.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/promisified/consumer/consumeMessages.spec.js | Adds a new test case to validate highWatermark and lag values. |
| lib/kafkajs/_consumer.js | Updates batch payload construction with highWatermark and lag calculations. |
| examples/typescript/kafkajs.ts | Adds configuration for auto.offset.reset. |
| CHANGELOG.md | Documents new features added in this release. |
3f6861d to
2c473e1
Compare
2c473e1 to
297cfa5
Compare
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
Adds support for reporting the high watermark and lag metrics in each batch callback and updates the consumer implementation and tests accordingly.
- Fetches and exposes
highWatermark,offsetLag(), andoffsetLagLow()ineachBatch - Updates the internal consumer to call
getWatermarkOffsetsand calculate lag - Adds tests and example usage demonstrating the new metrics
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/promisified/consumer/consumeMessages.spec.js | New test case validating highWatermark and lag values |
| lib/kafkajs/_consumer.js | Batch payload now fetches watermark offsets and computes lag |
| examples/typescript/kafkajs.ts | Example consumer config updated with offset reset option |
| CHANGELOG.md | Notes the new highWatermark and lag enhancements (v1.4.0) |
Comments suppressed due to low confidence (3)
test/promisified/consumer/consumeMessages.spec.js:920
- The test waits for new messages but doesn’t assert that
batch.highWatermarkor lag metrics are updated after the second send; consider adding explicit assertions for the updated watermark and lag.
await waitFor(() => (messagesConsumed === (partition0ProducedMessages + messages1.length + messages2.length)), () => { }, 100);
lib/kafkajs/_consumer.js:866
getWatermarkOffsetsis likely asynchronous; you need toawaitits result and mark#createBatchPayloadasasyncto ensure correct watermark values.
const watermarkOffsets = this.#internalClient.getWatermarkOffsets(topic, partition);
examples/typescript/kafkajs.ts:47
- [nitpick] The option
'auto.offset.reset'may not be recognized at this level; document or move it into the correct consumer config namespace or usefromBeginning: trueinstead.
'auto.offset.reset': 'earliest',
This comment has been minimized.
This comment has been minimized.
cbfdd07 to
2ffde4b
Compare
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.
LGTM! Thanks for the implementation
Fixes #282.