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/config: initial cmd/go and cmd/compile counters #67244
Comments
This LGTM, pending resolution of certain technical details that don't affect the overall proposal (the 'version' string is likely meaningless for programs in cmd). |
Change https://go.dev/cl/585198 mentions this issue: |
This does indeed require additional support in x/telemetry. I believe https://go.dev/cl/585198 should allow this CL to work, at which point you should probably also add a minimum version to each counter, by adding |
Main module version information is not stamped in toolchain binaries, and therefore we do not have a program version for toolchain programs. Rather than add special handling for toolchain programs in many places, to handle the missing version, instead enforce the convention that for toolchain programs, program version == go version. Specifically: - Add internal/telemetry.{IsToolchainProgram,ProgramInfo}, to share the logic for interrogating program information. - Update ProgramInfo to reuse the go version as program version for toolchain programs. - Update config generation to reuse the go version list as program version list, for toolchain programs. - Use the go/version package rather than copying it, now that it is available. This means that the config generator requires go1.22, which should be fine as it is only run by developers or RelUI. - Update config generation and validation to use go/version package rather than semver package for working with toolchain program versions. - Move chartconfig.Validate to configgen.ValidateChartConfig, since validation now requires the go/version package and requires go1.22 (chartconfig is linked by more packages). For golang/go#67244 Change-Id: I226b18d37e1ad973002b19afa8afdb8cf97b358c Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/585198 Auto-Submit: Robert Findley <rfindley@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change https://go.dev/cl/585376 mentions this issue: |
A subsequent CL will fix the parsing/validation to ensure that this would have been caught. For now, let's unbreak the config. Updates golang/go#67244 Change-Id: I0a867cd7f4547085e6d027f486a069ff8a4e2c4f Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/585376 Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Robert Findley <rfindley@google.com>
This is now done. |
Summary
We propose to add counters for cmd/go and cmd/compile invocations (that would be the "denominatior" when trying to determine percentage usage), and for cmd/compile stacks when there's a fatal error, and for the Go command's buildmode flag.
Proposed Config Change
https://golang.org/cl/584015
The text was updated successfully, but these errors were encountered: