Repository navigation
perf(telemetry): spool events locally and ship them from a detached process - #42
Merged
Merged
Conversation
…rocess The azure_monitor backend went through azure-monitor-opentelemetry, whose exporter shut down synchronously at process exit. That added ~2 s to every command (0.3 s -> 2.3 s for `bcli endpoint list`), and its Azure VM resource detector printed an IMDS timeout traceback on any machine that isn't an Azure VM. emit() now appends one Application Insights envelope per event to ~/.config/bcli/telemetry/spool.jsonl. flush(), called once at exit, starts a detached `python -m bcli.telemetry._ship` that posts the spool to the connection string's ingestion endpoint with httpx, after the command has returned. Commands run in the same time with telemetry on as with it off. The shipper holds an O_EXCL lock (taken over once stale), claims the spool by renaming it, posts in batches, deletes what Application Insights has answered for, and keeps the unsent remainder on a network error or a retryable status. The spool is capped at 5 MB. Events now arrive as custom events (AppEvents) with string-valued properties. The [telemetry] extra is kept empty so existing install commands still resolve.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d578414. Configure here.
At the 5 MB cap emit() dropped the event without marking work pending, so flush() never started the shipper and a full spool could never drain: telemetry stopped for good on that machine even after it came back online. Hitting the cap now marks the backlog pending like a successful write does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Problem
The
azure_monitortelemetry backend went throughazure-monitor-opentelemetry. In a short-lived CLI that costs a lot:bcli endpoint listwent from 0.3 s to 2.3 s. Disabling tracing, metrics and live metrics didn't change it.Change
emit()appends one Application Insights envelope per event to~/.config/bcli/telemetry/spool.jsonl. It takes microseconds and makes no network call.flush()(called once at exit) starts a detachedpython -m bcli.telemetry._ship(start_new_sessionon POSIX,DETACHED_PROCESSon Windows) that posts the spool to the connection string'sIngestionEndpointwithhttpx, after the command has returned.O_EXCLlock, and takes it over once stale (5 min);[telemetry]extra is now empty and kept so existing install commands resolve, mirroringcli = []. The lockfile drops the OpenTelemetry/Azure Monitor dependency tree.backend = "azure_monitor", sameconnection_string.Events now arrive as real custom events (
customEvents/AppEvents) instead of traces. Properties are string-valued, so numbers read back withtodouble(...).Verification
bcli endpoint listagainst a real Application Insights resource: 0.29–0.32 s with telemetry on vs 0.28–0.29 s off, nothing on stderr beyond normal output. Previously it took 2.3 s and printed the traceback.bcli.startup, 5 ×bcli.command) arrived inAppEvents, and the spool directory was empty afterwards.tests/test_telemetry/test_ship.pycovers: batching; retry on 503/429/connection error with only the remainder kept; final 400 not retried; malformed lines dropped; fresh lock respected; stale lock taken over.test_sink.pycovers: the envelope shape, connection-string parsing (including the default endpoint), the spool cap, an unwritable spool never raising, andflush()spawning only after something was spooled.ruff check src/is clean.Not exercised: the Windows
DETACHED_PROCESSpath on a real Windows machine.Note
Medium Risk
Changes how optional telemetry is persisted on disk and uploaded (background subprocess, spool retention), though it does not touch auth or core API paths.
Overview
Replaces the
azure_monitorbackend’sazure-monitor-opentelemetryintegration with local spooling and a detached shipper, so CLI commands no longer pay ~2s exit-time exporter shutdown or Azure VM IMDS probe noise on non-Azure machines.During a command,
emit()appends Application Insights event envelopes to~/.config/bcli/telemetry/spool.jsonl(string-valued properties, 5 MB cap with drops past the limit).flush()at exit startspython -m bcli.telemetry._ship, which posts batches viahttpx, uses a single-shipper lock (stale takeover), renames the live spool to outbox files for concurrent runs, and retains unsent lines on retryable failures.The
[telemetry]extra is a no-op (httpxis core); lockfile drops the OpenTelemetry/Azure Monitor tree. No config change — samebackendandconnection_string. Tests cover the shipper and updated sink behavior.Reviewed by Cursor Bugbot for commit de3df00. Bugbot is set up for automated code reviews on this repo. Configure here.