Added remote mode support #30
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed the CLA. |
|
CLAs look good, thanks! |
|
Please no. This tool is designed to work against PID -- that's the sole reason why it exists. |
|
@rakyll IDK really, it'd play nicely with current functionality since gops uses TCP socks under the hood (only client's UI limits us to use it remotely). Maybe I should create gops-compatible client for remote use then? |
|
What about accepting host:port along with PIDs? E.g. |
|
@rakyll fork does that already :) |
|
Ooops, I looked at the README and saw "target" and assumed it is remote only and breaking the existing behavior. I am going to take a look at the PR very soon! |
README.md
Outdated
| It is possible to use gops tool both in local and remote mode. | ||
|
|
||
| Local mode requires that you start the target binary as the same user that runs gops binary. | ||
| To use gops in a remote mode you need to know target binary's agent's host and port. |
There was a problem hiding this comment.
To use gops in remote mode, you need to know target's agent address.
README.md
Outdated
| @@ -82,31 +89,31 @@ created by github.com/google/gops/agent.Listen | |||
| # ... | |||
| ``` | |||
|
|
|||
| #### $ gops memstats \<pid\> | |||
| #### $ gops memstats \<target\> | |||
There was a problem hiding this comment.
We should use the one or the other notation:
gops memstats (<pid>|<addr>)
Use for the agent URL.
cmd.go
Outdated
| ) | ||
|
|
||
| var cmds = map[string](func(pid int) error){ | ||
| var cmds = map[string](func(target string) error){ |
main.go
Outdated
| pid, err := strconv.Atoi(os.Args[2]) | ||
| if err != nil { | ||
| usage("PID should be numeric") | ||
| usage("missing PID or host:port combo") |
main.go
Outdated
| } | ||
| fn, ok := cmds[cmd] | ||
| if !ok { | ||
| usage("unknown subcommand") | ||
| } | ||
| if err := fn(pid); err != nil { | ||
| if err := fn(os.Args[2]); err != nil { |
There was a problem hiding this comment.
shouldn't we do the os.Args[2] to addr net.TCPAddr conversion here?
cmd.go
Outdated
|
|
||
| out, err := cmd(*addr, signal.BinaryDump) | ||
| if err != nil { | ||
| return errors.New("Couldn't retrieve running binary's dump") |
|
@rakyll thanks for the CR! Fixed. |
|
Thanks much!!! There are a few minor things but I patch them fix after merging! |
This PR adds a possibility to use this tool for instrumented binaries running on remote hosts.
PR adds new protocol command, which dumps running service's binary file. Two dependencies were added, namely github.com/pkg/errors for errors' contexts and github.com/kardianos/osext, which helps agent find the path to executable file.
Interface changes are simple: it is now possible to pass
host:portcombo instead ofPIDto gops tool.