feat(telemetry): post at the host door, and stop retrying a verdict (backend#2385) - #558
Conversation
…backend#2385) Two changes, and the ORDER between the repos matters: this must not be released before backend#2385's route reaches the environment it posts to. The 404 rule below is what makes an early release non-destructive rather than a rule to remember. * The path moves to `/telemetry/v1/host/records/`. The old one is the EDGE boundary, gated on `IsAuthenticatedEdge`; this CLI sends a human's `ClientAccessToken`, so every POST it ever made was a 403 by construction — 16 in prod in the seven days to 2026-08-23. No re-login helps; a data scientist cannot become an edge. * `classifyStatus` moves two rows, in opposite directions, because the question is "can a later run go differently?" and not "is this 4xx": 403 now DISCARDS (at the host route it is a verdict on this principal — wrong credential, or a user with no account needing an operator — and re-posting reproduces it every invocation until the batch ages out at the spool cap), and 404 now RETRIES (on a versioned path that is a deployment state, so discarding would destroy exactly the records the route accepts once the promotion lands). Tests: the classify table gains both rows with their reasons; a new `TestDeliverActsOnTheVerdictItGets` runs the real `deliver` against a real server and reads the spool back, so a correct mapping wired to a `deliver` that ignores it cannot pass; and the path is pinned against a literal, because `TestPostBatchSendsBearerAndJSON` compares the constant with itself and would stay green if it were changed back to the door that 403s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Backend half: tracebloc/backend#2393 — the route this posts to. |
The version-bump gate is right: v0.10.10 is already released and this PR changes a published path (`internal/*`), so leaving VERSION stale would not fail here — it would fail the next prod hop, days later, on somebody else. The release train reads this file and cuts the tag from it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saqlainsyed007
left a comment
There was a problem hiding this comment.
Verified against the code — the transport half of #2385, consistent with the backend route in #2393 (which I approved).
The path moves from the edge door /telemetry/v1/records/ (gated on IsAuthenticatedEdge, so every human-credential CLI POST was a 403 by construction — 16 in prod over 7 days) to the host door /telemetry/v1/host/records/, matching the route #2393 adds. The retry reclassification is the interesting part and it's reasoned correctly:
- 403 now discards — at the host route it's
IsAuthenticatedHostTelemetryProducerrefusing (no client credential — which a signed-in CLI always has) or no-tenant, both verdicts on THIS principal that every re-post reproduces until the batch ages out. Retrying was the wedge #2385 measured. - 404 now retries — on a versioned path that's a deployment state, not a verdict: a CLI released ahead of the backend gets 404 until the promotion lands, and discarding would destroy exactly the records the endpoint accepts a day later. The cost of being wrong is bounded (one request/invocation, capped at the spool max).
- 401 still retries (credential), 5xx transient — unchanged and correct.
TestTelemetryIngestPathIsTheHostDoor pins the literal (unavoidable — the route name lives in the backend repo) and specifically asserts it is NOT the edge door, calling out that the pre-existing test compared the constant with itself and would stay green on any change. Rule 9 done right. The release-order caveat (don't ship ahead of #2393; the 404-retry rule makes early release non-destructive via the spool) is correct.
CI green, no threads. LGTM.
The transport half of tracebloc/backend#2385. Backend half: tracebloc/backend#2393.
Do not release this ahead of backend#2393 reaching the environment the CLI
posts to. The 404 rule below is what makes an early release non-destructive
rather than a rule someone has to remember: a CLI that runs against a backend
without the route spools its records and drains them once the promotion lands,
instead of throwing them away.
Why every telemetry POST this CLI ever made was refused
telemetryIngestPathpointed at/telemetry/v1/records/, which is the edgeingest boundary — backend#1905 gates it on
IsAuthenticatedEdge. This CLI sendsAuthorization: Bearer <cfg.CurrentToken()>, aClientAccessTokenthat resolvesto the signed-in human. Authenticated, not an edge: 403, by construction, every
invocation. No re-login helps; a data scientist cannot become an edge.
Measured in prod over the seven days to 2026-08-23:
Forbidden /telemetry/v1/records/× 16, most recent 13:59 that day.backend#2393 adds a door for host producers. This points at it.
Two changes
1. The path.
/telemetry/v1/host/records/— the route backend#2393 registersas
host-telemetry-ingest. Same versioning, same reason: we spool to disk andforward later, so a payload written by an old CLI can arrive after the backend
has moved on.
2.
classifyStatusmoves two rows, in opposite directions. The question eachrow answers is "can a later run go differently?", not "is this a 4xx".
credential state rather than a payload verdict. At the host route it is
neither: the permission refuses a request carrying no client credential —
which a signed-in CLI always has — and the endpoint's other 403 says the user
resolves to no tenant, which needs an operator to attach an account, not a
retry. Both are verdicts on this principal; re-posting reproduces them
exactly, once per invocation, until the batch ages out at the spool cap. That
is the wedge backend#2385 measured: a request burned per command, delivering
nothing.
a 404 is a deployment state, not a verdict — and discarding would destroy
precisely the records the endpoint accepts a day later. Being wrong the other
way is bounded and small: a permanently wrong path costs one request per
invocation and at most
telemetrySpoolMaxrecords, the same shape as anunreachable host.
401 still retries (the next
tracebloc loginfixes it). 408/429/5xx unchanged.Tests
TestClassifyStatusgains both rows with their reasons.TestDeliverActsOnTheVerdictItGets(new) runs the realdeliveragainst areal
httptestserver and reads the spool file back.TestClassifyStatusis astatement about a switch; this is a statement about what happens on disk, so a
correct mapping wired to a
deliverthat ignores it cannot pass.TestTelemetryIngestPathIsTheHostDoor(new) pins the path against aliteral.
TestPostBatchSendsBearerAndJSONalready asserts the request goes totelemetryIngestPath— but that compares the constant with itself and wouldstay green if it were changed to anything at all, including back to the door
that 403s. This is the assertion that would not. The literal is deliberate: the
path lives in another repository, so there is no symbol to derive it from, and
a backend route change must update this test in a commit that says so.
Mutation proof
Applied to the real code, anchor asserted present, reverted after:
postRetryTestClassifyStatusand the 403 subtest ofTestDeliverActsOnTheVerdictItGetspostDiscardTestClassifyStatusand the 404 subtest/telemetry/v1/records/TestTelemetryIngestPathIsTheHostDoor, on both assertionsVerification
go build ./...— cleango vet ./internal/cli/— cleangofmt -l internal/cli/— no outputgo test ./internal/cli/ -count=1—ok github.com/tracebloc/cli/internal/cli 30.302smake check— green (vet, fast tests, fmt-check, the 10 format.sh self-test properties, file-budget, style guard, tool pins)Note
Medium Risk
Changes where telemetry is sent and which HTTP failures drop vs. retry spooled events. Wrong path or classification can lose or wedge host telemetry, but this is not product auth or user data.
Overview
CLI telemetry now posts to the host ingest route (
/telemetry/v1/host/records/) instead of the edge route that always 403s a humanClientAccessToken. Bumps the CLI to0.10.11.classifyStatusis inverted for two codes: 403 discards (a principal/tenant verdict that would wedge the spool if retried) and 404 retries (a missing versioned route is treated as a deploy lag so an early CLI does not drop records). 401/408/429/5xx are unchanged.Tests pin the host path against the old edge path and assert that
deliveractually consumes vs. keeps the spool for 403 vs. 404. Do not ship this before the backend host route is live; 404 spooling is what makes an early CLI non-destructive.Reviewed by Cursor Bugbot for commit 0bb78f3. Bugbot is set up for automated code reviews on this repo. Configure here.