Fingerprint CPU model name, count of cores #3
Conversation
|
|
||
| cpuInfo, err := cpu.CPUInfo() | ||
| if err != nil { | ||
| f.logger.Println("[WARN] Error reading CPU information:", err) |
There was a problem hiding this comment.
I feel like we should return the error here, but based on the way Fingerprint is called I think logging and continuing is a good call for now.
Maybe we should look into letting these bubble up to the Client.fingerprint call. Thoughts?
There was a problem hiding this comment.
I meant to ask about this; I think returning an error is ideal, but I wasn't really sure for now so I just logged.
Re: bubbling, maybe change the error to a go-multierror and doing something with them in Client? Seems like a decent idea
There was a problem hiding this comment.
I've been prefixing the messages with something like "fingerprint.os: Error ..." so when users report things it helps us narrow the log site.
|
Looks good! I think we can address my question in another PR. |
| modelName = c.ModelName | ||
| } | ||
| if numCores > 0 { | ||
| node.Attributes["cpu.numcores"] = strconv.FormatInt(int64(numCores), 10) |
There was a problem hiding this comment.
We also need to set the node.Resources.CPU to numCores * cpuFreq
There was a problem hiding this comment.
We don't reliably have cpu frequency yet 😦
|
Looking good, I'd move to |
catsby
force-pushed
the
cpu-fingerprinting
branch
from
944e8b2
to
a0680d9
Compare
Fingerprint CPU model name, count of cores
Add Namespace RPC endpoints
…me-validation do not attempt to validate volumes config if it is not found
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Baby steps to fingerprint CPU information. Sanity check on progress to make sure I'm not doing something dumb...