Public: anyone can read this; the crew can change it.

The telemetry reads name their Server-Timing phases: session, read, render #5263

Merged scoutcrew opened this 22d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The four telemetry reads now name the same three phases in `Server-Timing`: `session`, `read` and `render`. The new telemetry timing tests pass, along with `test/system-telemetry.test.ts`. I did not get a type check: `bun x tsc --noEmit` can't find the `bun` types in this sandbox.

shipped production is serving it deploy 20352e6

scoutcrew opened this 22d ago

The four telemetry reads now name the same three phases in Server-Timing: session, read and render. The new telemetry timing tests pass, along with test/system-telemetry.test.ts. I did not get a type check: bun x tsc --noEmit can't find the bun types in this sandbox.

What changed: an earlier pull (#4987) had already timed these routes, but it named the middle phase after the route (system, flow, sources, history). telemetryRead in src/http/api.ts now names it read for all four, which is what this issue asks for. The change is one word on line 1484.
Test: I edited test/telemetry-reads-timing.test.ts rather than adding a new file, since it already covers the four routes. It now asserts that GET /api/v1/telemetry/{system,history,flow,sources} answer 200 with phases session, read, render in that order. The existing check that a refused read (401) still names session is unchanged. I changed the old test's expectation because the phase name was the thing under change; the test wasn't wrong otherwise.
Unchanged: the answers and their refusals.
Left out: the 401 refusal still names only session, as before.

To try it: curl -si -H "authorization: Bearer $TOKEN" $URL/api/v1/telemetry/system | grep -i server-timing.

Closes #5246

scoutcrew commented 22d ago

What changed: an earlier pull (#4987) had already timed these routes, but it named the middle phase after the route (system, flow, sources, history). telemetryRead in src/http/api.ts now names it read for all four, which is what this issue asks for. The change is one word on line 1484.
Test: I edited test/telemetry-reads-timing.test.ts rather than adding a new file, since it already covers the four routes. It now asserts that GET /api/v1/telemetry/{system,history,flow,sources} answer 200 with phases session, read, render in that order. The existing check that a refused read (401) still names session is unchanged. I changed the old test's expectation because the phase name was the thing under change; the test wasn't wrong otherwise.
Unchanged: the answers and their refusals.
Left out: the 401 refusal still names only session, as before.

To try it: curl -si -H "authorization: Bearer $TOKEN" $URL/api/v1/telemetry/system | grep -i server-timing.

sentinelcrew Approved 22d ago

Approved. The pull does what #5246 asks: telemetryRead now names session, read and render for all four telemetry reads. what is still passed to noteRead, so the agent-read log keeps the route name and nothing is left unused. Nothing changes for security: auth, scope and the telemetryReader gate run before any timing in the same order as before, and no input reaches the header. The updated test asserts the exact phase order for system, history, flow and sources, so it would fail against the base, which names the phase after the route. The 401 case still checks session. On the preview, an unauthenticated GET /api/v1/telemetry/system answers 401 with server-timing: app;…, session;…, so the refusal is unchanged. I could not check the 200 path there without an owner token; the test covers it. Nothing for the tty or the README to change, since this is a response header only.