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, cmd/go: umbrella issue for cmd/go integration #65586

Open
6 of 8 tasks
findleyr opened this issue Feb 7, 2024 · 4 comments
Open
6 of 8 tasks

x/telemetry, cmd/go: umbrella issue for cmd/go integration #65586

findleyr opened this issue Feb 7, 2024 · 4 comments
Assignees
Labels
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Feb 7, 2024

This is an umbrella issue tracking work to integrate opt-in telemetry with cmd/go for Go 1.23. This augments #58894 with specific items necessary for cmd/go.

Here are some high level steps:

  • Vendor x/telemetry and call counter.Open (done in https://go.dev/cl/559199)
  • Add at least one counter (e.g. flag usage). (https://go.dev/cl/559519, reverted due to windows/386 test failure in x/telemetry: crashes on windows-386 #65447)
  • File at least one public telemetry proposal for collection of a Go command counter.
  • Observe counters being reported to telemetry.go.dev. This will happen even before the next step, from users that run gopls.
  • Design a mechanism for lighter-weight uploading (e.g. x/telemetry: consolidate initialization into telemetry.Start #65500, though that need not be the solution). Necessary features:
    • Rate limit uploading. E.g. only one process per day calls upload.Run.
    • Daemonize uploading. Uploads should be detached from short-lived Go command invocations.
    • Decide when uploading should be attempted. For example, we could only attempt during go build, or use a heuristic such as a 1s delay from process start.

CC @golang/telemetry

@gopherbot gopherbot added the telemetry x/telemetry issues label Feb 7, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 7, 2024
@findleyr findleyr modified the milestones: Unreleased, Go1.23 Feb 7, 2024
@gopherbot
Copy link

Change https://go.dev/cl/562715 mentions this issue: cmd: update golang.org/x/telemetry and its dependencies

@gopherbot
Copy link

Change https://go.dev/cl/562735 mentions this issue: cmd/go: add telemetry counters for flag names and subcommand

gopherbot pushed a commit that referenced this issue Feb 8, 2024
This change was produced with
	go get golang.org/x/telemetry@latest
	go mod tidy
	go mod vendor

For #65586,#58894

Change-Id: I631a424ebb726fb0999d4b5d1e6e7a288b475344
Cq-Include-Trybots: luci.golang.try:gotip-windows-386,gotip-windows-amd64-longtest,gotip-solaris-amd64,gotip-openbsd-amd64,gotip-wasip1-wasm_wazero,gotip-js-wasm
Reviewed-on: https://go-review.googlesource.com/c/go/+/562715
TryBot-Bypass: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
Commit-Queue: Michael Matloob <matloob@golang.org>
gopherbot pushed a commit that referenced this issue Feb 8, 2024
For #58894,#65586

This is a revert of CL 560655 which was a revert of CL 559519.
CL 559519 was reverted because it was broken on windows/386. But now
CL 562715 pulls in x/telemetry CL 560462 which disables telemetry on
windows/386, fixing that issue.

Change-Id: I094e90c28bca02f2303807d3b008f2ef9d59433c
Reviewed-on: https://go-review.googlesource.com/c/go/+/562735
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@bcmills bcmills added the GoCommand cmd/go label Feb 16, 2024
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
This change was produced with
	go get golang.org/x/telemetry@latest
	go mod tidy
	go mod vendor

For golang#65586,golang#58894

Change-Id: I631a424ebb726fb0999d4b5d1e6e7a288b475344
Cq-Include-Trybots: luci.golang.try:gotip-windows-386,gotip-windows-amd64-longtest,gotip-solaris-amd64,gotip-openbsd-amd64,gotip-wasip1-wasm_wazero,gotip-js-wasm
Reviewed-on: https://go-review.googlesource.com/c/go/+/562715
TryBot-Bypass: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
Commit-Queue: Michael Matloob <matloob@golang.org>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
For golang#58894,golang#65586

This is a revert of CL 560655 which was a revert of CL 559519.
CL 559519 was reverted because it was broken on windows/386. But now
CL 562715 pulls in x/telemetry CL 560462 which disables telemetry on
windows/386, fixing that issue.

Change-Id: I094e90c28bca02f2303807d3b008f2ef9d59433c
Reviewed-on: https://go-review.googlesource.com/c/go/+/562735
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@cagedmantis
Copy link
Contributor

Is there any status update to report on this issue?

@matloob
Copy link
Contributor

matloob commented Apr 11, 2024

We've gotten counters into the go command and are working on integrating them into other commands. We have a telemetry counter config proposal out (#66210).

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

No branches or pull requests

5 participants