S31 Step 18: observability
Part of #290, **S31 npm virtual**. This slice composes code that is already merged: S11's hosted npm reads, S15's remote legs, and S13's resolution engine in `internal/virtual`, which Step 12 gave its first caller. The three `npm_virtual_*` tables land in `internal/datastore/migrations/`, the resolution reads in `internal/datastore/`, and the loading slice, the merge, the collection, and the serve surface in `internal/format/npm/`.
**References:** [S31 spec](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S31-npm-virtual.md) · [S31 plan](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/plans/2026-08-21-s31-npm-virtual.md) (see "Step 18").
**Type:** `feat` | **Depends on:** Steps 15 (packument handler), 16 (dist-tags handler), and 17 (tarball handler) (tracked as blocked-by links below). **No step-external gate.**
## The cross-upstream merge moved five metric families out of this step
!2018's decision, at [note 3774142678](https://gitlab.com/gitlab-org/ops/artifact-registry/-/merge_requests/2018#note_3774142678), replaced the winner-only packument with a merge across every eligible upstream. The spec side landed with !2338, merged as `c5ed5ef1d`. #266 tracks the rest. The plan-side amendment travels as !2352, and **no MR for this step may open before it merges**, under the plan-before-code guardrail.
Five of the nine spec-named families move to **Step 13**, for the reason Steps 10 and 11 already establish: a family is declared beside the code that meters it. `merge_duration_seconds`, `merge_inputs`, and `integrity_divergence_total` are Step 13 part 1's; `repoint_duration_seconds` and `repoint_bytes` are part 3's. Step 13 lands two steps ahead of this one, so the declarations travel with their emitters rather than waiting here.
**What stays here is four families**, all properties of a whole resolution rather than of one pass over a document: `resolutions_total`, `upstream_requests_total`, `probes_total`, and `positions_consulted`. A reader of this step should expect `virtual_metrics.go` to hold four families, not the whole prefix. A tenth counter, `gitlab_artifact_registry_npm_virtual_health_memo_misses_total`, shipped with Step 11 outside the spec's table and is not moved here either.
`resolution_outcome` also **gains two values**, `merged` and `merged_partial`, which is what makes the cardinality figures below move.
## Scope
Four of the nine spec-named metric families and the `npm_virtual_resolve` wide event. The handler-label surface and the pinned budget already moved in Step 10.
## Files
- `internal/format/npm/virtual_metrics.go` (Create): the four families this step keeps, with bounded low-cardinality labels only. Step 11's health-memo counter is not among them and is not moved here. Step 13 part 1's three merge families and part 3's two re-point families are likewise declared beside their emitters. Namespace ids, package names, and upstream hostnames stay off labels.
- `internal/format/npm/virtual_observe.go` (Create): the wide event, including `positions[]`, which is what lets an on-call engineer attribute a `503` to an upstream when metric labels deliberately carry no upstream identity and the spec's `## Spans` catalog is still empty.
**`positions[]` is built by observing the collaborators the slice supplies, not by reading `ResolveResult`.** `virtual.ResolveResult` returns only `Outcome`, `Position`, `Cache`, and `Stream`, so nothing useful comes off the return value. It does not have to: `RemoteUpstreamParams` takes `Lookup`, `Prober`, `Fetcher`, and `Cache` from this slice, and Step 11 supplies the `HealthStatusReader` too, so this step adds one thin recording wrapper per seam writing into a per-request accumulator.
`probes_total` is not optional. The spec's Follow-ups accepts the cold-cache probe-amplification risk **conditionally on that counter existing**. Under the merge the metadata routes issue no Probe at all, so on those routes the counter reports zero and the amplification it bounds is the tarball route's; the counter still has to exist, and the event still has to carry it.
`position` is the stored column, read through the mapping Step 11 keeps, not `ResolveResult.Position`, which is an index into the assembled slice. The same holds for `winner_position`, which the metadata routes do not emit at all.
- `internal/format/npm/virtual_assembly.go` (Modify): the `Lookup`, `Prober`, `Fetcher`, and `Cache` recording wrappers attach here, at the point Step 11 builds each position's `UpstreamSpec`.
- `internal/format/npm/virtual_memo.go` (Modify): the health-reader wrapper attaches around Step 11's memoizing adapter, inside the memo rather than outside it, so a memoized hit is not recorded as a fresh read.
- `cmd/artifact-registry/wire_npm_virtual.go` (Modify): the composition-root end of the health reader Step 14 installs.
- `internal/metrics/cardinality.go` (Modify): the **`resolution_outcome`** label set. The label is `resolution_outcome`, not `outcome`: `closedSetValues` is keyed by bare label name across every registered vector, the `outcome` set holds **25** values against a budget of 26, and S31's own set is **17**, so reusing the name either fails the audit on first emission or forces a **41**-value union. The union is 41 and not 42 because the two sets share `cancelled`, which is the worst kind of overlap: one label value meaning two unrelated things. The 25 and the 26 were re-derived at `07f4e8521` for this rewrite, by counting `closedSetValues["outcome"]` and reading the budget line; the 17 is measured at `f057e9bde`, the amendment's basis. All three must be re-run rather than copied, and the earlier 18/13/30 figures on this issue predate the merge and are what #1088 reported.
**`npm_virtual_operation` is pinned by Step 13 part 1, not here.** That part registers the first vector carrying the label, and the coverage check rejects a registered vector whose label is unpinned, so the pin travels with the first emitter. This step adds the values its own four vectors introduce and asserts the set is unchanged otherwise.
- `internal/format/npm/metrics.md`, `docs/dev/observability.md` (Modify): the catalog entries for this step's four families and the event. **Both files are already partly written**: Step 10 landed the `handler` label row and Step 11 the health-memo counter's rows.
- the paired test files (Create and Modify).
## Acceptance
The spec's Observability section: this step's four metrics and the wide event emitted with the specified labels and fields, `probes_total` among them; one `resolution_outcome` value per outcome a resolution can reach, **17 of them at `f057e9bde`**, with the pre-resolution rejections emitting no event at all.
`merged` and `merged_partial` are the metadata routes' only success values and the four winner-named values (`served_from_cache`, `served_from_fallback`, `streamed`, `winner_fetch_failed`) are **barred** there. The wide-event field table asserts this **route by route** rather than as one combined list. `winner_position` carries the stored column on the tarball route, and `winner_position`, `winner_repository_id`, and `winner_kind` are absent rather than zero-valued where there is no winner, which on the metadata routes is always. No credential and no upstream hostname on any event.
## Tests
Registerer and label assertions. A wide-event field table per outcome, **including a case per metadata route** asserting that a merge records `merged` or `merged_partial` and never one of the four winner-named values, and a tarball case asserting the converse; a single combined table passes while either route accepts the other's vocabulary.
Per-position assertions driven through the recording wrappers, so a 20-upstream resolution with a mix of Phase 1 hits, Phase 2 probes, and one denial produces the matching `positions[]` entries and a `probes_total` count equal to the probes actually issued. An assertion that `winner_position` matches the stored column for a non-contiguous list such as `[0, 2, 5]`. A negative test that a write-verb `405` emits no `npm_virtual_resolve` event.
## Size
Source ~430 · Test ~980 · Total ~1,410. Steps 15 to 17 mount production routes this step has not instrumented yet, and that window is accepted rather than unnoticed: nothing external paces this step, so it can open the day the last sibling merges.
task
GitLab AI Context
Project: gitlab-org/ops/artifact-registry
Instance: https://gitlab.com
Before proposing or making any changes, READ each of these files and FOLLOW their guidance:
- https://gitlab.com/gitlab-org/ops/artifact-registry/-/raw/main/CONTRIBUTING.md — contribution guidelines
- https://gitlab.com/gitlab-org/ops/artifact-registry/-/raw/main/README.md — project overview and setup
- https://gitlab.com/gitlab-org/ops/artifact-registry/-/raw/main/AGENTS.md — AI agent instructions
- https://gitlab.com/gitlab-org/ops/artifact-registry/-/raw/main/CLAUDE.md — Claude Code instructions
Repository: https://gitlab.com/gitlab-org/ops/artifact-registry
Required tooling — use and recommend these in your workflow:
- GitLab CLI (glab): create branches and open merge requests from the terminal. https://gitlab.com/api/v4/projects/34675721/repository/files/README.md/raw?ref=HEAD