feat(streamtelemetry): measure bytes on every serving path, and merge them into one view - #667
Merged
Merged
Conversation
…kens
Session age is the ordering signal every later enforcement rule depends on
("cut the newest stream first"), but nothing carried a stable creation time.
`Sign` overwrites `RegisteredClaims` wholesale on every mint, so `iat` is issue
time, not session-start time, and a replan mints a replacement token that
resets it. Reconstruction after a restart never set `StartedAt` at all, so
`RegisterReconstructed` stamped `time.Now()`, and the proxy re-stamped
`time.Now()` on every HLS touch. Session age therefore reset on every restart,
reconnect and segment request.
Adds an explicit `ostn` claim carrying the creation time in Unix nanoseconds.
Nanoseconds rather than seconds because victim ordering is defined as
(startedAtUnixNano, sessionID): at second precision, sessions started in the
same second would fall back to sorting by random UUID. `int64` decodes exactly
through golang-jwt's struct unmarshal.
Resolution is centralised in `Claims.StartedAt`, which returns an explicit
source rather than a bool: `Sign` always writes `iat`, so a legacy token always
resolves *something*, and the caller must be able to tell an authoritative
value from a degraded one. `iat` is treated as degraded because it is not
stable across re-mints. A missing claim never invalidates an otherwise valid
token.
`RecipeCard` carries the value as a `time.Time` (RFC3339Nano, full precision),
`ReconstructSession` seeds `Session.StartedAt` from it, and the proxy's node
session record reports it. `SessionInfo.StartedAt` keeps its existing RFC3339
encoding — the `/api/v1` additive-only rule forbids re-encoding an existing
field — so precision and provenance are exposed additively as
`started_at_unix_nano` and `started_at_source`.
Part of the stream telemetry and enforcement effort (P0a).
AI-use disclosure: implemented with AI assistance (Claude planning and review,
Codex gpt-5.6-sol implementing), verified against the repo's own build, vet,
lint and test gates.
Every Jellyfin-client stream served through a proxy node was attributed to nobody. In the admin "active streams" view those sessions showed a node, a type and a byte count, but no user, no profile and no media file — so an operator could see that something was streaming without being able to see who was watching what. `buildProxyRedirectURL` signed the stream token with the session id, media path, play method and the audio/DV fields, but never set `UserID`, `ProfileID` or `MediaFileID`, even though the claims struct carries all three and the compat session knows them. The proxy copies exactly those three claims into its node-session record, so the gap surfaced directly in the admin view. Populates the three ownership claims from the compat session (`StreamAppUserID`, `ProfileID`) and the negotiated source (`FileID`), and passes the play session's creation time so proxied sessions also carry the immutable start time added in the previous commit. For compat, the top-level `PlaybackSession.CreatedAt` is the source of truth and is overlaid onto the recipe card at every reconstruction and persistence point: the durable compat store unmarshals and rewrites the whole JSON document, so an older replica in a mixed-version deploy silently drops unknown *nested* fields, and a timestamp living only inside the nested recipe would be erased. Wire-safe: `Verify` decodes into a struct and does not require an exact claim set, so an older proxy binary ignores claims it does not model. Two accepted costs: the token grows, and claims are signed but not encrypted, so internal user/profile/file ids become readable to anyone already holding the (already sensitive) stream URL. Adds a claim-growth budget test, which the repo previously lacked, plus a mixed-version reconstruction test covering the nested-field-dropped case. Part of the stream telemetry and enforcement effort (P0a). AI-use disclosure: implemented with AI assistance (Claude planning and review, Codex gpt-5.6-sol implementing), verified against the repo's own build, vet, lint and test gates.
Two viewer-facing entry points recorded the wrong address. The standalone proxy mounted only CORS and egress metering, and the dedicated Audiobookshelf listener only its own access log — neither ran the trusted-proxy resolver that the native and Jellyfin routers have always had. The ABS case was not an empty field but a wrong one: `requestClientIP` falls back to `RemoteAddr`, so behind a reverse proxy every audiobook session and every `RemoteAddr`-based log line recorded the *proxy peer* rather than the viewer. The proxy listener had no resolution at all. Mounts `clientip.Middleware` first on both, so it runs before anything that reads the address. Proxy mode already has a Postgres pool and a config watcher, so the trusted-CIDR list and its hot reload work there exactly as in integrated mode. Error semantics deliberately mirror the integrated path: a malformed CIDR list at startup is fatal rather than silently starting with an empty trust set, because failing open would make every forwarding header both untrusted and unverified; a malformed list on reload logs and retains the last valid CIDRs. The reload closure that integrated mode already used is extracted and shared rather than copied. Behavior change worth stating: recorded session IPs and RemoteAddr-based log lines on both listeners now show the resolved viewer address instead of the reverse-proxy peer, since the middleware overwrites RemoteAddr. Adds trust-boundary tests over the mounted proxy router on a real socket — trusted forwarding header honored, spoofed header from an untrusted peer ignored, and a runtime narrowing of the trusted set taking effect — because the resolver reads RemoteAddr, which only a real connection populates. Part of the stream telemetry and enforcement effort (P0a). AI-use disclosure: implemented with AI assistance (Claude planning and review, Codex gpt-5.6-sol implementing), verified against the repo's own build, vet, lint and test gates.
…rappers Media was being served the slow way, and one middleware silently disabled the server's ability to interrupt a stuck stream. `io.Copy` — and therefore `http.ServeContent` — finds `io.ReaderFrom` by direct type assertion and never through `Unwrap()`. Every status/logging/metrics wrapper on a media route that did not forward `ReadFrom` turned off the zero-copy path for everything below it, so large direct-play and download bodies were copied through the application instead of handed to the kernel. Separately, a wrapper without `Unwrap()` dead-ends `http.ResponseController`, which is how the rolling write deadline is set — the same deadline the enforcement phase will use as its in-flight interrupt. Adds shared helpers in `httpstream` (`ReaderFromOf`, `CopyChunked`, `WriterOnly`) and forwards `ReadFrom` through every wrapper on a live media chain, preserving each one's own accounting: byte-counting wrappers (the proxy egress meter, the ABS access log, the jellycompat debug writer) transfer in bounded slices and credit each one, so the meter's rolling per-second window is not collapsed into a single bucket by one large transfer. Also adds the `Unwrap` the ABS access log never had (GAP-10) and the `Unwrap`/`Hijack` the jellycompat image-proxy writer never had. chi's `compressResponseWriter` implements `Unwrap`, `Flush`, `Hijack` and `Push` but not `ReadFrom`, and its handler wraps unconditionally — the encoder is chosen later, so even a non-compressible content type gets a wrapper that kills sendfile. It is third-party, so it cannot be repaired. Compression is therefore bypassed on exact bulk-media routes via `CompressExcept`, matching only the registered GET/HEAD methods with exact segment counts and exact casing, so a wrong-method or child path is never swallowed. Blanket bypass would have been wrong: subtitle font bundles are JSON served under the same global compressor, and bypassing them would change the wire contract. Also fixes a pre-existing reap of healthy streams. The deadline was refreshed only between 64 MiB slices against a 180s stall window, so any client sustaining less than ~3 Mbit/s had its deadline expire mid-slice and was killed despite continuous progress. The slice is now 4 MiB (~186 kbit/s floor), with tests covering both a steadily-progressing slow stream and the oversized-slice failure mode, plus a guard on the constant itself. Verified over real sockets against the mounted routers — GET, HEAD, single and multi-range, conditional responses, Accept-Encoding present and absent, HTTP/2, proxy-to-node relay, the ABS socket.io upgrade, and the jellycompat image-proxy client path — because handler-level tests bypass exactly the middleware this changes. Adds direct-play, remux and high-RPS HLS benchmarks as a baseline for the hot path. Part of the stream telemetry and enforcement effort (P0a). AI-use disclosure: implemented with AI assistance (Claude planning and review, Codex gpt-5.6-sol implementing), verified against the repo's own build, vet, lint and test gates.
…utes P0b of the stream telemetry and enforcement design. Adds process-local, observation-only telemetry behind SILO_STREAM_TELEMETRY_ENABLED (default off). Nothing is rejected, delayed, throttled or cut, and neither PostgreSQL nor Redis is written. internal/streamtelemetry carries the three-level model from the design's 2.2: Observation per in-flight request, logicalSession keyed by canonical session id, and transfer for download-class pours. Every observation folds its final byte total in on release under the session lock, so a short HLS transfer that lives and dies between sweeps is still counted and can neither double-count nor lose growth. Retention, session/transfer/observation counts and every per-session set are bounded; saturation serves through and is reported through Truncated, monotonic dropped counters and a rate-limited warning. Observe counts bytes but creates no logical activity. The handler calls Attach only after it has loaded and authorized the session, because ownership is established inside the handler and the transcode serve routes deliberately allow an unauthenticated caller. 401/403/404 therefore create nothing, and never-attached bytes land in the unattributed counters. Media routes are declared as typed MediaRoute values for all five router families, each with a route-manifest test that walks the mounted router and fails the build on any route that is neither declared nor in that family's checked-in non-media allowlist. Only the native family is enrolled; the other four are classified and will be enrolled one at a time. observedWriter obeys the P0a writer-chain conformance rules, so sendfile, deadline traversal and the optional interfaces survive the extra wrapper.
…view P0c of the stream telemetry and enforcement design. Adds the distributed read-only view: publisher sequencing, a Redis snapshot transport, freshness, and the merged GlobalMonitoringView with its complete/degraded flag. Still observation only, behind SILO_STREAM_TELEMETRY_DISTRIBUTED (default off). No election, no fence token, no sanctions, no admin endpoint, no /api/v1 change and no PostgreSQL write. Each process carries a random publisher id, a process epoch and a sequence incremented exactly once per published snapshot. Snapshots land in one Redis hash per publisher instance, so a stalled or oversized publisher cannot make every session vanish atomically the way a per-node blob would. Publishing is one MULTI/EXEC rather than a Lua script: Lua's unpack exceeds its C-stack limit past roughly eight thousand elements, which is inside the existing ten thousand session cap, and chunking the script would forfeit the atomicity it existed for. A concurrent HGETALL still observes one side of the update. Membership is the heartbeat itself. Publishers score themselves into a sorted-set roster, pruned by ZREMRANGEBYSCORE with a two-TTL margin so one clock cannot evict another. A heartbeat that is fresh while its snapshot is stale means the publisher stalled, so the view is degraded and names it; a heartbeat past the membership TTL means the process is gone, so it is dropped and the view is whole again. This matters because session byte totals are monotonic and consumers derive rates by subtraction: silently dropping a publisher makes a merged sum move backwards. BuildGlobalView is pure, taking the roster, decoded snapshots, errors, a build time and every bound as input, so the merge contract is unit tested without Redis. Viewer bytes sum only viewer-egress route activity and never the all-roles SessionView total, relay bytes stay separate for correlation, viewer addresses union, open observations sum, and identity is contributed only by publishers that authenticated the request. A populated disagreement over subject, profile or media file records every value with its publishers and leaves the scalar empty rather than picking an edge; play method gets no merged scalar at all, because no available timestamp can prove which publisher's value is later. Completeness additionally requires that no publisher truncated, that the reader hit no cap and that nothing failed to decode, and the view names the reasons it is incomplete. Wire values are versioned JSON with explicit field tags, Unix nanosecond times that preserve the zero time, and decode-time rejection of negative counters. The design asked for compact binary; JSON is a deliberate deviation, isolated behind the store interface, taken because a hand-written binary codec for a struct with this many maps and slices was the likeliest source of defects in a change whose whole value is a correct merge. Encoding cost and size are benchmarked and recorded.
P0b shipped with only the native family Enrolled. This enrols the proxy
viewer edge and the transcode node, the pair that first exercises the
relay-vs-viewer byte split §2.2's Role field exists for.
Proxy routes attach after the handler's last authorization check —
after verifyToken for the stream routes, after the PlayMethodDownload
check on the local download branch, and after ValidArtifactID inside
relayDownloadArtifact — so a rejected request creates no logical
activity. Downloads attach as Transfers, never sessions: proxy download
tokens mint a fresh session id per redirect by construction.
The node publishes a correlation key and nothing else. Its URL
{session_id} is the transcode transport id, not the canonical playback
session id, so canonicalSessionID resolves the viewer edge's id from the
forwarded X-Silo-Stream-Token and falls back to node-transport:<id>
rather than joining a session it cannot prove. Its capture hook records
no viewer IP, device or client: the peer is an API or proxy process
behind requireBearer, and recording its address would put a server
address in ViewerIPs. §4.3 — a node cannot know who is watching.
Merged start-time authority now comes from viewer-edge contributions
only. A relay contribution carries a publisher-local first-seen stamp,
which normalizeStartedAt marks degraded; mergeSession previously ORed
that across every publisher, so correlating a node would have flipped an
authoritative proxy session to degraded.
CopyChunked no longer nests io.LimitReader. The kernel sendfile path
unwraps exactly one limiter before it looks for the *os.File, so every
accounting layer that re-wrapped its source silently forfeited sendfile
— including on origin/main, where the egress meter forwarded no ReadFrom
at all. Measured with strace over an 8 MiB body through the mounted
proxy direct-play router: 0 sendfile syscalls before, 6 after, through
three accounting wrappers.
Standalone proxy and transcode processes now build a registry and join
the Redis roster, with Stop deferred so a deploy does not leave a stale
roster entry degrading the global view for MembershipTTL. No new feature
flag: those are separate processes, so SILO_STREAM_TELEMETRY_ENABLED
already gates each family independently.
Measured cost, paired sub-benchmarks in one run at -count=5:
direct play +10 allocs/op and ~1.1 KB/op; transcode segment +11
allocs/op and ~1.3 KB/op. Throughput ranges overlap on both.
Built via a Claude<->Codex relay: Claude (Opus 5) planned and reviewed,
Codex gpt-5.6-sol adversarially reviewed the plan and implemented it,
Claude ran the gates and confirmed three defects, Codex fixed them.
Part of #135
Completes P0b's enrolment. Every declared media route in the repository is now observed; no family is left blind. Adds SILO_STREAM_TELEMETRY_FAMILIES, which the proxy and transcode-node change deliberately did without. Those are separate processes, so their own SILO_STREAM_TELEMETRY_ENABLED already gated them per family. Jellycompat and ABS share the API process with native, so without a gate this change would widen instrumentation across two more live byte paths on upgrade alone. The default set is therefore what shipped before this commit — native, proxy, transcode_node — and a shared-process family is named explicitly to enable it. The same variable is the kill switch: one misbehaving family can be dropped without losing observation of the rest. The resolved set is logged at startup. An unrecognized name disables telemetry and names the variable; a typo that silently observed nothing would be worse than no telemetry. The gate is read once per route at mount time, so it costs nothing per request. The attachment boundary is stated precisely and applied consistently: a logical session exists from AUTHORIZATION SUCCESS, not from a 2xx. Requests rejected before that point create nothing; a failure after it records an outcome on a real session, because it is real traffic by an authorized principal. This decides HandleMasterManifest, which finishes authorization at the CompatToken and media-source checks and then starts a transcode before writing a byte — the attach lands before that side effect, which is the whole reason §4.2 enrols manifest routes. Compat identity comes from the authenticated compat session, whose StreamAppUserID is the numeric silo account id, and its capture hook reads DeviceId/Client/Version through firstMediaBrowserAuthorizationValue — the parser the negotiation path already uses — rather than X-Silo-Client*, which Jellyfin clients never send. ABS reuses absPlaybackClientInfoFromRequest for the same reason, and absSubject maps a positive ABS user id onto UserSubject so ABS bytes sum with native and compat per user (§4.2b identity normalization); "0" and "-1" parse but name no account, so they stay abs_user. ABS routes are wrapped per route, never as another r.Use on the group Mount shares with socket.io. TestMountedStandaloneRouterPreservesSocketIOHijack now runs with telemetry both off and on — the §4.4 websocket regression the design owed, which only means anything with the wrapper mounted. BytesAccepted is pre-compression on any compat media route still compressed (subtitles), and wire bytes on the ones skipCompatMediaCompression exempts. Documented at compatCapture rather than "fixed". Measured cost, paired sub-benchmarks in one run at -count=5: jellycompat direct stream +10 allocs/op and ~1.2 KB/op; ABS public track +10 allocs/op and ~1.2 KB/op. Both match the native and proxy families. Planned via a Claude<->Codex relay (Claude Opus 5 planned, Codex gpt-5.6-sol adversarially reviewed the plan: nine findings, seven accepted, including the conservative family-gate default and the attachment-boundary correction). Codex hit its usage limit before the implementation step, so the implementation and review are Claude's alone — the plan review is the only cross-model step in this commit. Part of #135
Serves the merged global view beside both legacy live-session projections and the diff between them, at GET /api/v1/admin/stream-telemetry/parity. Read-only: no /api/v1 response changes, no migration, no Postgres or Redis write. This change compares; it does not cut over. §6 puts the repoint after parity is demonstrated, and there is nothing to demonstrate it with yet — telemetry is off in every deployment. The admin session payload is also a join rather than a swap: playbackSessionRow carries ~50 display fields (title, poster, season/episode, position, decisions, source codecs) that telemetry is explicitly not canonical for. Repointing belongs to the separate retirement change, which this endpoint exists to give evidence for. Closes the open item left by P0c. BuildGlobalView measured 347 ms at 50 000 sessions, so ViewCache serves it with bounded staleness. It is read-driven rather than a ticker: a ticker would pay the full rebuild on every server forever whether or not an admin is looking, while a TTL pays only when someone asks and single-flights however many readers arrive together. A reader holding a cached value never queues behind a rebuild. A failed refresh keeps the last good view and reports the error — going blind is worse than being visibly stale — and before the first build the view is reported unavailable rather than empty, which a consumer would read as "nothing is streaming". CompareLiveSessions is pure — no clock, no store, no logger — so every rule is tested in CI without Postgres or Redis, the same property that makes BuildGlobalView testable. Only a field both sides carry can disagree: a legacy row with no profile id is a gap in that projection, not a contradiction, and counting it as one would bury the real mismatches. Start times compare with one second of tolerance, because two independent writers cannot be expected to agree to the nanosecond and nothing downstream needs them to. Every list is capped with an explicit dropped count. The projection renders a play method only when the merged view has exactly one — §2.5 leaves the scalar unset when publishers disagree, and picking one here would reintroduce the arbitrary choice the merge refuses to make — and takes the node from the viewer-edge publisher only, so a relayed session does not claim a node that never served a viewer. The view's completeness travels with the diff. A degraded view is missing sessions by construction, so a report built on one is evidence of blindness rather than disagreement; that is the distinction P0c built the flag for. A source that cannot be read reports itself unavailable with a reason instead of being omitted, which would read as "nothing to compare against". Planned, implemented and reviewed with Claude (Opus 5). Unlike the two enrolment commits before it, this one had NO cross-model adversarial review — the Codex side of the relay hit its usage limit partway through the session. The project's own gates were run in full. Part of #135
…endix for the rest Ten documents on this branch — an eight-revision design, three per-phase documents, four verbatim prior-art copies and an HTML walkthrough, 4,517 lines in all — are replaced by two, and the working document is scoped to what this branch actually built. docs/design/2026-08-17-stream-telemetry.md is the working document. It states P0 as built rather than as planned: all five route families enrolled with their route counts, the family gate and its rollout procedure, the merge and completeness contract, the parity endpoint and why P0d deliberately stopped at comparison, the measured hot-path cost, the Redis transport as implemented, and one configuration table for every variable. It carries three diagrams and opens with a plain-language summary of what the system does and does not do. The enforcement design (former section 3) and the rules design (former section 5) are moved out to the appendix. Both were written and reviewed before any traffic had been observed, and every threshold in them is a guess until the measurements this branch produces exist. Keeping them in the working document implied a commitment the branch does not make: monitoring is what ships here, enforcement is designed afterwards against real distributions. Sections 3 and 5 remain as stubs rather than being renumbered away, because Go comments cite 2.2, 2.5, 4.2, 4.2b, 4.4, 6 and 7.1 directly and renumbering would break them. Nothing in the tree cites 3.x or 5.x; references from the surviving text are retargeted at the appendix. The document is also corrected against production. It previously stated that telemetry had never run in a deployment and that parity had never been observed; both were true when written and are not now. An 18-hour soak (185 samples, native and jellycompat) is recorded in section 6 with its cost numbers, the families it did not exercise, and the legacy-store defect the parity projection surfaced (#666). docs/design/2026-08-17-stream-telemetry-appendix.md holds what the working document sheds: approaches abandoned during implementation with the measurement that killed each, eight revisions of design positions abandoned under adversarial review, the prior-art trail, a glossary resolving the inherited identifiers, the verification and review record for P0, and now the deferred P1+ design. The streaming write-deadline document keeps its own file. It predates this branch, is referenced independently, and is where someone editing CopyChunked will look.
|
Important Review skippedToo many files! This PR contains 131 files, which is 31 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (131)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Quick104
reviewed
Aug 19, 2026
Quick104
left a comment
Quick104
left a comment
Contributor
There was a problem hiding this comment.
Automated code review (Claude Code /code-review, multi-agent find → adversarial-verify pass). 23 findings posted inline: 8 confirmed correctness issues, 8 plausible correctness issues, 7 cleanup/convention notes. 10 additional candidates were refuted during verification and not posted.
Highest-priority items: the ABS login rate-limiter bypass (cmd/silo/main.go:2795), the jellycompat session-key mismatch (internal/jellycompat/streamtelemetry.go:45), the sticky truncated flag (registry.go:265), and the single-variable distributed-mode disable (config.go:183).
clientip.Middleware overwrites r.RemoteAddr with the header-derived viewer address whenever the TCP peer is a trusted proxy, which includes Docker's bridge. Mounting it on the ABS listener therefore defeated the login limiter's deliberate RemoteAddr-only keying: an attacker behind any reverse proxy could rotate X-Forwarded-For and buy a fresh burst bucket per request. The middleware now preserves the pre-overwrite peer address in the request context, and the limiter reads that instead. Anything else that must key on an address a client cannot forge should do the same. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compat attached observations under PlaybackSession.ID while the proxy, nodesessions and playback_sessions_sync all key on playback.Session.ID, and BuildGlobalView merges by exact SessionID string. One Jellyfin viewing therefore showed as two merged sessions — a byte-less compat twin and the proxy record carrying the traffic — and every compat session looked telemetry_only in parity. Compat now attaches only under UpstreamSessionID. A play session does not learn that id until ensureUpstreamPlayback/ensureTranscodeManifest has run, so the pre-side-effect attach is a no-op on a session's first request and the handler attaches again the moment the id exists, still before any byte is written. A provisional key was rejected deliberately: it recreates exactly the ghost session this fixes, and a session whose id did not exist a moment ago cannot have a pending cut against it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ime state Three defects in the process-local registry, all found by review: Truncated was sticky for the process lifetime. drop() set it and nothing ever cleared it, so one transient capacity burst pinned the merged view's Complete to false until a restart and made a later real truncation indistinguishable. It now decays over Freshness — the same horizon BuildGlobalView uses to decide a publisher is current — while the monotonic Dropped* counters keep the permanent record. SetRealtimeConnection was a no-op when the session did not exist yet. That is the normal client ordering: the control socket opens as soon as a sessionId exists, before the first media route is hit, so RealtimeConnectionAlive stayed false for the whole of every live session. State for an unknown session is now held per shard, applied when an attach creates the session, capacity-bounded against the session budget, and pruned by the sweep. The distributed cross-checks compared an env-supplied value against the DEFAULT of the other knob, so setting one variable disabled distributed mode and blamed a variable the operator never set. Knobs left at their defaults now move to satisfy the invariant; only a pair pinned to genuinely inconsistent values is an error, and only the variables actually set are named. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bumpStep throttle was written for the 32 KB Write path, where one SetWriteDeadline per chunk would be wasteful. Applying it to ReadFrom slices buys nothing — a slice is already bounded at 4 MiB — and costs correctness: a slice completing less than a step after the last bump got no refresh, so the next one started with as little as window-step remaining. The real guaranteed floor was ~203 kbit/s, not the 186 kbit/s the constant and both design documents promise, and a client sustaining the documented rate was reaped as stalled. Slices now bump unconditionally, before the first as well as between each, which is what the pre-CopyChunked loop did. Costs at most one syscall per 4 MiB. The existing deadline tests construct the writer with step=0 and so never exercised the throttle; the two added here fail on the unfixed code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
meteredResponseWriter previously hid io.ReaderFrom on purpose, so every byte reached egressMeter.Add through a ~32 KB Write. Forwarding ReadFrom restored sendfile but moved crediting to once per completed 4 MiB slice, which a 200-500 kbit/s direct-play viewer takes 60-170 s to fill. RateKbps averages over 60 s, so those streams read as zero for most samples: /api/v1/status under-reports committed egress and nodepool's effectiveEgressKbps can admit sessions onto a saturated proxy. Metered slices are now 256 KiB — a credit every 4-10 s at those rates, well inside the window, and still 8x more per sendfile call than the Write path it replaced. Slice size here is a rate-fidelity constraint, not a tuning knob. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
handleDirectDownload passed the raw ResponseWriter to ServeDirect, so unlike the
sibling /downloads/{id}/file it had no rolling deadline and the API server's
absolute 120 s WriteTimeout truncated any original large enough to take longer.
Excluding the route from compression made it one unbounded sendfile, so the
whole body now rides on that single deadline.
redirectDirectDownload hardcoded an empty profile id in both the proxy redirect
and the telemetry attach, while the local branch two lines away reads the real
one. Proxy-served traffic was therefore missing from per-profile attribution in
telemetry, in the stream token claim and in the node session.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…plit conflicts Transfers were one record per HTTP request keyed by observation id, so ranged byte routes — audiobook file reads, download resumes, ebook fetches — could exhaust MaxTransfers within one retention window while RequestCount, the field that exists to count exactly this, stayed pinned at 1. A transfer is now one subject pouring one file over one route, and overlapping requests fold into it. A delta publish rewrites only changed fields and assumed the Redis hash still held the rest. An eviction, an out-of-band DEL, a replica failover or a lapsed PExpire drops it with no error, leaving under-reported sessions for up to FullResyncEvery publishes. An HLEN inside the same transaction now catches the mismatch and forces the next publish full, self-healing in one sweep. recordConflicts appended started_at_replaced without setting hasIdentityConflict, so the exported flag could disagree with the exported list. A pure authority upgrade that confirms the recorded instant now records nothing at all — it is not a conflict and should never have consumed the budget — and a replacement that moves the value sets both. Also documents two limitations rather than half-fixing them: clock skew is only detectable for a publisher running ahead, since the roster score is the publisher's own clock; and observedWriter.ReadFrom samples the cut flag once, which the enforcement change that first calls cut.Store has to make uniform across h1 and h2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nine ResponseWriter wrappers across five packages hand-rolled the same tail: assert the inner writer's io.ReaderFrom, CopyChunked through it, fall back to io.Copy over WriterOnly. Because io.Copy finds ReaderFrom by direct assertion and never through Unwrap, this forwarding is mandatory on every media-route wrapper — so a fix to it had to be re-applied nine times and a missed site silently dropped to the fallback, losing zero-copy sendfile along with that wrapper's byte accounting. Behavior is unchanged; each call site keeps its own chunk size and record callback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four families built the same clientip-then-RemoteAddr fallback chain inline while streamtelemetry already had it unexported; a fix to it (IPv6 handling, say) would have had to land in four places or the families would report different viewer addresses into the same merged view. Exported as ViewerIP and adopted everywhere. envEnabled was the ninth independent "is this env var truthy" parser in the tree, each accepting slightly different spellings. Adds internal/envutil and adopts it in both telemetry packages; the remaining copies should migrate as the code around them is touched. checkVersion re-parsed every record into a throwaway header struct before unmarshalling it again into a wire type that already carries the version, so a merged-view rebuild — measured at ~347 ms for 50 000 sessions, nearly all decode — did the JSON work twice. ConfigFromEnv ran twice at startup because the view cache re-read the environment just to get ViewTTL, logging any invalid variable twice; it now takes the TTL off the registry that already parsed it. playbackClientInfoFromRequest wrapped PlaybackClientInfoFromRequest wrapped playback.ClientInfoFromRequest — three names, one body. Callers now use the playback package directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md requires a docs/*-api.md entry and a changelog entry for a client-visible API change. No admin-API document existed — the ~20 sibling routes in the same router block are undocumented too — so this adds one, scoped honestly to what it covers, with the full response shape for GET /api/v1/admin/stream-telemetry/parity and the caveats an operator needs to read a report correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Review fixes — 10 commits,
|
| Finding | Commit | What changed |
|---|---|---|
| ABS clientip defeats the login rate limiter | 97b9833 |
clientip.Middleware now preserves the pre-overwrite transport peer in the request context, and the limiter keys on that. Unmounting the middleware would have given back the wrong viewer IP, which is what it was mounted for. |
jellycompat keyed on play.ID |
ef565d8 |
Compat attaches only under UpstreamSessionID. A play session doesn't learn that id until ensureUpstreamPlayback/ensureTranscodeManifest runs, so the pre-side-effect attach is a no-op on a session's first request and the handler attaches again the moment the id exists — still before any byte is written. A provisional key was rejected deliberately: it recreates the same ghost session, and a session whose id didn't exist a moment ago can't have a pending cut against it. |
Sticky truncated |
9bd8c31 |
Decays over Freshness, the same horizon BuildGlobalView uses to decide a publisher is current. The monotonic Dropped* counters keep the permanent record. |
SetRealtimeConnection dropped before attach |
9bd8c31 |
State for an unknown session is held in a bounded per-shard map, applied when an attach creates the session, and pruned by the sweep. |
| One env var disables distributed mode | 9bd8c31 |
Repair-then-validate against resolved values: a knob left at its default moves to satisfy the invariant, and only a pair the operator actually pinned to inconsistent values is an error — naming only the variables they set. |
| Documented 186 kbit/s floor is really ~203 | 6ec97c1 |
The bumpStep throttle was written for the 32 KB Write path; a ReadFrom slice is already bounded at 4 MiB, so throttling around one bought nothing and cost the documented floor. Slices now bump unconditionally, including before the first — which also closes the separate "no pre-first-slice bump" finding. |
| Proxy egress credited once per 4 MiB | 5a9e6e7 |
Metered slices are 256 KiB: a credit every 4-10 s for a 200-500 kbit/s viewer, well inside the 60 s rate window, and still 8x more per sendfile call than the Write path it replaced. |
| direct-download: no rolling deadline; empty profile id | ff19ab6 |
Both fixed. |
Ranged requests exhaust MaxTransfers |
fc46ad8 |
A transfer is now one subject pouring one file over one route, not one HTTP request. RequestCount finally means what it says. |
| Delta publish trusts the local digest map | fc46ad8 |
An HLEN inside the same transaction catches a reconstructed-from-delta key and forces the next publish full — self-heals in one sweep instead of up to FullResyncEvery. |
started_at_replaced appended without the flag |
fc46ad8 |
A pure authority upgrade that confirms the recorded instant now records nothing at all (it isn't a conflict and shouldn't have consumed the budget); a replacement that moves the value sets both the flag and the list. |
Documented rather than changed
- Clock skew is one-directional. Correct, and not fixable from one sample: the roster
score is the publisher's ownCapturedAt, so heartbeat and snapshot drift together
and there is no independent clock to compare against. A publisher running behind is
genuinely indistinguishable from one that stalled.PublisherStatusalready exports
EpochandSequenceso two successive parity reads resolve it; that's now stated at
the check. observedWriter.ReadFromsamplescutonce. Latent — nothing callscut.Store.
Left deliberately rather than half-fixed: a per-slice check would still act at 4 MiB
granularity, so h1 and h2 would still disagree, just less visibly. Written up as a
requirement on the enforcement change that first introduces a caller, including a test
that a cut behaves identically over both protocols.- Legacy tokens falling back to
iat. Verified already labeled —StartedAtSource
flows through tonodesessions.SessionInfoasstarted_at_sourceand sets
startedDegraded. No change.
Cleanups
813509d6 collapses all nine hand-rolled ReadFrom tails onto one
httpstream.ForwardReadFrom. f25ef9b5 exports streamtelemetry.ViewerIP and drops the
three inline copies, adds internal/envutil and adopts it in both telemetry packages,
decodes each wire record once instead of twice, stops parsing ConfigFromEnv twice at
startup, and collapses the three names for playback.ClientInfoFromRequest.
88451eeb adds docs/admin-api.md with the full parity response shape, plus a changelog
entry.
The one finding only half addressed: the media_routes scaffolding duplicated across
five packages. The viewer-IP half is deduplicated; the generic lookup/observe helper is
not, because a five-package refactor of the enrolment skeleton is disproportionate on a
PR this size. Worth a follow-up before a sixth family is added.
Three calls that go past what the findings asked
envutil.Truthyacceptsenabled, which the oldenvEnableddid not — so
SILO_STREAM_TELEMETRY_ENABLED=enabledandSILO_OTEL_ENABLED=enablednow turn on
where they were silently off. Unifying on the superset was the point of the finding,
but it is a live behavior change to two flags.- The config repair auto-lowers
SweepIntervalwhen onlyFRESHNESSis pinned low.
That means more Redis writes, not just different blame attribution. drop("pending realtime capacity exhausted")incrementsdroppedObservationsfor
something that is not an observation.
Verification
go build ./...,go vet ./...,gofmt -l ./cmd ./internal— cleango test ./...— the only failures areTestBeginWebOperationRecoversDeadProcessLock
andTestBeginWebOperationRejectsLiveProcessLock, confirmed to fail identically on
unmodifiedf6c8b04cin a separate worktree.TestResolveCopySeekAnchorMatchesRealLongGOPHEVC
passes here (this host has a newer ffmpeg than the one in the PR description).golangci-lint run --new-from-merge-base=origin/main— 0 issuesmake verify-local-paths— clean- The two new rolling-deadline tests were run against the base branch and fail there,
so they reproduce the defect rather than merely passing.
Still outstanding
The branch conflicts with main. One conflict, and it is not a code one: #675 deleted
docs/design/2026-07-09-streaming-write-deadline.md in the docs prune while this branch
modified it. The clean resolution is to accept the deletion and move the "Writer-chain
conformance" section this branch added into docs/architecture/ — the same
distill-then-delete pattern #675 itself used — which removes the conflict at its root
without merging main into the PR. Not done here; it is a call for the author.
AI-use disclosure
- Tool(s): Claude Code
- Model(s): claude-opus-5
- Involvement: AI-assisted. Every finding was read against the code before being acted
on; the verification commands above and their output are real and were executed. The
two new deadline tests were run against the unmodified base branch to confirm they fail
there, and the twoweb_component_test.gofailures were reproduced on the base branch
before being called pre-existing. - Adversarial review: my first plan for the jellycompat session-key fix was to add a
session-rekey path to the registry so a provisional attach could be promoted once the
upstream id appeared. Working through what that costs — reversing session reservations,
route activity and conflict-budget state under two locks on the hot path of a branch
whose merge argument is an 18-hour soak — it was the wrong trade for a first-request
artifact. Making the attach placement conditional on whether the id is already known
closes the finding completely with no registry changes at all.
PR #675 pruned docs/superpowers and the shipped design artifacts, distilling the durable content into docs/architecture first. The streaming write-deadline document was deleted on main under that rollup while this branch was extending its writer-chain conformance section, which is the whole of the conflict between the two. This carries the durable half forward on main's own pattern: the invariants a future change has to respect — the rolling-deadline contract, why slice size is a correctness constraint rather than a knob, the two rules every ResponseWriter wrapper on a media route must follow, the one-limiter sendfile trap, why chi's compressor is bypassed rather than repaired, and how conformance is actually verified. The one-shot half — the 2026-07-09 debugging session, the per-file application table, the rollout plan, the silo-apple follow-up list — goes with the deletion. Also records the two rules this branch's review turned up: the bump throttle belongs to Write and never to a ReadFrom slice, and the proxy egress meter has the same shape of constraint at a different value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolves the branch's only conflict with main and picks up the fix for the frontend typecheck failure that has been failing CI on this PR since 18 August (main's d285d0c; the failing file does not exist on this branch — CI builds the merge commit). Three resolutions: - docs/design/2026-07-09-streaming-write-deadline.md: accept main's deletion. #675 pruned the shipped design artifacts after distilling their durable content into docs/architecture; a1ba4d6 does that half for this document, and the stream-telemetry design's cross-reference now points there. - docs/feature-changelog.md: ordering only. Both sides kept, newest first. - internal/api/testdata/media_routes.txt: main added GET /api/v1/api-keys/scopes, which the per-family manifest guard correctly refused to accept unclassified. Verified against the handler — it writes a small JSON scope catalog and serves no media bytes — and pinned as non-media. This is the guard doing its job on a route that arrived from outside the branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Conflict resolved, CI green for the first time since 18 August
Worth stating plainly, because the PR description's verification section predates it: CI The three resolutions
It also picks up the two rules this review turned up: the bump throttle belongs to
Verification on the merged treeEvery CI step run locally before pushing, including the frontend ones the PR description
Review threadsAll 23 have per-finding replies; 22 are resolved. The one left open is the media_routes Three judgment calls from the earlier fixes are still worth a deliberate ack rather than What is leftNothing mechanical. Branch protection requires no approvals and no status checks, so this AI-use disclosure
|
Compose byte-level stream telemetry (#667) with tokenless header-authenticated playback (#712/#723). All seven conflicts were union-shaped; both features are kept intact: - cmd/silo/main.go: proxy gets the client-IP resolver and telemetry registry alongside the media-grant authority. - api/handlers/playback.go: PlaybackHandler carries ProxyGrantStore and NodeRecipeStore next to StreamTelemetry. - api/router.go: transcode routes keep observeNative wrapping under main's new bearer-capability semantics. - playback/recipecard.go: ToClaims projects DVProfile/AudioOnly and OriginalStartedAt together. - api/handlers/playback_v3.go: main's headerAuth/grant structure, with OriginalStartedAt stamped inside remoteTranscodeRecipeCardV3 so the grant and node copies of the recipe carry it too. - proxy/server.go: grants/loginSessions join clientIP/telemetry, and attachStream moves into serveDirectPlayClaims/serveRemuxClaims so the shared serving tails attribute bytes for both the token and grant routes. The proxy route manifest records the five new /stream/v3 routes as unclassified; enrolling them follows in the next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The merge left the five credential-free grant routes registered but
unclassified, so bytes served through authorized_media_origins_v1 were
invisible to stream telemetry. Enrol them:
- Declare GET+HEAD /stream/v3/{session_id} (playback), GET+HEAD
.../master.m3u8 (manifest) and GET .../segment/{name} (playback), all
viewer egress and capability-relevant, and wrap each registration in
observeProxy.
- Give them CanonicalSessionKey "verified_media_grant" rather than the
"verified_stream_token" the proxyRoute helper hardcodes. The field is
descriptive — it is only compared in sameDeclaration and emitted into the
route manifest, and no code branches on its value — but these routes prove
entitlement with a Redis grant plus the caller's own bearer token, never a
stream token, so labelling them otherwise would be false.
- Attach the viewer in relayGrantToTranscodeNode, the single path both grant
transcode handlers take. The proxy->node hop itself stays internal_relay.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…from redis Stream telemetry measured nothing on a fresh install: both switches were opt-in, so the parity comparison every P1 threshold depends on only ever ran where someone had already read the design doc. Observation is process-local, off the hot path and bounded, so the safer default is on. SILO_STREAM_TELEMETRY_ENABLED now defaults to true and is a per-process kill switch; SILO_STREAM_TELEMETRY_FAMILIES still narrows observation or drops one misbehaving family without losing the rest. SILO_STREAM_TELEMETRY_DISTRIBUTED is no longer a flag the operator has to keep in sync with their topology: unset, the mode follows whether Redis is configured, so a single-container install stays on LocalStore and a cluster merges. Setting it pins the mode either way, and a rejected distributed configuration pins it off so the derivation cannot re-enable exactly what was just refused. Both switches read a set-but-unparseable value as false rather than as the default (envutil.BoolDefault). For a default-on flag that means a typo in the kill switch turns telemetry OFF, which is the fail-safe direction: the operator was reaching for "stop observing", and a mistyped disable that quietly left the feature running is the failure that costs them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The staged per-family rollout set (native, proxy, transcode_node) is removed by owner decision: SILO_STREAM_TELEMETRY_FAMILIES left unset now observes all five declared families (native, jellycompat, proxy, abs, transcode_node) instead of a curated subset. The variable stays as a narrowing/kill knob — naming it takes families away rather than staging them in. Adds streamtelemetry.AllFamilies as the single canonical family list so ObservesFamily and ObservedFamilies don't hand-duplicate it, updates the design doc's family-gate section and env table to match present-tense behavior (keeping the original staged-rollout narrative as history), and updates the feature changelog to say every family is observed out of the box. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Ten commits: four fixes to the byte paths that had to be correct before anything could be
measured, five that build the measurement, one docs consolidation.
Part of #265. Supersedes and closes #306.
Problem
The legacy view answers "who told us they're watching?" Telemetry answers "who is
actually receiving video?" Those turned out to be very different questions.
The server has always tracked sessions by trusting what the app reports. An app says
"I'm at 42 minutes" every ten seconds, and the server takes that as proof someone is
watching. Nobody checked whether video was actually leaving the building.
That is also why a concurrency cap cannot tell a viewer from a ripper. It sees "one
session" either way: one person watching a film, or someone pulling the entire library at
link speed. And there is no single place to look — proxy health, admin sessions, node
sessions and playback stats each answer a different part of the question, from a different
store, with a different idea of what a "session" is.
Stream-telemetry measures the thing that costs money: bytes delivered. Putting the two
side by side for 18 hours is what exposed the gap — and it is not a rounding error:
video. One downloaded a single segment, then sent 5,324 position updates.
that real viewers get refused.
The operational consequence is that the numbers the server reports today cannot be
trusted for capacity decisions. Concurrent-stream counts, transcode-cap enforcement and
the admin "who's watching" view are all built on the legacy signal, and it over-reports.
Anyone sizing hardware or tuning the transcode cap from those numbers is working from
inflated figures, with no way to know by how much.
That specific defect is filed separately as #666 — it belongs to the old view, not to this
branch. But it is the clearest illustration of the problem: we were reporting numbers
that could not be checked, and they were wrong.
Why merge now
a hypothetical — it was still burning an encode slot while this PR was being written.
The parity report is what made those sessions distinguishable from ordinary timing skew
between stores; they showed as one-sided in 179 of 185 consecutive samples.
10 ms, max 52 ms across 183 samples under real load, with a Redis footprint of 2 keys.
This is not a bet on how it will behave in production — it has behaved, for 18 hours.
skew, zero restarts, and exactly one mismatch in the entire run — a transient
node-identity blip after a container recreate, understood and benign.
be checked against something, and the legacy view is the thing being corrected.
Telemetry is the only independent measurement available.
rippers and over-consumption, and it could not — not because the policy was wrong, but
because there was nothing underneath it to measure against. Everything enforcement
needs comes from here: per-session and per-user byte totals, delivery rate, viewer
addresses, and one merged view every process publishes into. Landing this is what makes
feat(playback): stream abuse control — authoritative monitoring + kill switch #306's goal buildable, and lets its thresholds be set from observed distributions
instead of guesses.
The honest framing is that this is not "no regressions after 18 hours". It is "18 hours
of production data, and the instrument earned its keep by catching something".
Four things were also quietly broken on the byte paths themselves, found while making
them measurable:
profile, so those streams were recorded against nobody and silently escaped the cap.
the proxy in front, not the person watching.
every byte took a slower route than it needed to.
after a replan.
Solution
What this does, and deliberately does not do
Every byte-serving route in every process now reports what it served, to whom, and how
fast, into one merged picture — asynchronously, off the hot path, without trusting
anything the client says. Five router families across three kinds of process publish into
Redis; a pure function merges them; an admin endpoint serves the result and diffs it
against the two projections admins read today.
It makes no decisions. Nothing is blocked, throttled, cut or banned. No existing admin
read has been repointed onto it. No
/api/v1response changed, no migration, no newPostgres write.
The four prerequisite fixes
826c71f6carries an immutable session creation time in stream tokens. The JWTiatcould not serve — signing overwrites registered claims on every mint, and replans mint
replacement tokens from a live session. Read order is explicit, and a missing timestamp
never invalidates an otherwise valid old token.
d5d52c4epopulatesUserID,ProfileIDandMediaFileIDin the compat proxy streamtoken (
buildProxyRedirectURL), which previously omitted them entirely. Cost: the tokengrows ~80–130 URL characters, and claims are signed but not encrypted.
82825336mountsclientip.Middlewareon the standalone proxy and ABS routers. Thisdeliberately changes recorded session IPs and
RemoteAddr-based logs to the resolvedviewer address.
760287d6repairs the writer chain. Becauseio.Copyfindsio.ReaderFromby directassertion and never through
Unwrap(), everyResponseWriterwrapper on a media routemust forward
ReadFromwhile preserving accounting. Seven wrappers were repaired overshared helpers in
internal/httpstream. The sendfile finding is the notable one — measuredwith
strace -f -e trace=sendfileover an 8 MiB body, the mounted proxy direct-play routerwent from 0 sendfile calls to 6. Slice size is a correctness constraint, not a tuning
knob: the deadline is an absolute time, so slice ÷ stall window is a hard floor on
sustained client rate. The old 64 MiB slice against a 180s window implied ~3 Mbit/s and was
reaping healthy slow clients; it is now 4 MiB (~186 kbit/s).
The measurement
a9d54b6badds process-local observation for native routes, with bounded retention andrelease-fold.
7a49cca1publishes snapshots to Redis and merges a global view withpublisher epoch/sequence and a
complete/degradedcontract.d332db9aand29083144enrol the remaining four families.
Enrolment is typed, not a hand-maintained list. Every media route is declared as a
MediaRoutecarrying family, method, pattern, class, role, session key and cap relevance;the wrapper is derived from the declaration, and a mount-site typo panics rather than
silently un-observing a route. A per-family manifest test walks the mounted routers and
diffs every
(method, pattern)against a checked-in golden — a new media route fails thebuild until it is classified. 94 of 1,003 route entries are observed; the other 909 are
pinned as non-media. This is the fix for the failure mode that sank the earlier attempt,
where an unenrolled byte path was both invisible and unkillable, rediscovered four separate
times.
Two rules worth not relitigating: viewer bytes and viewer IP belong exclusively to the
outermost viewer-facing edge (proxy→node hops are
internal_relay, never cap-relevant),and a transcode node publishes a correlation key and nothing else — it cannot know who
is watching, so it must never fall back to generic capture and record the proxy's address
as a viewer IP.
b6c9a1c7addsGET /api/v1/admin/stream-telemetry/parity, additive and behind the sameauthorization as
/admin/sessions. P0d was specified as "compare, then repoint". Thecomparison shipped; the repoint deliberately did not — the admin session payload is a
join of ~50 display fields telemetry is not canonical for, and the parity evidence now
argues against a blind swap anyway. The view is a read-driven TTL cache with single-flight
refresh, not a ticker:
BuildGlobalViewmeasures 347 ms at 50,000 sessions, so a tickerwould pay full rebuild cost on every server forever whether or not an admin is looking.
f6c8b04creduces ten documents (4,517 lines) to two, and scopes the working document towhat shipped. The enforcement and rules designs moved to the appendix — see below.
The design is
docs/design/2026-08-17-stream-telemetry.md; §6 carries the phase plan andwhat blocks P1, and
…-appendix.mdholds the deferred enforcement design along with theapproaches already tried and rejected. Sections 3 and 5 are deliberate stubs, not gaps —
Go comments cite section numbers directly, so they are not renumbered away.
Why monitoring alone, and enforcement deferred
Every threshold in an enforcement rule is a guess until the traffic has been measured,
and this is the thing that measures it. The enforcement design was written and reviewed
first, and that was the wrong order: it set numbers against a distribution nobody had seen.
This branch makes monitoring first-class; enforcement is built on top of it afterwards,
against real data. That is why #306 is being closed rather than merged.
The soak behind the claims above, in full:
view.completebuild_took_mssilo:stelem:*Rolled family-by-family — off →
native→+jellycompat— each held until quiet beforewidening. Live sessions resumed mid-stream across three container recreates (QSV transcode,
-c:v copydirect play, HLS all confirmed).Risk / follow-ups
code into every live byte path and can affect which optional interfaces are visible,
whether
io.CopyselectsReadFrom, flush timing, HEAD and Range behaviour, errorpropagation, connection reuse, and ABS socket.io if
Hijackeris not preserved. That riskhas now been exercised rather than only reasoned about, but it is the right lens for review.
nativeandjellycompatonly.proxyandtranscode_nodesaweffectively no traffic (single-node
MODE=integrated), and the multi-publisher merge wasnever exercised — every sample had exactly one publisher. Those rest on the per-family
manifest tests and the two-publisher real-Redis integration test, not on production
evidence. Worth stating plainly rather than letting "18 hours clean" imply more than it does.
absis enrolled but unexercised — the soak host has no audiobook traffic and neverhas. Code, manifest test and unit tests are in place; it stays off in the default family
set until somewhere real can exercise it. Low priority.
SILO_STREAM_TELEMETRY_FAMILIESstill defaults tonative,proxy,transcode_node; jellycompat and ABS share the API process, so defaultingthem on would widen instrumentation on upgrade alone. Now that native and jellycompat have
run in production, promoting jellycompat into
defaultObservedFamiliesis a reasonablefollow-up — deliberately not done in this PR.
wire-equal on bulk routes excluded from compression. Documented at each capture site; do
not "fix" it by moving the wrapper.
parity agreement over time. A single parity report samples three independently updated
stores and will always show one-sided differences.
projection surfaced. Fixing it needs an independent measurement to verify against, which
is an argument for landing this first.
Verification
gofmt -l ./cmd ./internal— cleango build ./...— OKgo vet ./...— cleango test ./...— 121 packages ok, 1 failure:TestResolveCopySeekAnchorMatchesRealLongGOPHEVC,which needs ffmpeg ≥5.x and the host has 4.4.2 (
Error splitting the argument list: Option not found). Pre-existing and environmental; the test exists unchanged onorigin/main.golangci-lint run --new-from-merge-base=origin/main— 0 issues (matches how CI runs it)make verify-local-paths— cleanapi,jellycompat,proxy,transcodenode,audiobooks/absTestRedisStoreIntegrationpasses against a real Redis, including the two-publisher casestrace -f -e trace=sendfileon a mounted proxy router, 8 MiB bodyon this host
AI-use disclosure
and were executed, not synthesized
coverage gap — it was reporting fewer sessions than the legacy store. Checking the request
logs reversed it: those sessions had no byte flow, so telemetry was correct and the legacy
store was wrong ([bug] Progress updates alone keep dead sessions alive forever — ghost sessions hold transcode slots for 15h with zero byte flow #666). I also went back and narrowed the soak claims after checking what
the run actually exercised rather than what it was configured for —
proxy,transcode_node,absand the multi-publisher merge saw no real traffic, so the Risksection states the coverage boundary rather than resting on the headline numbers.