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/telemetry: expand the scope of Go Telemetry to include Delve #68384

Closed
hyangah opened this issue Jul 11, 2024 · 7 comments
Closed

x/telemetry: expand the scope of Go Telemetry to include Delve #68384

hyangah opened this issue Jul 11, 2024 · 7 comments

Comments

@hyangah
Copy link
Contributor

hyangah commented Jul 11, 2024

Proposal Details

Delve, the Go debugger, is important for providing robust debugging capabilities for Go users. It is deeply integrated with the Go team-maintained VS Code Go extension. However, Delve is currently excluded from Go telemetry, which limits insights into debug feature usage and trends.

Proposal

Expand Go telemetry to include Delve, enabling valuable insights into debugging behavior within VS Code Go and beyond, ultimately improving the user debugging experience. This involves

  1. Updating the Go telemetry documentation to reflect the inclusion of Delve.

Current

Go telemetry is a way for Go toolchain programs to collect data about their performance and usage. Here “Go toolchain” means developer tools maintained by the Go team, including the go command and supplemental tools such as the Go language server gopls or Go security tool govulncheck. Go telemetry is only intended for use in programs maintained by the Go team. …

Proposed update

Go telemetry is a way for Go toolchain programs to collect data about their performance and usage. Here “Go toolchain” means developer tools maintained by the Go team, including the go command and supplemental tools such as the Go language server gopls, the Go security tool govulncheck, or the Visual Studio Go extension vscode-go and other tools used by vscode-go. Go telemetry is only intended for use in programs maintained by the Go team and their selected dependencies like delve. ….

  1. Adding telemetry in Delve using the golang.org/x/telemetry/counter API. The API will record the measured counters to the files on the local computer. The Delve team decides what and how to measure. The Go team and other Delve integrators can propose addition of new metrics by filing feature requests in the delve issue tracker.

  2. Adding Delve’s counters in the telemetry upload config, following the official telemetry proposal process. Once added, the Go command will find the counters from the files on the local computer and upload them to telemetry.go.dev.

Example metrics

  • Usage by GOOS/GOARCH, Go/Delve versions
  • Usage by Delve subcommands, DAP mode, RPC versions, flags
  • Performance metrics (latency)
  • Crash monitor (This needs dependency on the golang.org/x/telemetry API)

Alternatives considered

  • Monitoring from VS Code Go: while VS Code Go can collect some usage and performance metrics by watching DAP communication, this is insufficient as a standalone solution. Server-side monitoring in Delve is necessary for accurate latency breakdowns, remote debugging metrics, crash counters, and insights into usage by other Delve clients.
  • Building Delve’s own telemetry: the cost and effort required to build and maintain a privacy-preserving telemetry infrastructure for Delve are disproportionate to the benefits for its small team of maintainers.
  • Using other telemetry frameworks: the choice of transparent, privacy-preserving telemetry solution is currently very limited or non-existent.

Rationale

  1. Deep Integration with VS Code Go: VS Code Go relies heavily on Delve for debugging, much like it relies on gopls for language intellisense. We have witnessed the positive impact of telemetry data on improving gopls stability and decision-making. We expect similar benefits for debugging, leading to an enhanced UX around debugging functionalities. And
  2. Collaborative Development: The Go team and the Delve team have a long history of collaboration to improve Go’s debugging capabilities. Expanding telemetry to include Delve would foster this collaboration by providing shared insights into user behavior and pain points.
  3. Better Debugging UX: Delve is not only used by VS Code Go, but also by other editor integrations. Telemetry data from Delve would provide insights that benefit the broader Go community.

cc @derekparker @aarzilli @golang/tools-team @rsc

@gopherbot gopherbot added this to the Proposal milestone Jul 11, 2024
@rsc
Copy link
Contributor

rsc commented Jul 25, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@TheCoreMan
Copy link

TheCoreMan commented Aug 3, 2024

I really like this proposal and if it stays unchanged I think it should still be accepted. Improving the tooling using telemetry is a great idea.

A few improvement suggestions:

  1. I think that pointing out which data points are missing for the Delve team to direct their efforts today will make this proposal a lot more complete. Maybe some relevant metrics are from Delve itself (counters for each operation type, histogram of time duration for each operation, etc.) and some will still need to come from VSCode (opened debugger with keyboard shortcut VS command panel etc.).
  2. Will these metrics work in Goland as well?

@aarzilli
Copy link
Contributor

aarzilli commented Aug 3, 2024

I think that pointing out which data points are missing for the Delve team to direct their efforts today will make this proposal a lot more complete

  • There are a few places in delve where panics and unexpected conditions are turned into internal errors and returned to the user. We don't expect this to actually ever happen, but if it does it's a bug and we would like to know so that we can fix. I think we could use stack counters for that
  • We don't fully implement the expression language, knowing which unimplemented parts the users attempts to use would help us with prioritizing what to implement
  • VSCode-go has, IMO, suboptimal UX for some situations (very many goroutines being displayed, breakpoints being hit concurrently by multiple goroutines, etc) we've told them about this a long time ago, but maybe having data that shows that this does affect real users would make it more presuasive (or maybe we find out that it does not)

Will these metrics work in Goland as well?

Very likely yes. Goland ships its own precompiled version of delve, so they could disable telemetry completely but I don't think they would.

@rsc
Copy link
Contributor

rsc commented Aug 14, 2024

It sounds like the line being proposed is: "Go telemetry is only intended for use in programs maintained by the Go team and their selected dependencies like Delve."

This seems like a reasonable line. And just like we do for other programs, only counters for a specific enumerated set of released Delve versions would be uploaded. So this seems fine.

Do I have that right?

@rsc
Copy link
Contributor

rsc commented Aug 29, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The previous line for what programs we are willing to take telemetry from was "Go telemetry is only intended for use in programs maintained by the Go team."

The proposal is to change this to "... programs maintainted by the Go team and their selected dependencies like Delve."

@rsc
Copy link
Contributor

rsc commented Sep 4, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The previous line for what programs we are willing to take telemetry from was "Go telemetry is only intended for use in programs maintained by the Go team."

The proposal is to change this to "... programs maintainted by the Go team and their selected dependencies like Delve."

@rsc rsc changed the title proposal: x/telemetry: expand the scope of Go Telemetry to include Delve x/telemetry: expand the scope of Go Telemetry to include Delve Sep 4, 2024
@rsc rsc modified the milestones: Proposal, Backlog Sep 4, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/611875 mentions this issue: _content/doc: expand the scope of telemetry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

5 participants