From 0f79c7e7a1e23598acaf0cf9a07933bfe8d3b049 Mon Sep 17 00:00:00 2001
From: Mike Landau <mikeland86@gmail.com>
Date: Fri, 4 Oct 2024 13:23:56 -0700
Subject: [PATCH] [logging] Fix segment logging

---
 internal/boxcli/midcobra/telemetry.go | 8 +-------
 internal/boxcli/usererr/usererr.go    | 7 ++++++-
 internal/telemetry/telemetry.go       | 4 +++-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/internal/boxcli/midcobra/telemetry.go b/internal/boxcli/midcobra/telemetry.go
index 45aa2b7538b..5d94d695bc6 100644
--- a/internal/boxcli/midcobra/telemetry.go
+++ b/internal/boxcli/midcobra/telemetry.go
@@ -8,11 +8,9 @@ import (
 	"runtime/trace"
 	"sort"
 
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"go.jetpack.io/devbox/internal/boxcli/featureflag"
-	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/devbox"
 	"go.jetpack.io/devbox/internal/devbox/devopt"
 	"go.jetpack.io/devbox/internal/envir"
@@ -42,11 +40,6 @@ func (m *telemetryMiddleware) postRun(cmd *cobra.Command, args []string, runErr
 	defer trace.StartRegion(cmd.Context(), "telemetryPostRun").End()
 	defer telemetry.Stop()
 
-	var userExecErr *usererr.ExitError
-	if errors.As(runErr, &userExecErr) || !usererr.ShouldLogError(runErr) {
-		return
-	}
-
 	meta := telemetry.Metadata{
 		FeatureFlags: featureflag.All(),
 		CloudRegion:  os.Getenv(envir.DevboxRegion),
@@ -68,6 +61,7 @@ func (m *telemetryMiddleware) postRun(cmd *cobra.Command, args []string, runErr
 
 	if runErr != nil {
 		telemetry.Error(runErr, meta)
+		// TODO: This is skipping event logging of calls that end in error. We probably want to log them.
 		return
 	}
 	telemetry.Event(telemetry.EventCommandSuccess, meta)
diff --git a/internal/boxcli/usererr/usererr.go b/internal/boxcli/usererr/usererr.go
index dcc76e5b28d..0e7cd94c1fd 100644
--- a/internal/boxcli/usererr/usererr.go
+++ b/internal/boxcli/usererr/usererr.go
@@ -83,11 +83,16 @@ func Extract(err error) (error, bool) { // nolint: revive
 	return nil, false
 }
 
-// ShouldLogError returns true if the it's a logged user error or is a non-user error
+// ShouldLogError returns true if the it's a combined error specifically marked to be logged
+// or if it's not an ExitError.
 func ShouldLogError(err error) bool {
 	if err == nil {
 		return false
 	}
+	var userExecErr *ExitError
+	if errors.As(err, &userExecErr) {
+		return false
+	}
 	c := &combined{}
 	if errors.As(err, &c) {
 		return c.logged
diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
index af04e09863f..5720aa8d51b 100644
--- a/internal/telemetry/telemetry.go
+++ b/internal/telemetry/telemetry.go
@@ -24,6 +24,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
 	segment "github.com/segmentio/analytics-go"
+	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/devbox/providers/identity"
 	"go.jetpack.io/devbox/internal/nix"
 
@@ -138,7 +139,8 @@ func commandEvent(meta Metadata) (id string, msg *segment.Track) {
 // Error reports an error to the telemetry server.
 func Error(err error, meta Metadata) {
 	errToLog := err // use errToLog to avoid shadowing err later. Use err to keep API clean.
-	if !started || errToLog == nil {
+
+	if !started || !usererr.ShouldLogError(errToLog) {
 		return
 	}