Fix for #945, cpu temperature is signed. #965
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a type conversion to cpu temperature sysctl. Will still collect/report -1 when the value is -1, this is because it should be up to interpretation whether this is the correct value for the system or not. Some drivers will report -1 for cpu temperature. Other sensors will report "an input into the fan control algorithm", i.e. not the actual temperature, but how much fan it wants. Some people cool their machines with liquid nitrogen. Signed-off-by: Derek Marcotte <554b8425@razorfever.net>
derekmarcotte
force-pushed
the
dm-fix-#945
branch
from
64f1969 to
18b68a5
Compare
|
LGTM but maybe extend the comment a bit to explain what exactly is going on there? |
|
Let's re-arrange this a little so it's easier to understand. |
derekmarcotte
force-pushed
the
dm-fix-#945
branch
from
bd9cd0e to
5778fd7
Compare
Signed-off-by: Derek Marcotte <554b8425@razorfever.net>
derekmarcotte
force-pushed
the
dm-fix-#945
branch
from
5778fd7 to
38cd211
Compare
Signed-off-by: Derek Marcotte <554b8425@razorfever.net>
derekmarcotte
force-pushed
the
dm-fix-#945
branch
from
38cd211 to
c9c5bb0
Compare
|
Codespell ftw!
But LGTM beside that one. |
SuperQ
reviewed
Jun 6, 2018
collector/cpu_freebsd.go
Outdated
| ch <- c.temp.mustNewConstMetric(float64(temp-2732)/10, lcpu) | ||
|
|
||
| // Temp is a signed integer in deci-degrees Kelvin. | ||
| // Cast uint32 to int32 and convert to float64 degrees Celcius. |
There was a problem hiding this comment.
Codespell error :-)
./collector/cpu_freebsd.go:143: Celcius ==> Celsius
Signed-off-by: Derek Marcotte <554b8425@razorfever.net>
oblitorum
pushed a commit
to shatteredsilicon/node_exporter
that referenced
this pull request
Apr 9, 2024
* Fix for prometheus#945, cpu temperature is signed. Added a type conversion to cpu temperature sysctl. Will still collect/report -1 when the value is -1, this is because it should be up to interpretation whether this is the correct value for the system or not. Some drivers will report -1 for cpu temperature. Other sensors will report "an input into the fan control algorithm", i.e. not the actual temperature, but how much fan it wants. Some people cool their machines with liquid nitrogen. Signed-off-by: Derek Marcotte <554b8425@razorfever.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added a type conversion to cpu temperature sysctl. Will still collect/report -1 when the value is -1, this is because it should be up to interpretation whether this is the correct value for the system or not.
Some drivers will report -1 for cpu temperature. Other sensors will report "an input into the fan control algorithm", i.e. not the actual temperature, but how much fan it wants. Some people cool their machines
with liquid nitrogen.
Closes: #945