Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: show message with link to debug server with -debug=:0 #37215

Open
hyangah opened this issue Feb 13, 2020 · 7 comments
Open

x/tools/gopls: show message with link to debug server with -debug=:0 #37215

hyangah opened this issue Feb 13, 2020 · 7 comments

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Feb 13, 2020

currently we ask users to supply flags such as -port and -rpc.trace and restart gopls when debugging is needed. Please consider turning on the debug port by default and make some traces available.

  • Port number needs to be picked up randomly - so we will need a way for the client to ask gopls about the debug port location.
  • May need too allow opt-out.
@gopherbot gopherbot added this to the Unreleased milestone Feb 13, 2020
@hyangah hyangah removed the Tools label Feb 13, 2020
@stamblerre stamblerre removed this from the Unreleased milestone Feb 13, 2020
@stamblerre stamblerre added this to the gopls/v0.4.0 milestone Feb 13, 2020
@hyangah hyangah added the Tools label Feb 13, 2020
@findleyr
Copy link
Contributor

@findleyr findleyr commented Mar 5, 2020

FWIW I opted for this when using a shared gopls daemon (#34111): -debug is enabled on the daemon by default (though some users have already asked to be able to disable it, so I'll support that as well).

I also used an extra gopls/handshake RPC to exchange debug information between forwarder and daemon, including the debug port and the location of logs. We could perhaps rename and generalize this to expose this information to LSP clients.

Loading

@leitzler
Copy link
Contributor

@leitzler leitzler commented Mar 5, 2020

-rpc.trace by default is a good idea! I'm not sure however that I think exposing the debug port is.

The gopls process often runs on a local environment, and people that use multi user systems will probably not be aware of the fact that "anyone" on that machine can connect to their gopls instance. The main concern is that the contents of the file being edited is available directly and that makes it trivial to grab information.

Loading

@stamblerre
Copy link

@stamblerre stamblerre commented Mar 20, 2020

@heschik made the point that turning on -rpc.trace by default would make logs difficult to parse for users who are just looking to find a single error message, so I think we should maybe keep it off by default.

I am not sure about turning on the debug server by default because of the concerns that @leitzler mentioned, but maybe we can start with a flag that makes it a bit easier to enable and then go from there.

Loading

@findleyr
Copy link
Contributor

@findleyr findleyr commented Mar 23, 2020

@stamblerre what would an easier flag look like? -debug=<addr> is pretty easy. The one thing I wish were easier is finding the port when run with -debug=:0 (you have to scrape the start of the logs).

One of the nice things about daemon mode is that it was easy to hook up some debugging over jsonrpc2 without a debug server (https://golang.org/cl/222669). I wonder if we could do the same for sidecar serving (i.e., add a debugHandler to the jsonrpc2 handler stack and add functionality to the vscode client to inspect debug information).

Pie in the sky: If we standardized a jsonrpc2 protocol (meta-protocol?) for debug information, we could even turn the current debug webserver into a separate jsonrpc2 client process that can be attached even after the gopls server is started...

Don't want to derail this discussion though! My opinion is that the current state, while suboptimal, is better than enabling -rpc.trace or -debug by default.

Loading

@stamblerre
Copy link

@stamblerre stamblerre commented Mar 23, 2020

Don't want to derail this discussion though! My opinion is that the current state, while suboptimal, is better than enabling -rpc.trace or -debug by default.

That's fair. I was thinking something like -debug=auto which could automatically turn on -rpc.trace and also start up the debug server on a random port, which could then show the link to the user with window/showMessage.

Loading

@findleyr
Copy link
Contributor

@findleyr findleyr commented Mar 23, 2020

That's fair. I was thinking something like -debug=auto which could automatically turn on -rpc.trace and also start up the debug server on a random port, which could then show the link to the user with window/showMessage.

Not sure about coupling -debug and -rpc.trace, but I really like the idea of showing the debug link with window/showMessage. This would be very useful with -debug=:0. That seems worthwhile in any case (we might also want to consider this for the logfile location).

Loading

@stamblerre stamblerre removed this from the gopls/v0.4.0 milestone Apr 2, 2020
@stamblerre stamblerre added this to the gopls/v0.5.0 milestone Apr 2, 2020
@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 24, 2020
@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Jul 7, 2020

FYI - in vscode-go, we have client-side tracing that can be enabled/disabled anytime without restarting the gopls. So for vscode-go, -rpc.trace is not critical. But the debug server may be useful.

Loading

@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Jul 23, 2020
@stamblerre stamblerre changed the title x/tools/gopls: enable debug server and some tracing by default x/tools/gopls: show message with link to debug server with -debug=:0 Sep 9, 2020
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Sep 9, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre stamblerre removed this from the gopls/unplanned milestone Nov 13, 2020
@stamblerre stamblerre added this to the gopls/vscode-go milestone Nov 13, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default Nov 13, 2020
@stamblerre stamblerre moved this from Needs Triage to Non-critical in vscode-go: gopls by default Nov 13, 2020
@stamblerre stamblerre removed this from Non-critical in vscode-go: gopls by default Dec 28, 2020
@stamblerre stamblerre removed this from the gopls/vscode-go milestone Dec 28, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Dec 28, 2020
@stamblerre stamblerre removed this from the gopls/unplanned milestone Dec 28, 2020
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Dec 28, 2020
@stamblerre stamblerre added this to To Do in gopls on-deck Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants