-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Metricbeat] Fix silent failures in kafka and prometheus #13353
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.
LGTM
Only one extra suggestion, what do you think about logging the error in the kafka module at the debug level only? (the one that is logged now at the error level if !offOk
). These errors can flood logs when happen.
} | ||
|
||
block := resp.GetBlock(topic, partition) | ||
if len(block.Offsets) == 0 { | ||
return -1, nil | ||
return -1, errors.Wrap(block.Err, "block offsets is empty") |
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 am worried this can change some behaviour. But I don't think so, at the end an error was reported by the metricset on this case.
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.
Yeah previously it will return nil but offOK
returned from queryOffsetRange
will still be false so it goes into the same if statement.
Yeah good point! |
* Fix silent failures in kafka and prometheus * Change error log to warning
This PR contains two tiny fixes for silent failures due to missing proper return error. One is in kafka module, when
GetAvailableOffsets
failed, the error is not reported. The other place is in prometheus module, it silently fails if metric name is incorrect (#13252).