telemetry: add support for uploading
No support for rate limiting yet but want to keep things small here.
This change adds a dependency on x/sync for errgroup.
I've also had to run go mod tidy in the godev module because it has a
replacement instead of depending on a version of this module.
For golang/go#65500
Change-Id: Icbcd8d3d58fb443c14cf21e339baa3edb76fbca1
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/564615
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/cmd/gotelemetry/main.go b/cmd/gotelemetry/main.go
index 58d5def..6538d97 100644
--- a/cmd/gotelemetry/main.go
+++ b/cmd/gotelemetry/main.go
@@ -341,7 +341,7 @@
func runUpload(_ []string) {
upload.Run(&upload.Control{
- Logging: os.Stderr,
+ Logger: os.Stderr,
})
}
diff --git a/go.mod b/go.mod
index 74d5da9..3b9d932 100644
--- a/go.mod
+++ b/go.mod
@@ -4,4 +4,7 @@
require golang.org/x/mod v0.15.0
-require golang.org/x/sys v0.17.0
+require (
+ golang.org/x/sync v0.6.0
+ golang.org/x/sys v0.17.0
+)
diff --git a/go.sum b/go.sum
index b89e429..e33729a 100644
--- a/go.sum
+++ b/go.sum
@@ -1,4 +1,6 @@
golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8=
golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
+golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
+golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y=
golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
diff --git a/godev/go.mod b/godev/go.mod
index 5d55415..cd93047 100644
--- a/godev/go.mod
+++ b/godev/go.mod
@@ -30,7 +30,7 @@
golang.org/x/crypto v0.14.0 // indirect
golang.org/x/net v0.17.0 // indirect
golang.org/x/oauth2 v0.13.0 // indirect
- golang.org/x/sync v0.4.0 // indirect
+ golang.org/x/sync v0.6.0 // indirect
golang.org/x/text v0.13.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
diff --git a/godev/go.sum b/godev/go.sum
index 246be43..0ad42e0 100644
--- a/godev/go.sum
+++ b/godev/go.sum
@@ -106,8 +106,8 @@
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
-golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
+golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
+golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
diff --git a/start.go b/start.go
index 0a05933..01c0dfd 100644
--- a/start.go
+++ b/start.go
@@ -10,8 +10,10 @@
"os"
"os/exec"
+ "golang.org/x/sync/errgroup"
"golang.org/x/telemetry/counter"
"golang.org/x/telemetry/internal/crashmonitor"
+ "golang.org/x/telemetry/upload"
)
// Config controls the behavior of [Start].
@@ -47,11 +49,15 @@
//
// If [Config.Upload] is set, and the user has opted in to telemetry
// uploading, this process may attempt to upload approved counters
-// to telemetry.go.dev. [But: this option doesn't appear to exist yet.]
+// to telemetry.go.dev.
//
-// If [Config.ReportCrashes] is set, Start re-executes the current
-// executable as a child process, in a special mode in which it acts
-// as a crash monitor for the parent process (the application).
+// If [Config.ReportCrashes] is set, any fatal crash will be
+// recorded by incrementing a counter named for the stack of the
+// first running goroutine in the traceback.
+//
+// If either of these flags is set, Start re-executes the current
+// executable as a child process, in a special mode in which it
+// acts as a telemetry sidecar for the parent process (the application).
// In that mode, the call to Start will never return, so Start must
// be called immediately within main, even before such things as
// inspecting the command line. The application should avoid expensive
@@ -59,23 +65,21 @@
// be executed twice (parent and child).
func Start(config Config) {
counter.Open()
- if !config.ReportCrashes || !crashmonitor.Supported() {
- // TODO(matloob): Once support for uploading is added to start,
- // we'll just start the uploader instead of returning here.
- return
- }
- if os.Getenv(telemetryChildVar) != "" {
- child(config.Logger)
- panic("unreachable")
- }
- parent(config.Logger)
+ if (config.ReportCrashes && crashmonitor.Supported()) || config.Upload {
+ if os.Getenv(telemetryChildVar) != "" {
+ child(config)
+ panic("unreachable")
+ }
+
+ parent(config)
+ }
}
const telemetryChildVar = "X_TELEMETRY_CHILD"
-func parent(logw io.Writer) {
- logger := log.New(logw, "", 0)
+func parent(config Config) {
+ logger := log.New(config.Logger, "", 0)
// This process is the application (parent).
// Fork+exec the telemetry child.
exe, err := os.Executable()
@@ -86,19 +90,44 @@
cmd.Env = append(os.Environ(), telemetryChildVar+"=1")
cmd.Stderr = os.Stderr
cmd.Stdout = os.Stderr
- pipe, err := cmd.StdinPipe()
- if err != nil {
- logger.Fatalf("StdinPipe: %v", err)
- }
- crashmonitor.Parent(pipe.(*os.File)) // (this conversion is safe)
+ if config.ReportCrashes {
+ pipe, err := cmd.StdinPipe()
+ if err != nil {
+ logger.Fatalf("StdinPipe: %v", err)
+ }
+
+ crashmonitor.Parent(pipe.(*os.File)) // (this conversion is safe)
+ }
if err := cmd.Start(); err != nil {
logger.Fatalf("can't start telemetry child process: %v", err)
}
}
-func child(logw io.Writer) {
- // TODO(matloob): add support for uploading.
- crashmonitor.Child(logw)
+func child(config Config) {
+ // Start crashmonitoring and uploading depending on what's requested
+ // and wait for the longer running child to complete before exiting:
+ // if we collected a crash before the upload finished, wait for the
+ // upload to finish before exiting
+ var g errgroup.Group
+
+ if config.Upload {
+ g.Go(func() error {
+ uploaderChild(config.Logger)
+ return nil
+ })
+ }
+ if config.ReportCrashes {
+ g.Go(func() error {
+ crashmonitor.Child(config.Logger)
+ return nil
+ })
+ }
+ g.Wait()
+}
+
+func uploaderChild(logger io.Writer) {
+ // TODO(matloob): Do rate-limiting here.
+ upload.Run(&upload.Control{Logger: logger})
}
diff --git a/upload/upload.go b/upload/upload.go
index 0a2b13a..122b725 100644
--- a/upload/upload.go
+++ b/upload/upload.go
@@ -14,8 +14,8 @@
// Run generates and uploads reports, as allowed by the mode file.
// A nil Control is legal.
func Run(c *Control) {
- if c != nil && c.Logging != nil {
- upload.SetLogOutput(c.Logging)
+ if c != nil && c.Logger != nil {
+ upload.SetLogOutput(c.Logger)
}
// ignore error: failed logging should not block uploads
upload.LogIfDebug("")
@@ -32,7 +32,7 @@
// reporting and uploading choices.
// Future versions may also allow the user to set the upload URL.
type Control struct {
- // Logging provides a io.Writer for error messages during uploading
+ // Logger provides a io.Writer for error messages during uploading
// nil is legal and no log messages get generated
- Logging io.Writer
+ Logger io.Writer
}
