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: bug in telemetry.Start during config download delegation #67211
Comments
Change https://go.dev/cl/583495 mentions this issue: |
Change https://go.dev/cl/583575 mentions this issue: |
gopherbot
pushed a commit
to golang/telemetry
that referenced
this issue
May 7, 2024
Add an end-to-end test for Start, which exacerbates the integration bug described in golang/go#67211. To achieve this, make the following API changes: - Add telemetry.Config.{UploadStartTime, UploadURL}, to pass through to the upload configuration. - Add a StartResult return for Start, with a single Wait() method. Also fix a bug that regtest.RunProg appeared to accidentally lose the os.Environ when invoking its subprocess. Updates golang/go#67211 Fixes golang/go#66003 Change-Id: Ic9e53742cd96991452a37601fb5454ef2bdc3d1a Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/583495 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopherbot
pushed a commit
to golang/telemetry
that referenced
this issue
May 7, 2024
As described in the code, failing to modify X_TELEMETRY_CHILD can lead to go subcommands acting as the telemetry child themselves, rather than as the go command. On the other hand, we don't want the delegated go commands to *also* fork themselves. Therefore, we establish a convention that X_TELEMETRY_CHILD=1 implies that the process is a telemetry child process, and X_TELEMETRY_CHILD=2 implies that the process is a child of a telemetry child process. For golang/go#67211 Change-Id: Ia6924cc1671f9500f21cc9102cc7d5da33251a83 Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/583575 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Change https://go.dev/cl/584796 mentions this issue: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The telemetry.Start command uses a fork+exec environment variable trick to run the upload and crash monitor in a subprocess.
However, the upload itself uses the Go command to download the configstore: if the magic environment variable is set, this Go command will also think it is a telemetry.Start subprocess. We must unset the environment variable to avoid this broken delegation.
Filing this issue to track both the fix and vendoring the fix into Go 1.23.
The text was updated successfully, but these errors were encountered: