Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/features/telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ The `docker_cli_source` field is likewise a **fixed enum** (`path`, `bundled`, `

Docker isolation failures surface in `error_code_counts_24h` via three stable diagnostic codes (schema v5): `MCPX_DOCKER_CLI_NOT_FOUND` (isolation requested but the `docker` binary is unresolved — issue #696), `MCPX_DOCKER_EXEC_NOT_FOUND` (the image lacks the interpreter the server needs, e.g. `uvx` missing in `python:3.11`), and `MCPX_DOCKER_OCI_RUNTIME` (OCI runtime / architecture-mismatch failures).

## When heartbeats are sent

A heartbeat goes out 5 minutes after start (so short-lived processes stay quiet), then once every 24 hours, and **once more on graceful shutdown**.

The shutdown heartbeat exists because the windowed counters live in memory and are cleared only after the endpoint accepts a send. Without it, everything an install did after its 5-minute heartbeat was discarded whenever the process exited before the 24-hour tick — which is most desktop sessions. It is skipped when telemetry is disabled or opted out, and when nothing has been recorded since the last accepted heartbeat. It is hard-bounded to a few seconds, so an unreachable endpoint can never delay quitting, and because counters are cleared only on an accepted send, a failed shutdown flush simply leaves the counts for the next run rather than dropping them.

Delivery is therefore **at-least-once**, not exactly-once. If a send is cut off after the endpoint accepted it but before mcpproxy sees the response, the counts stay pending locally and are re-sent — a lost response is indistinguishable from a lost request. Receivers should treat a heartbeat as idempotent per `(anonymous_id, timestamp)`. Retrying is the deliberate choice: the alternative is silently discarding the window, which is the problem the shutdown heartbeat was added to solve.

Two housekeeping steps are deliberately *not* performed by the shutdown heartbeat — the 365-day anonymous-ID rotation and the one-shot `installer` launch-source attribution. Both are consumed when the payload is built rather than when it is accepted, so running them for a send that may never land would spend state a normal run still has. The next start performs both.

## What is NOT collected

The following is **never** collected:
Expand Down
7 changes: 7 additions & 0 deletions internal/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,13 @@ func (r *Runtime) Close() error {
// ActivityService.Stop above — terminally stops the service so a Start
// goroutine not yet scheduled (lifecycle.go launches it via `go`) becomes
// a no-op instead of writing after this point.
//
// Stop also performs the graceful-shutdown heartbeat flush — one bounded
// (4s) final send of the counters recorded since the last accepted
// heartbeat, which would otherwise die with the process. It runs after the
// loop is joined, so it is the only sender at that moment, and it happens
// HERE — while the BBolt handle is still open and before the marker
// resolves below — because its buildHeartbeat writes funnel activity.
if r.telemetryService != nil {
r.telemetryService.Stop()
}
Expand Down
22 changes: 18 additions & 4 deletions internal/telemetry/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,22 +337,36 @@ func TestInstallerPending_ClearedAfterFirstHeartbeat(t *testing.T) {
activationDB: db,
}

// First call → "installer" and the pending flag must be cleared.
ls1 := s.resolveLaunchSource()
// The shutdown flush must NOT consume the one-shot: it builds a payload that
// may never be accepted, so a non-consuming resolve has to leave the flag
// armed for the next run and report the runtime-detected source instead.
if ls := s.resolveLaunchSource(false); ls == LaunchSourceInstaller {
t.Fatal("non-consuming resolveLaunchSource claimed installer; the shutdown flush must not spend the one-shot")
}
pendingAfterFlush, err := store.IsInstallerPending(db)
if err != nil {
t.Fatalf("IsInstallerPending after non-consuming resolve: %v", err)
}
if !pendingAfterFlush {
t.Fatal("non-consuming resolveLaunchSource cleared installer_heartbeat_pending; a failed shutdown flush would destroy the attribution")
}

// First consuming call → "installer" and the pending flag must be cleared.
ls1 := s.resolveLaunchSource(true)
if ls1 != LaunchSourceInstaller {
t.Fatalf("first resolveLaunchSource = %q, want %q", ls1, LaunchSourceInstaller)
}
pending, err := store.IsInstallerPending(db)
if err != nil {
t.Fatalf("IsInstallerPending after first call: %v", err)
t.Fatalf("IsInstallerPending after first consuming call: %v", err)
}
if pending {
t.Fatalf("installer_heartbeat_pending should be cleared after first heartbeat")
}

// Second call → no longer installer; should return the runtime
// detector's value (whatever it is in the test env), not installer.
ls2 := s.resolveLaunchSource()
ls2 := s.resolveLaunchSource(true)
if ls2 == LaunchSourceInstaller {
t.Fatalf("second resolveLaunchSource still returned installer — one-shot broken")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/telemetry/feedback.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (s *Service) SubmitFeedback(ctx context.Context, req *FeedbackRequest) (*Fe
return nil, fmt.Errorf("failed to marshal feedback: %w", err)
}

url := s.endpoint + "/feedback"
url := s.liveEndpoint() + "/feedback"
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(data))
if err != nil {
return nil, fmt.Errorf("failed to create feedback request: %w", err)
Expand Down
6 changes: 3 additions & 3 deletions internal/telemetry/optout.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (s *Service) EmitOptOutBeacon(ctx context.Context) bool {
if disabled, _ := IsDisabledByEnv(); disabled {
return false
}
if s.config.GetAnonymousID() == "" {
if s.liveAnonymousID() == "" {
return false
}
if err := s.SendOptOutBeacon(ctx); err != nil {
Expand All @@ -98,7 +98,7 @@ func (s *Service) EmitOptOutBeacon(ctx context.Context) bool {
// caller-supplied URL, so this never sends to an arbitrary, request-derived
// host.
func (s *Service) SendOptOutBeacon(ctx context.Context) error {
anonID := s.config.GetAnonymousID()
anonID := s.liveAnonymousID()
if anonID == "" {
// Nothing to dedup on — never send an identity-less beacon.
return errors.New("opt-out beacon skipped: no anonymous_id")
Expand All @@ -122,7 +122,7 @@ func (s *Service) SendOptOutBeacon(ctx context.Context) error {
// require a host, and issue the request against the re-serialized URL. This
// mirrors validateRegistryURL and guarantees a malformed/non-http endpoint
// can never aim the beacon at, e.g., file:// or a schemeless host.
beaconURL, err := validateTelemetryURL(strings.TrimRight(s.endpoint, "/") + "/heartbeat")
beaconURL, err := validateTelemetryURL(strings.TrimRight(s.liveEndpoint(), "/") + "/heartbeat")
if err != nil {
return err
}
Expand Down
44 changes: 44 additions & 0 deletions internal/telemetry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,50 @@ func (r *CounterRegistry) Snapshot() RegistrySnapshot {
return snap
}

// HasPendingCounters reports whether anything has been recorded since the last
// Reset — i.e. whether a heartbeat would carry usage data that has not yet been
// accepted by the endpoint.
//
// The counter set inspected here is EXACTLY the set Reset zeroes: those are the
// counters whose window is "since the last accepted send", so they are the ones
// that would be lost if the process exited without transmitting. Lifetime
// tallies that Reset deliberately preserves (anonymityViolations) are excluded —
// they are never transmitted and must not, on their own, justify a send.
//
// Used by the graceful-shutdown flush to skip a final heartbeat that would carry
// no new information.
func (r *CounterRegistry) HasPendingCounters() bool {
if r == nil {
return false
}
for i := range r.surfaceCounts {
if r.surfaceCounts[i].Load() > 0 {
return true
}
}
if r.upstreamTotal.Load() > 0 {
return true
}

r.mu.RLock()
defer r.mu.RUnlock()

if len(r.builtinCalls) > 0 || len(r.restEndpoints) > 0 ||
len(r.errorCategories) > 0 || len(r.doctorChecks) > 0 {
return true
}
if r.tpaScansCompleted > 0 || r.tpaScansFailed > 0 || r.tpaScansWithFindings > 0 ||
r.tpaToolChangeGateScans > 0 || r.tpaPromptScans > 0 {
return true
}
for _, v := range r.tpaFindings {
if v > 0 {
return true
}
}
return false
}

// Reset zeros all counters. Called only after a successful heartbeat send.
//
// Deliberate, registry-wide trade-off: an event recorded between Snapshot()
Expand Down
Loading
Loading