[sshshim] add sentry without using middleware (#366)

## Summary

This PR is an alternative to both #361 and #363.

This PR's approach is lighterweight:
1. refactor sentry code into its new `telemetry` package.
2. directly call that in the sshshim command execution function.

The advantages it has are:
1. Major: The implementation in #363 is rather gross because we need to
introduce an interface equivalent of cobra.Command. This is really ugly,
and not very maintainable. If new middleware uses other functions of
cobra commands, then we'll have to add those to the interface and also
handle them in the sshshim-command version.
2. Minor: We keep the `midcobra.telemetry` middleware for the regular
devbox command.

Disadvantages are:
1. We may in the future need to incorporate new middleware into the
sshshim command in a non-middleware manner.

## How was it tested?

compiles.

Tested via:
1. Look up sentryDSN in sentry dashboard > Settings > Project (devbox) >
ClientKeys (DSN)
2. Inserted that in `internal/build/build.go`
3. Manually inserted errors in devbox.Open and invokeSSHOrSCPCommand.
Ran devbox commands that triggered them.
4. verified in sentry dashboard that the errors were logged.

Author: savil

internal/boxcli/midcobra/midcobra.go

@@ -25,7 +25,7 @@ type Middleware interface {
 func New(cmd *cobra.Command) Executable {
 	return &midcobraExecutable{
 		cmd:         cmd,
-		executionID: executionID(),
+		executionID: ExecutionID(),
 		middlewares: []Middleware{},
 	}
 }
@@ -75,7 +75,7 @@ func (ex *midcobraExecutable) Execute(ctx context.Context, args []string) int {
 	}
 }
 
-func executionID() string {
+func ExecutionID() string {
 	// google/uuid package's String() returns a value of the form:
 	// xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
 	//

internal/boxcli/midcobra/telemetry.go

@@ -9,14 +9,13 @@ import (
 	"log"
 	"os"
 	"runtime"
-	"strconv"
 	"time"
 
 	"github.com/denisbrodbeck/machineid"
-	"github.com/getsentry/sentry-go"
 	segment "github.com/segmentio/analytics-go"
 	"github.com/spf13/cobra"
 	"go.jetpack.io/devbox"
+	"go.jetpack.io/devbox/internal/telemetry"
 )
 
 // We collect some light telemetry to be able to improve devbox over time.
@@ -26,14 +25,10 @@ import (
 // 2. Data is only stored in SOC 2 compliant systems, and we are SOC 2 compliant ourselves.
 // 3. Users should always have the ability to opt-out.
 func Telemetry(opts *TelemetryOpts) Middleware {
-	doNotTrack, err := strconv.ParseBool(os.Getenv("DO_NOT_TRACK")) // https://consoledonottrack.com/
-	if err != nil {
-		doNotTrack = false
-	}
 
 	return &telemetryMiddleware{
 		opts:     *opts,
-		disabled: doNotTrack || opts.TelemetryKey == "" || opts.SentryDSN == "",
+		disabled: telemetry.DoNotTrack() || opts.TelemetryKey == "" || opts.SentryDSN == "",
 	}
 }
 
@@ -43,6 +38,7 @@ type TelemetryOpts struct {
 	SentryDSN    string // used by error reporting
 	TelemetryKey string
 }
+
 type telemetryMiddleware struct {
 	// Setup:
 	opts     TelemetryOpts
@@ -65,7 +61,9 @@ func (m *telemetryMiddleware) postRun(cmd *cobra.Command, args []string, runErr
 	if m.disabled {
 		return
 	}
-	initSentry(m.opts, m.executionID)
+
+	sentry := telemetry.NewSentry(m.opts.SentryDSN)
+	sentry.Init(m.opts.AppName, m.opts.AppVersion, m.executionID)
 	segmentClient, _ := segment.NewWithConfig(m.opts.TelemetryKey, segment.Config{
 		BatchSize: 1, /* no batching */
 		// Discard logs:
@@ -82,13 +80,12 @@ func (m *telemetryMiddleware) postRun(cmd *cobra.Command, args []string, runErr
 		return // Ignore invalid commands
 	}
 
+	// verified with manual testing that the sentryID returned by CaptureException
+	// is the same as m.ExecutionID, since we set EventID = m.ExecutionID in sentry.Init
+	sentry.CaptureException(runErr)
 	var sentryEventID string
 	if runErr != nil {
-		defer sentry.Flush(2 * time.Second)
-		_ /*eventIDPointer*/ = sentry.CaptureException(runErr)
 		sentryEventID = m.executionID
-		// verified with manual testing that the sentryID returned by CaptureException
-		// is the same as m.executionID, since we set EventID = m.executionID in initSentry()
 	}
 
 	trackEvent(segmentClient, &event{
@@ -109,32 +106,6 @@ func (m *telemetryMiddleware) withExecutionID(execID string) Middleware {
 	return m
 }
 
-func initSentry(opts TelemetryOpts, executionID string) {
-	sentrySyncTransport := sentry.NewHTTPSyncTransport()
-	sentrySyncTransport.Timeout = time.Second * 2
-	release := opts.AppName + "@" + opts.AppVersion
-	environment := "production"
-	if opts.AppVersion == "0.0.0-dev" {
-		environment = "development"
-	}
-
-	_ = sentry.Init(sentry.ClientOptions{
-		Dsn:              opts.SentryDSN,
-		Environment:      environment,
-		Release:          release,
-		Transport:        sentrySyncTransport,
-		TracesSampleRate: 1,
-		BeforeSend: func(event *sentry.Event, hint *sentry.EventHint) *sentry.Event {
-			for i := range event.Exception {
-				// edit in place and remove error message from tracking
-				event.Exception[i].Value = ""
-			}
-			event.EventID = sentry.EventID(executionID)
-			return event
-		},
-	})
-}
-
 func deviceID() string {
 	salt := "64ee464f-9450-4b14-8d9c-014c0012ac1a"
 	hashedID, _ := machineid.ProtectedID(salt) // Ensure machine id is hashed and non-identifiable

internal/boxcli/root.go

@@ -5,7 +5,6 @@ package boxcli
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"strings"
 
@@ -61,31 +60,10 @@ func Execute(ctx context.Context, args []string) int {
 	return exe.Execute(ctx, args)
 }
 
-// TODO savil. Add Sentry and other monitoring.
-func executeSSH() int {
-	sshshim.EnableDebug() // Always enable for now.
-	debug.Log("os.Args: %v", os.Args)
-
-	if alive, err := sshshim.EnsureLiveVMOrTerminateMutagenSessions(os.Args[1:]); err != nil {
-		debug.Log("EnsureLiveVMOrTerminateMutagenSessions error: %v", err)
-		fmt.Fprintf(os.Stderr, "%v", err)
-		return 1
-	} else if !alive {
-		return 0
-	}
-
-	if err := sshshim.InvokeSSHOrSCPCommand(os.Args); err != nil {
-		debug.Log("InvokeSSHorSCPCommand error: %v", err)
-		fmt.Fprintf(os.Stderr, "%v", err)
-		return 1
-	}
-	return 0
-}
-
 func Main() {
 	if strings.HasSuffix(os.Args[0], "ssh") ||
 		strings.HasSuffix(os.Args[0], "scp") {
-		code := executeSSH()
+		code := sshshim.Execute(context.Background(), os.Args)
 		os.Exit(code)
 	}
 	code := Execute(context.Background(), os.Args[1:])

internal/cloud/openssh/sshshim/command.go

@@ -0,0 +1,52 @@
+package sshshim
+
+import (
+	"context"
+	"fmt"
+	"os"
+
+	"go.jetpack.io/devbox/internal/boxcli/midcobra"
+	"go.jetpack.io/devbox/internal/build"
+	"go.jetpack.io/devbox/internal/debug"
+	"go.jetpack.io/devbox/internal/telemetry"
+)
+
+func Execute(ctx context.Context, args []string) int {
+	defer debug.Recover()
+
+	err := execute(args)
+
+	logSentry(err)
+
+	if err != nil {
+		return 1
+	}
+	return 0
+}
+
+func execute(args []string) error {
+	EnableDebug() // Always enable for now.
+	debug.Log("os.Args: %v", args)
+
+	if alive, err := EnsureLiveVMOrTerminateMutagenSessions(args[1:]); err != nil {
+		debug.Log("ensureLiveVMOrTerminateMutagenSessions error: %v", err)
+		fmt.Fprintf(os.Stderr, "%v", err)
+		return err
+	} else if !alive {
+		return nil
+	}
+
+	if err := InvokeSSHOrSCPCommand(args); err != nil {
+		debug.Log("InvokeSSHorSCPCommand error: %v", err)
+		fmt.Fprintf(os.Stderr, "%v", err)
+		return err
+	}
+	return nil
+}
+
+func logSentry(runErr error) {
+	const appName = "devbox-sshshim"
+	s := telemetry.NewSentry(build.SentryDSN)
+	s.Init(appName, build.Version, midcobra.ExecutionID())
+	s.CaptureException(runErr)
+}

internal/telemetry/sentry.go

@@ -0,0 +1,64 @@
+package telemetry
+
+import (
+	"time"
+
+	"github.com/getsentry/sentry-go"
+)
+
+type Sentry struct {
+	disabled bool
+
+	dsn string
+}
+
+func NewSentry(dsn string) *Sentry {
+	return &Sentry{
+		disabled: DoNotTrack() || dsn == "",
+		dsn:      dsn,
+	}
+}
+
+func (s *Sentry) Init(appName, appVersion, executionID string) {
+	if s.disabled {
+		return
+	}
+
+	sentrySyncTransport := sentry.NewHTTPSyncTransport()
+	sentrySyncTransport.Timeout = time.Second * 2
+	release := appName + "@" + appVersion
+	environment := "production"
+	if appVersion == "0.0.0-dev" {
+		environment = "development"
+	}
+
+	_ = sentry.Init(sentry.ClientOptions{
+		Dsn:              s.dsn,
+		Environment:      environment,
+		Release:          release,
+		Transport:        sentrySyncTransport,
+		TracesSampleRate: 1,
+		BeforeSend: func(event *sentry.Event, hint *sentry.EventHint) *sentry.Event {
+			for i := range event.Exception {
+				// edit in place and remove error message from tracking
+				event.Exception[i].Value = ""
+			}
+			event.EventID = sentry.EventID(executionID)
+			return event
+		},
+	})
+}
+
+// CaptureException
+func (s *Sentry) CaptureException(runErr error) string {
+	if s.disabled || runErr == nil {
+		return ""
+	}
+	defer sentry.Flush(2 * time.Second)
+
+	eventIDPointer := sentry.CaptureException(runErr)
+	if eventIDPointer == nil {
+		return ""
+	}
+	return string(*eventIDPointer)
+}

internal/telemetry/telemetry.go

@@ -0,0 +1,15 @@
+package telemetry
+
+import (
+	"os"
+	"strconv"
+)
+
+func DoNotTrack() bool {
+	// https://consoledonottrack.com/
+	doNotTrack_, err := strconv.ParseBool(os.Getenv("DO_NOT_TRACK"))
+	if err != nil {
+		doNotTrack_ = false
+	}
+	return doNotTrack_
+}