x/tools/gopls: replace gopls.add_telemetry with a hidden `gopls add_telemetry` command

[hyangah]

We added gopls.add_telemetry to allow vscode-go to collect editor metrics through gopls. However, this LSP command works only gopls is up and running, which limits its utility. Instead, I propose to have a gopls subcommand, and vscode-go invokes that as a separate process. That will allow vscode-go to collect telemetry even when the language server is disabled.

[gopherbot]

Change https://go.dev/cl/546419 mentions this issue: gopls/internal/cmd: add 'execute' command

[hyangah]

Thank you for the CL, but as I said in the meeting, I want to have a light-weight hidden command for this and discuss general gopls execute in a separate issue. This issue also asks to remove the gopls.add_telemetry LSP command. What I am thinking is just set up a line scanner on stdin, parses name, count, and call the telemetry counter.Inc call. No JSON encoding/decoding or LSP dependency.

I am still not sure about gopls execute subcommand. Who are the target users and when they want to use the interface designed for LSP outside LSP session?

• Commands like gopls.check_upgrades, gopls.run_vulncheck, gopls.toggle_gc_details, gopls.reset_go_mod_diagnostics are used to trigger diagnostics or turn off diagnostics. Without capturing the followup diagnostics messages, exposing them is not doing any useful work.

• Commands like gopls.vendor, gopls.generate, gopls.run_tests, gopls.run_go_work, gopls.tidy, gopls.list_imports, gopls.edit_go_directive have a better and easier-to-use alternative, go.

• Commands like gopls.start_profile, gopls.stop_profile, gopls.start_debugging, gopls.mem_stats aren't useful outside server mode.

• Commands like gopls.apply_fix, gopls.fetch_vulncheck_results aren't working without prior diagnostics.

• Commands like gopls.change_signature or other refactoring commands may be useful with better command line interface. (e.g. should users provide UTF16 position info?)

Most commands are designed to be used along with Code Lens, Code Action, and Diagnostics. That allowed us to change API of the custom commands (e.g. we can change gopls.tidy arg type or even the command name without worrying about backwards/forwards compatibility). We made a few exceptions for simple commands, but they are mostly experimental or hidden for advanced usage. I am afraid that we are moving the bar by exposing them through CLI used by human directly.

[adonovan]

The gopls CLI is no more supported or stable an interface than the various commands supported through the ExecuteCommand service method, so I'm not worried about increasing exposure through the CLI tool. You're right that many of the commands are low-level, internal, or redundant with other features, but they are often the only the way (short of writing code) to test an LSP-level feature in isolation while trying to minimize a reproducible test case for a user-reported issue---that's the main reason I've been adding commands recently, and I have wanted this 'gopls execute' on several occasions.

At some point we should think about designing the command-line interface that we really want and committing to support it, because gopls is currently the only way for Go users to script various operations (such as renaming) that used to have a dedicated command-line tool. But that's not on our plans yet. When that happens, any existing command that we don't want to publish can be pushed beneath an "experimental" subcommand.

[hyangah]

It looks like gopls execute deserves a separate discussion. Is there a reason to keep gopls.add_telemetry LSP command? Strictly speaking, the proxying capability doesn't need to be in gopls and it's not gopls's core functionality. We are currently adding this to gopls just because we don't want users to install another binary and gopls already has dependency on x/telemetry/counter.

[findleyr]

Are there any counters that VS Code would want to update with high frequency? I.e., is there any performance advantage for an LSP command? I understand not now, but perhaps in the future?

As described, the advantage of an add_telemetry command is that it need not involve any session initialization.

I'm fine with both execute and add_telemetry. However, perhaps they both should be hidden from the help text, as neither are expected to be useful for users. Their presence is confusing.

[adonovan]

The 'gopls execute' command now exists and could be used for this purpose, but in discussion with @hyangah today she was inclined towards a Go-based helper program in the vscode-go project that would include any features that need to be implemented in Go (such as telemetry) but have nothing to do with LSP or gopls. That also seems like reasonable solution.