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

Jump to bottom

x/tools/gopls: add a command to start the debug server #45518

Closed
findleyr opened this issue Apr 12, 2021 · 6 comments
Closed

x/tools/gopls: add a command to start the debug server #45518

findleyr opened this issue Apr 12, 2021 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
gopls on-deck
Milestone
gopls/v0.6.11

Comments

@findleyr
Copy link
Contributor

The gopls debug server can be very useful for debugging, but...

  1. It's often not available when the user needs it, because -debug=addr must be passed at the command line when gopls is started.
  2. It's hard to find, because best practice is to bind to localhost:0, and the actual port the server is listening on is buried in the logs.

The reason for (1) is that the debug server exposes the internals of the gopls server, including file contents, and so for privacy on shared machines we shouldn't always bind. The reason for (2) is simply that we have no better way to surface the information: we could use a ShowMessage request, but depending on LSP client those are easy to miss, transient, or annoying.

We propose to solve this by providing a new command (gopls.startDebugging) that starts the debug server if it wasn't already running, and returns its URL. We can integrate this in VS Code via a menu item, and either VS Code can open the returned address, or we can use the new window/showDocument request to ask clients to open it. T.B.D.: my suspicion is that not many clients support window/showDocument.

The hope is that this makes the debug server more commonly available for diagnostic purposes, and therefore makes it worthwhile to add more functionality to it. For example we could always have recent RPC logs in-memory, start profiling on-demand, inspect more internal state - the possibilities are endless.

Special consideration for the gopls daemon: we should ideally start debugging for each server in the serving path, both forwarder(s) and daemon. Each server can intercept the gopls.startDebugging command and add its own information.

CC @heschi @stamblerre @ianthehat
@findleyr findleyr self-assigned this Apr 12, 2021
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 12, 2021
@gopherbot gopherbot added this to the Unreleased milestone Apr 12, 2021
@findleyr findleyr modified the milestones: Unreleased, gopls/v1.0.0 Apr 12, 2021
@findleyr findleyr added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 12, 2021
@ianthehat
Copy link

I assume this means the debugging collector would always be running and collecting the information, it would just not be bound to a serving port until the command was issued?
If so we probably ought to take a hard look at the memory/performance cost (I think it is fine, I have not run without it in a very very long time, but I have also not tested very large repositories or very long up times with it either)
I do really like the idea of easily being able to open the debug pages within the editor, but we probably also ought to prioritize making them more functional and helpful if we are going to encourage their use.

@findleyr
Copy link
Contributor Author

I assume this means the debugging collector would always be running and collecting the information

It looks like it is always running and collecting information:
https://cs.opensource.google/go/x/tools/+/master:internal/lsp/cmd/cmd.go;l=144;drc=b3556f0f832d320fd623493aeed82579d8eecd43
Maybe we enabled this for memory debugging. In any case, I haven't seen a significant performance penalty, though perhaps we should confirm.

Either way, I don't see a reason we couldn't start collection when the debug server is started.

@ianthehat
Copy link

Some of the information it collects required ends to have corresponding starts, it would not surprise me at all if it crashes if you just start it on demand, if we really want to we can make it all work I am sure.
We would also want to re-phrase some of the information, at the moment things like counts are presented as an actual count since gopls started, we probably ought to keep track of when we started collecting and display it somewhere.
In general, I think it would be better to always collect and only serve on demand, it would help with things like when something goes wrong and you want the numbers after the fact (we could attach collected information to bug reports on demand and things)

@findleyr
Copy link
Contributor Author

but we probably also ought to prioritize making them more functional and helpful if we are going to encourage their use

For additional context, this issue came out of a discussion we had about making it easier to help new users troubleshoot. Not having logging enabled or not being able to find logs was a pain point. (actually, this is not just for new users: I often have to use some combination of ps and find to locate the gopls debug logs, because I have multiple gopls instances, multiple neovim instances, and neovim modifies $TMPDIR).

The tentative plan is to spend some effort making the debug server a one-stop shop for debugging resources.

@gopherbot
Copy link

Change https://golang.org/cl/309409 mentions this issue: internal/lsp: add a command to start the debug server

@gopherbot
Copy link

Change https://golang.org/cl/309810 mentions this issue: internal/lsp/cmd: add a command-line command to start daemon debugging

@stamblerre stamblerre added this to To Do in gopls on-deck Apr 15, 2021
@findleyr findleyr mentioned this issue Apr 16, 2021
Open
gopherbot pushed a commit to golang/tools that referenced this issue Apr 26, 2021
@findleyr
internal/lsp: add a command to start the debug server
7c93484 
The utility of the debug server is limited by the requirement to start
gopls with the `-debug` flag and then look in the logs to see which port
the debug server is bound to.

This CL adds a new custom command `gopls.startDebugging` to start the
debug server on demand if it isn't already running, and return its debug
address. It also does some gymnastics to make this turn on debugging for
any intermediate gopls forwarders, when using daemon mode.

For golang/go#45518

Change-Id: I48a90088f96aca54f34f93bedbfe864515320f61
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309409
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls on-deck automation moved this from To Do to Done Apr 27, 2021
@findleyr findleyr mentioned this issue May 4, 2021
Open
@golang golang locked and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
No open projects
gopls on-deck
Done
Milestone
gopls/v0.6.11
Development

No branches or pull requests
4 participants
@ianthehat @stamblerre @gopherbot @findleyr