feat(npm): count a hosted fill's mid-body deadline kill (npm-hosted-fill-body-write-deadline plan: 1/2)
What
Step 1 of docs/plans/2026-08-31-npm-hosted-fill-body-write-deadline.md.
A hosted npm metadata fill writes its body under the 65 s instant
armInlineBuildResponseDeadline arms. Until now a deadline firing inside that
write was indistinguishable from a client hanging up: one Warn naming a
disconnect, no counter and no outcome code, so a truncated 200 counted as a
success on npm_request_total.
This makes the ending attributable and countable:
armInlineBuildResponseDeadlinereturns the instant it armed, and the zerotime.Timeplus aninlineBuildDeadlineArmFailuresincrement when the arm fails.buildInlinerecords that instant onHostedDocument.WriteDeadlinefor every fill-arm delivery, the redirect and the304included, and observes the route's document-size histogram on every fill-arm delivery carrying a size.- Both hosted
streamDocumentarms wrap the cache blob insourceReadTrackerwhen an instant was armed, then classify the ending.
Five new label-free metric families, all hosted-path only:
..._npm_packument_inline_build_deadline_kills_total,
..._npm_disttags_inline_build_deadline_kills_total,
..._npm_inline_build_deadline_arm_failures_total,
..._npm_packument_inline_build_document_bytes and
..._npm_disttags_inline_build_document_bytes.
What each ending books
| Ending | Counter | Outcome code | Log |
|---|---|---|---|
| The armed instant elapsed mid-body | the route's kill counter | internal_server_error |
Error, rationed per repository per minute |
| The client disconnected | none | none | Warn |
| The cache blob's own read faulted | none | internal_server_error |
Error |
| Any other failed write on a clean read | none | none | Warn |
inlineBuildWriteDeadlineKill is clock-only. It drops
remoteTarballWriteDeadlineKill's net.Error fallback rather than guarding
its zero-deadline entry, because a zero instant here means a warm read armed
nothing, and a warm read stalling past server.timeouts.write is exactly the
shape that fallback matches. packument_cache.md carries the departure, the
four blind spots it opens and how each is closed or bounded, and links #913.
metrics.md carries what the five families count, the population each
histogram observes, and the bucket derivation.
Review findings fixed in this MR
A branch review found one blocking defect in the first four commits, fixed in
29fa86ea0 before this MR opened. Both classifiers had introduced readErr
into the decision without the context.Canceled guard that
recordCopyOutcome (download.go) and recordRemoteTarballCopy both carry.
The cache blob is opened with the request context, net/http cancels that
context when the client goes away, and io.Copy reads before it writes, so a
plain client disconnect on a cache-miss delivery was booked as
internal_server_error with an unrationed Error line naming storage. The
mirrored suites already pinned that ending; this suite had dropped it, which
is why it shipped green. assertClientDisconnectIsChurn now pins it on both
routes at both clock states and is mutation-checked.
The same commit corrects five documentation claims a reader acts on: the
written-bytes arithmetic (borrowed from the ErrContentLength mechanism the
same paragraph excludes), the histograms' population under collapse, a #269
citation that points at unrelated container work, the log volume of the
unrationed records, and the dist-tags histogram's saturation of the first
bucket.
Deviation from the plan
This MR reverses one ruling of the merged plan, deliberately and in one place.
The plan's ### The observability table does not carry the inline-build families concluded:
no
observability.mdrow is added, because adding rows for these while the four existing ones stay out would read as coverage the table does not have.
The five rows are added anyway. The asymmetry the plan names is real: the four
pre-existing families
(packument_inline_build_collapsed_total,
disttags_inline_build_collapsed_total,
inline_build_capacity_exceeded_total, inline_build_timed_out_total) are
still absent from that table. The answer is to backfill those four, which is a
docs change worth its own MR, rather than to withhold accurate rows for five
new families whose proxy-side twins already sit in the same table. Raising it
here so the reversal is reviewed rather than discovered.
Open question for step 2
The plan's Files: entry writes the last bucket edge as 477 MiB while its
## Research Findings calls 477 MiB an approximation of the exact product
npm.max_package_json_size x npm.max_versions_per_package = 500,000,000
bytes. The two readings differ by 170,752 bytes. This MR ships 500_000_000,
the "configured ceiling" reading, and
TestInlineBuildDocumentBytesBuckets_BracketTheBudgetDerivation pins it. Step
2 reads this edge for its size-derived budget, so flip the literal and correct
the plan text if the binary 477 << 20 was meant.
Diff size
1524 reviewable LOC (added + removed against origin/main), past the 500 in
docs/dev/development-model.md.
Splitting would not help review it. The production core is 275 LOC and is one
change: the instant has to be returned, carried and classified together, and
each half is unreachable without the other.
| Group | LOC | Files |
|---|---|---|
| Production Go | 275 | metrics.go 68, packument_get.go 58, disttags.go 56, inline_build.go 47, hosted_document_read.go 24, remote_tarball_kill_log.go 22 |
| Tests | 915 | packument_get_test.go 370, hosted_document_read_internal_test.go 256, inline_build_internal_test.go 95, disttags_test.go 75, export_test.go 49, metrics_test.go 37, inline_build_deadline_test.go 33 |
| Docs | 334 | metrics.md 232, packument_cache.md 93, observability.md 9 |
Merge order
Depends on !2256 (merged) (docs(plans): reverse the ruling against observability.md rows). It amends the plan ruling the section above reverses, so it must merge first; a hard merge block is set on this MR for it.
Three open MRs of mine append to files this one appends to, and whichever merges second resolves the conflict. No pipeline reports it, so it is recorded here:
- !2197 (merged) overlaps on five files:
docs/dev/observability.md,internal/format/npm/metrics.go,metrics.md,metrics_test.goandexport_test.go. This is the one that matters, because it appends to the samenpmCollectorslist and the same expected-name list. - !2204 (merged) and !2193 (merged) overlap on
docs/dev/observability.mdandinternal/format/npm/metrics.md.
Testing
go test ./internal/format/npm/passes.golangci-lint runat the pinned 2.13 reports 0 issues for the package.- No conformance run is owed: no route, status code, header, response body or error code changes, so no npm protocol behaviour moves. The same bytes go out under the same committed status.
- No e2e scenario in
docs/testing/is added or affected, for the same reason: every ending here is past a committed200and is visible only in metrics and logs, which the scenario catalogs do not assert.
Known gap, not fixed here
remoteTarballKillLog's type doc still names RemoteTarballHandler as the
owner and recordRemoteTarballCopy as the writer. Two more owners and two
more writers have made that an incomplete list. Correcting it means touching a
35-line block whose cap is one line, so the prose has to move to a sidecar
that does not exist yet, dragging tarball-owned text into an npm-hosted MR.
The file header this branch already rewrote names both windows, so a reader of
the file is not misled.
Related to #952 (closed)