-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(metrics): track consumer records lag #2319
Conversation
5e9e0c7
to
84d21df
Compare
cad49e9
to
0a0303a
Compare
@dnwe would you mind reviewing the PR please? I fixed the cla failure and lint errors it was reporting. |
0a0303a
to
271404c
Compare
if len(b.RecordsSet) > 0 { | ||
logEndOffset := *b.LastRecordsBatchOffset - 1 | ||
partitionLag := logEndOffset - b.RecordsSet[len(b.RecordsSet)-1].RecordBatch.LastOffset() | ||
if lagMetric != nil { | ||
lagMetric.Update(partitionLag) | ||
} | ||
} |
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.
I think it makes sense to pull the lagMetric != nil
check up so we avoid doing working out for nothing. That is:
if len(b.RecordsSet) > 0 { | |
logEndOffset := *b.LastRecordsBatchOffset - 1 | |
partitionLag := logEndOffset - b.RecordsSet[len(b.RecordsSet)-1].RecordBatch.LastOffset() | |
if lagMetric != nil { | |
lagMetric.Update(partitionLag) | |
} | |
} | |
if lagMetric != nil && len(b.RecordsSet) > 0 { | |
logEndOffset := *b.LastRecordsBatchOffset - 1 | |
partitionLag := logEndOffset - b.RecordsSet[len(b.RecordsSet)-1].RecordBatch.LastOffset() | |
lagMetric.Update(partitionLag) | |
} |
Thank you for your contribution! However, this pull request has not had any activity in the past 90 days and will be closed in 30 days if no updates occur. |
No description provided.