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: improve daemon logging behavior #40105

Closed
findleyr opened this issue Jul 7, 2020 · 5 comments
Closed

x/tools/gopls: improve daemon logging behavior #40105

findleyr opened this issue Jul 7, 2020 · 5 comments
Assignees
Labels
Milestone

Comments

@findleyr
Copy link
Contributor

@findleyr findleyr commented Jul 7, 2020

Right now, when gopls is in daemon mode, we by default start the daemon with -logfile=auto. Note that the daemon does not log RPC traces -- only direct calls to log.Printf, of which there are few. RPC trace logging is left to the sidecar, so that RPCs from multiple sessions are not intermingled.

This means that the gopls daemon creates a mostly (or entirely) empty logfile, which causes clutter and can lead to confusion when looking for actual RPC logs. Arguably, we should instead:

  • disable daemon logging by default. It is configurable via -remote.logfile, so can be enabled if necessary
  • log a message at the start of the daemon logs to say something to the effect of "I am a daemon"
  • log from the daemon when sessions connect and disconnect
@gopherbot gopherbot added this to the Unreleased milestone Jul 7, 2020
@findleyr
Copy link
Contributor Author

@findleyr findleyr commented Jul 7, 2020

It would be good to also differentiate the daemon logfile by name (e.g. /tmp/gopls-daemon-<pid>.log).

CC @bhcleek -- I noticed this from a user report using vim-go, in golang/vscode-go#299. It is confusing that the current default vim-go settings results in an empty logfile, most of the time. This is my bad, for making the default logging behavior of the daemon inconsistent with the default logging behavior of the sidecar. I'll fix this soon, but wanted you to be aware: it would be good to get the fix in before the next vim-go release. Thanks for using daemon mode!

@findleyr findleyr self-assigned this Jul 7, 2020
@bhcleek
Copy link
Contributor

@bhcleek bhcleek commented Jul 8, 2020

@findleyr are you asking for vim-go to use --remote.logfile=$TMPDIR/gopls-daemon-$PID.log in its default configuration?

@findleyr
Copy link
Contributor Author

@findleyr findleyr commented Jul 8, 2020

@bhcleek sorry for being unclear. No change needed in vim-go: I'll make this the default behavior for gopls.

@bhcleek
Copy link
Contributor

@bhcleek bhcleek commented Jul 8, 2020

Thanks for clarifying!

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Jul 8, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 8, 2020

Change https://golang.org/cl/241440 mentions this issue: internal/lsp/lsprpc: improvements to daemon logging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.