feat(npm): mount the remote dist-tags proxy and meter it

🎯 What this MR does

Fills the npm dispatcher's kind=remote dist-tags slot, so GET/HEAD /{slug}/npm/{repo}/-/package/{package}/dist-tags on a kind=2 repository stops answering 501 and serves the proxy. Step 11c of a 3-MR stack, and the MR that makes the route reachable.

📚 The stack:

Step MR What it delivers
11a !1673 (closed) npm.RemoteFlights + ServeResult.BlobRef
11b !1674 (merged) npm.RemoteDistTagsHandler and its suite
11c · this MR 👉 !1675 (closed) Composition root: mount, metric label, docs

🧩 The three pieces

1️⃣ Wiring

The shared upstream doer, the two resolution knobs, and the upstream request budget travel from wireRemote to buildNpmDispatcher in a new npmRemoteDeps. Its zero value is the "no remote foundation in this process" state, which leaves every kind=remote read on its 501 stub: the isolated wireNPM boot seam and the DB-less unit stub both run in it, and buildNpmRemoteDispatchOptions sets no slot for it.

One npm.RemoteFlights is built here, once per mount, and every slot added later takes that same value. A handler composing its own would coalesce nothing and give its route a private copy of max_concurrent_fills_per_repo.

The handler's write-deadline budget is derived here rather than fixed in the format package: request_total_timeout + single_flight_wait_timeout + a grace. The two waits add rather than overlap — a follower that joins as a leader starts pays both — and deriving it means raising either knob cannot silently leave the deadline too short.

2️⃣ Instrumentation

WithRemoteDistTagsHandler wraps the handler it is given in the instrument seam under a new remote_disttags_get label. Without it the first production kind=remote route would emit neither the npm request metric nor the completion log line its hosted twin on the same URL emits, and WriteError's recordOutcomeCode would no-op, so even the outcome code would be lost. The seam and its handlerLabel values are unexported, so a composition root cannot apply them — the option does, and handler.go's contract comment now says that rather than assigning it to the caller.

The label is its own value rather than sharing disttags_get. The two answer the same URL but have unrelated latency distributions: a hosted dist-tags read is a keyed cache lookup, a remote one can run as long as the upstream fetch and the coalescing wait allow, so one histogram would make the hosted percentiles unreadable. The handler cardinality budget moves 9 → 10 to match, which TestRegisterMetrics_PassesCardinalityAudit enforces.

3️⃣ Configuration and the run recipe

npm.public_registry_url is boot-known but was announcing itself only as one 500 per request, forever. logNpmRemotePublicRegistryURLGap emits one breadcrumb at boot when a remote slot is mounted without it.

The run recipe's remote-npm notes are narrowed to what is actually served. The previous text claimed any kind=remote read fails with 500 while the base URL is unset; in fact only the dist-tags slot has a handler — the packument and tarball routes answer 501 regardless — and no API creates a kind=remote repository at all: cmd_seed posts kind=hosted and the management API rejects anything else with 422. The documented symptom was unreachable through the driver, which is the recipe/reality disagreement guardrail 21 asks be surfaced rather than worked around.

🔎 Review feedback

Finding Outcome
Duo + AppSec: logNpmRemotePublicRegistryURLGap takes no context.Context, unlike every peer breadcrumb Fixed. Takes ctx and logs through WarnContext, so the record carries the startup trace and correlates with the 500s that follow. wireServices already had ctx in scope.
AppSec: inline message string, no named constant, no pinning test Fixed. npmRemotePublicRegistryURLGapMsg is named beside the emitter for the reason credentialAuditSuppressedMsg is, and TestLogNpmRemotePublicRegistryURLGap asserts against the constant rather than a literal copy, so a reword breaks the emitter and the test together instead of silently invalidating a runbook.
AppSec: prose-only record, no structured fields Fixed. Carries config_field and affected_route_kind, so alerting can filter on the field to set rather than substring-matching the sentence.
Duo: deps.httpClient never asserted against real wireRemote output Fixed. The real-wiring subtest now asserts httpClient and that the derived serveDeadline() is positive, so a regression that drops or mis-sources the field fails here rather than shipping an unarmable deadline.

Verification

  • go build ./... and go vet ./...: clean.
  • go test -race -count=1 ./cmd/artifact-registry/... ./internal/format/npm/... ./internal/remote/... ./internal/metrics/... ./internal/config/...: all green.
  • golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 ./cmd/artifact-registry/... ./internal/format/npm/...: 0 findings on any line this MR adds or changes. It reports 8 in wire_npm_boot_integration_test.go and wire_npm_observability_integration_test.go; all 8 are present on main, the first file is untouched here, and the change to the second is a single call-site update.
  • .claude/skills/run-artifact-registry/driver.sh smoke: 22 passed, 0 failed, on this branch.

📏 Diff size

697 LOC, past the 500 reviewable-LOC line in docs/dev/development-model.md. Split by file group:

Group Files LOC
Production wire_npm.go +214, metrics.go +48, handler.go +26, internal/config/npm.go +24, wire.go +10, wire_root_dispatcher.go +9, observe.go +2, internal/metrics/cardinality.go +2 335
Tests wire_npm_test.go +179, metrics_test.go +9, four wire_* call-site updates +9 197
Docs and skill plan +122, SKILL.md +26, driver.sh +22, configuration reference +4 174

Docs are a quarter of it and the production half is the mount itself. Splitting the metric label from the mount would land a label nothing emits under; splitting the docs would leave the plan describing a stack whose parts are already open.

🧪 E2E scenario impact

None added or affected. docs/testing/e2e/npm.md scopes virtual and remote npm repositories out until the capability ships, and it is still not user-reachable: no API creates a kind=remote repository, so a scenario would have no way to set one up.

Related to #348 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading