Skip to content

Commit c682fc3

Browse files
authored
Refactor sentry logic and fix bug (#455)
## Summary - Refactor sentry logic into it's own function - Ensure we do not call sentry when the `runErr` is `nil`. ## How was it tested? I didn't ... I'm thinking it's a small refactor, but let me know if you think I should test it more before landing.
1 parent daea5cc commit c682fc3

File tree

1 file changed

+55
-40
lines changed

1 file changed

+55
-40
lines changed

internal/boxcli/midcobra/telemetry.go

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -65,52 +65,28 @@ func (m *telemetryMiddleware) postRun(cmd *cobra.Command, args []string, runErr
6565
return
6666
}
6767

68-
segmentClient, _ := segment.NewWithConfig(m.opts.TelemetryKey, segment.Config{
69-
BatchSize: 1, /* no batching */
70-
// Discard logs:
71-
Logger: segment.StdLogger(log.New(io.Discard, "" /* prefix */, 0)),
72-
Verbose: false,
73-
})
74-
75-
defer func() {
76-
_ = segmentClient.Close()
77-
}()
78-
7968
subcmd, subargs, parseErr := getSubcommand(cmd, args)
8069
if parseErr != nil {
8170
return // Ignore invalid commands
8271
}
8372

8473
pkgs := getPackages(cmd)
85-
86-
sentry.ConfigureScope(func(scope *sentry.Scope) {
87-
scope.SetTag("command", subcmd.CommandPath())
88-
scope.SetContext("command", map[string]interface{}{
89-
"subcommand": subcmd.CommandPath(),
90-
"args": subargs,
91-
"packages": pkgs,
92-
})
93-
})
94-
// verified with manual testing that the sentryID returned by CaptureException
95-
// is the same as m.ExecutionID, since we set EventID = m.ExecutionID in sentry.Init
96-
sentry.CaptureException(runErr)
97-
var sentryEventID string
98-
if runErr != nil {
99-
sentryEventID = m.executionID
74+
evt := &event{
75+
AppName: m.opts.AppName,
76+
AppVersion: m.opts.AppVersion,
77+
Command: subcmd.CommandPath(),
78+
CommandArgs: subargs,
79+
CommandError: runErr,
80+
DeviceID: telemetry.DeviceID(),
81+
Duration: time.Since(m.startTime),
82+
Failed: runErr != nil,
83+
Packages: pkgs,
84+
Shell: os.Getenv("SHELL"),
10085
}
10186

102-
trackEvent(segmentClient, &event{
103-
AppName: m.opts.AppName,
104-
AppVersion: m.opts.AppVersion,
105-
Command: subcmd.CommandPath(),
106-
CommandArgs: subargs,
107-
DeviceID: telemetry.DeviceID(),
108-
Duration: time.Since(m.startTime),
109-
Failed: runErr != nil,
110-
Packages: pkgs,
111-
SentryEventID: sentryEventID,
112-
Shell: os.Getenv("SHELL"),
113-
})
87+
m.trackError(evt) // Sentry
88+
89+
m.trackEvent(evt) // Segment
11490
}
11591

11692
func (m *telemetryMiddleware) withExecutionID(execID string) Middleware {
@@ -147,11 +123,29 @@ func getPackages(c *cobra.Command) []string {
147123
return box.Config().Packages
148124
}
149125

126+
func (m *telemetryMiddleware) trackError(evt *event) {
127+
if evt == nil || evt.CommandError == nil {
128+
// Don't send anything to sentry if the error is nil.
129+
return
130+
}
131+
132+
sentry.ConfigureScope(func(scope *sentry.Scope) {
133+
scope.SetTag("command", evt.Command)
134+
scope.SetContext("command", map[string]interface{}{
135+
"subcommand": evt.Command,
136+
"args": evt.CommandArgs,
137+
"packages": evt.Packages,
138+
})
139+
})
140+
sentry.CaptureException(evt.CommandError)
141+
}
142+
150143
type event struct {
151144
AppName string
152145
AppVersion string
153146
Command string
154147
CommandArgs []string
148+
CommandError error
155149
DeviceID string
156150
Duration time.Duration
157151
Failed bool
@@ -160,8 +154,29 @@ type event struct {
160154
Shell string
161155
}
162156

163-
func trackEvent(client segment.Client, evt *event) {
164-
_ = client.Enqueue(segment.Track{ // Ignore errors, telemetry is best effort
157+
func (m *telemetryMiddleware) trackEvent(evt *event) {
158+
if evt == nil {
159+
return
160+
}
161+
162+
if evt.CommandError != nil {
163+
// verified with manual testing that the sentryID returned by CaptureException
164+
// is the same as m.ExecutionID, since we set EventID = m.ExecutionID in sentry.Init
165+
evt.SentryEventID = m.executionID
166+
}
167+
168+
segmentClient, _ := segment.NewWithConfig(m.opts.TelemetryKey, segment.Config{
169+
BatchSize: 1, /* no batching */
170+
// Discard logs:
171+
Logger: segment.StdLogger(log.New(io.Discard, "" /* prefix */, 0)),
172+
Verbose: false,
173+
})
174+
175+
defer func() {
176+
_ = segmentClient.Close()
177+
}()
178+
179+
_ = segmentClient.Enqueue(segment.Track{ // Ignore errors, telemetry is best effort
165180
AnonymousId: evt.DeviceID, // Use device id instead
166181
Event: fmt.Sprintf("[%s] Command: %s", evt.AppName, evt.Command),
167182
Context: &segment.Context{

0 commit comments

Comments
 (0)