adds CmdLine() exception handler#5
Conversation
|
👍 but let's have @grobie take a look to verify. |
proc.go
Outdated
There was a problem hiding this comment.
Could you do: "could not read cmdline for pid: %d"
There was a problem hiding this comment.
Actually, should this really return an error, or just an empty command-line?
There was a problem hiding this comment.
Hmm, just return an empty command-line make more sense to me.
There was a problem hiding this comment.
Yeah. Curious - are you using this function from somewhere? Seems like the "prometheus" github org is not even using it anywhere yet.
There was a problem hiding this comment.
Okay, I'm building some kind of system event listener including docker and kernel proc events.
As a part of the listener, I found this repository for proc fs reader.
And we're planning to use prometheus for our system metric server. :)
|
Please add a test case as well. |
|
I added a test case for an empty cmdline file. |
proc.go
Outdated
There was a problem hiding this comment.
I think we should return []string{}, nil here, as the assumption in go for type, error return signatures is usually: if err is nil then the returned type is an initialized value.
|
Thanks for your comment. |
|
👍 Great, thanks! Now comes the last pedantry thingy ;), we usually tend to squash our commits together at the end of a PR review, so that we have only a few coherent commits with all changes to code+tests+fixtures together. I'll merge immediately after squashing them to a nice commit. |
|
👍 Great! |
|
@jhseol Hey hey, but the commits are still not squashed - could you squash them? Then we'll merge :) |
|
Commits are squashed. Is it okay to merge? |
|
@jhseol Hmm, somehow the "Update example" commit by @audebert made it into your squashed commits (https://github.com/prometheus/procfs/pull/5/commits), but under a different SHA than in master. This should fix it: |
|
Commits are squashed again. I'm not sure how "Update example" commit was squashed with my commit. :( |
|
@jhseol no worries - looking good now :) Thanks! Merging. |
adds CmdLine() exception handler
Updates OWNERS file
read kernel config from /boot
4 participants
If /proc/pid/cmdline has no content, slice operation of the 'data' will be panicked. (e.g. kernel threads)