telemetry: report segment events asynchronously #1222
Conversation
Instead of blocking the process from exiting, buffer Segment events to disk and start another process to upload them. With this change all telemetry should be non-blocking. Most of the Segment event logic now lives in the telemetry package instead of the cobra middleware. The telemetry package API now looks something like: ```go type EventName int const ( EventCommandSuccess EventName = iota EventShellInteractive EventShellReady ) func Error(err error, meta Metadata) func Event(e EventName, meta Metadata) func Start() func Stop() func Upload() ``` Fixes #1003.
|
FWIW, this completely breaks in my env :) I wanted to test this because the telemetry is completely killing me. You never knew how much a couple 100ms can drive you nuts. Anyhow.... This hangs forever I'm debugging why. It's odd because if I run things manually it seems all happy. To be continued.... |
|
This seems suspicious... I have a bunch of |
|
And this commit definitely breaks it. I just tested if this is a just a regression else so I rebased on main. |
|
Ok, it's definitely that. The |
The background process that uploads telemetry was hanging when the telemetry keys weren't set. This was due to a goroutine returning before calling `WaitGroup.Done`. Add a defer for the `WaitGroup.Done` call and add a new goroutine that calls `os.Exit` if the process doesn't exit after 5 seconds.
|
@ibuildthecloud thanks for catching that. It turns out this was happening because a goroutine was returning without calling |
|
Cool this works now (sorta/maybe). I just now realized that dev build probably don't have telemetry enabled so I don't really know if this is now faster. |
Instead of blocking the process from exiting, buffer Segment events to disk and start another process to upload them. With this change all telemetry should be non-blocking.
Most of the Segment event logic now lives in the telemetry package instead of the cobra middleware. A bunch of the unused logic around tracking CLI SSH sessions has also been removed. The telemetry package API now looks something like:
Fixes #1003.