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.

Sign up for GitHub

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

Closed
8 tasks done
findleyr opened this issue Feb 7, 2024 · 9 comments
Closed
8 tasks done

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

findleyr opened this issue Feb 7, 2024 · 9 comments
Assignees
@matloob
Labels
GoCommand cmd/go release-blocker telemetry x/telemetry issues
Milestone
Go1.23

Comments

@findleyr
Copy link
Contributor

findleyr commented Feb 7, 2024 •
edited
Loading

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
Contributor

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

@gopherbot
Copy link
Contributor

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
@matloob
cmd: update golang.org/x/telemetry and its dependencies
c2de291 
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
@matloob
cmd/go: add telemetry counters for flag names and subcommand
58fea5d 
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
@matloob @ezz-no
cmd: update golang.org/x/telemetry and its dependencies
acd7d49 
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
@matloob @ezz-no
cmd/go: add telemetry counters for flag names and subcommand
5b3460b 
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).

@cherrymui
Copy link
Member

@matloob this is a release blocker. Any update on the counters? Thanks.

@matloob
Copy link
Contributor

matloob commented May 8, 2024

Hi we plan to have counters flowing onto the website and a collection proposal out by the end of the week.
I have a CL out to add counters to the compiler and will soon send out CLs for the other tools.

@cagedmantis
Copy link
Contributor

@matloob Is there is status update on this issue? This came up in the release meeting.

@findleyr
Copy link
Contributor Author

I checked the last box, since we have observed counters.

@matloob I think this can be closed.

@matloob
Copy link
Contributor

matloob commented May 22, 2024

Closing.

@matloob matloob closed this as completed May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matloob matloob

Labels
GoCommand cmd/go release-blocker telemetry x/telemetry issues
Projects
None yet
Milestone
Go1.23
Development

No branches or pull requests
6 participants
@cagedmantis @bcmills @gopherbot @cherrymui @matloob @findleyr