Add process ID to server data #171
Conversation
It's useful to report the PID of the process that raised the exception, this commit adds it to server data so we can see it in Rollbar.
| @@ -370,6 +370,7 @@ def server_data | |||
| } | |||
| data[:root] = configuration.root.to_s if configuration.root | |||
| data[:branch] = configuration.branch if configuration.branch | |||
| data[:pid] = Process.pid | |||
There was a problem hiding this comment.
Because of the platform uncertainty, perhaps this should be wrapped in begin/rescue?
There was a problem hiding this comment.
Perhaps Process.pid if Process.respond_to?(:pid) is enough.
There was a problem hiding this comment.
Mmm, not enough not. Process.pid seems to be defined always.
There was a problem hiding this comment.
The docs don't say what actually happens on platforms its not supported on. Does it return nil? Does it raise? We could tack on a "rescue nil", and assume its not supported if its missing from server_data.
There was a problem hiding this comment.
https://github.com/ruby/ruby/blob/trunk/process.c#L7578
If defined for all platforms, I'd say it will not crash at least.
There was a problem hiding this comment.
@jondeandres sounds good. Worst-ish case, it'll report an internal error, so at least there will still be some visibility.
There was a problem hiding this comment.
Awesome! Server.pid shows up in the web interface as expected. Thanks again Brian.
There was a problem hiding this comment.
Great! Thanks for this PR - I'll merge as soon as the build passes.
There was a problem hiding this comment.
Perhaps could be very cool do something like this:
data[:process] = "#{$0} (#{Process.pid})"
What do you think?
There was a problem hiding this comment.
In some other libraries (like pyrollbar) we send the process args as server.argv. Should probably stick with that here.
e.g. https://github.com/rollbar/pyrollbar/blob/master/rollbar/__init__.py#L952
|
Merged, thanks! This will be part of the next release (1.2.8). |
|
Released in 1.2.8 |
It's useful to report the PID of the process that raised the exception,
this commit adds it to server data so we can see it in Rollbar.
I will manually test it a little more, please don't merge until I report that it works properly.
I've only run the specs with this Ruby:
ruby 2.1.2p95 (2014-05-08 revision 45877) [x86_64-linux]
Process.pid seems to work on one Windows computer we have here, though I don't know how universal support is or whether an exception is thrown if it is not supported. Ruby documentation says it doesn't work on all platforms.