-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Don't ignore unit if NRestarts isn't available #1039
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
This is a duplicate of #1038. |
@SuperQ That was one was |
@fastest963 Nice, thanks! |
collector/systemd_linux.go
Outdated
} | ||
unit.currentConnections = currentConnectionCount.Value.Value().(uint32) | ||
|
||
// NRefused wasn't added until systemd 239 |
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.
Nit, please end comments with a .
Would you mind updating the commit description with a full list of changes? Also, please update the |
Beside @SuperQ's comment, LGTM! |
* If NRestarts or NRefused are not available, don't ignore the unit itself * Don't report systemd metrics (NRestarts/NRefused) that are not available Signed-off-by: James Hartig <[email protected]>
Thanks @SuperQ and @discordianfish! I just updated the commit message and changelog. I filed the ignoring units as BUGFIX and the not reporting as CHANGE, let me know if that's sufficient. |
@fastest963 ugh, sorry about that. Thank you for fixing! |
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
@fastest963 Great, thanks! Just a final rebase to solve the changelog conflicts and we can merge this. |
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
…metheus#1039) * If NRestarts or NRefused are not available, don't ignore the unit itself * Don't report systemd metrics (NRestarts/NRefused) that are not available Signed-off-by: James Hartig <[email protected]>
Pulled from #1036. The latest systemd in our CentOS is not new enough to have NRestarts so this effectively means the systemd collector cannot collect any services. Instead of ignoring a service without NRestarts, just log and leave it at 0.
Fixes: #567
@SuperQ @discordianfish