-
Notifications
You must be signed in to change notification settings - Fork 215
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
Fix kafka buffer metrics #3805
Fix kafka buffer metrics #3805
Conversation
Signed-off-by: Krishna Kondaka <[email protected]>
recordsWrittenCounter.increment(); | ||
recordsInBuffer.incrementAndGet(); | ||
if (!isByteBuffer()) { | ||
recordsWrittenCounter.increment(); |
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.
With protobuf wrapper over byteBuffer can we store numRecords along with byteBuffer? and then increment/decrement these metrics? these are good metrics for customers IMO.
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.
Maybe. I am not sure if it's always possible because it means we have to parse the input, which we do not plan to do afaik.
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.
We should add some metrics for the byte counts.
Also, I think we should move away from this AbstractBuffer
. We make our second buffer and it is already producing erroneous results. :)
But, that can come later.
recordsWrittenCounter.increment(); | ||
recordsInBuffer.incrementAndGet(); | ||
if (!isByteBuffer()) { | ||
recordsWrittenCounter.increment(); |
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.
We should add some metrics for the byte counts.
Also, I think we should move away from this AbstractBuffer
. We make our second buffer and it is already producing erroneous results. :)
But, that can come later.
Description
Fix Kafka Buffer metrics
While reading from Kafka inner buffer
doRead
API was used. This skips updating the metrics. modified to useread
API which callsdoRead
and also updates metrics.Issues Resolved
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.