feat(remote): meter the cache-fill window after the blob commits

What this merge request does

On the remote cache fill path a fill can commit its blob to content-addressable storage and then fail before it writes its cache row. The transaction rolls back, so no version row survives. A retry then deduplicates onto the blob that is already committed. The per-format emit is gated on if !entry.Deduplicated, so the retry charges the namespace no deduplicated bytes.

This merge request does not correct that behavior. It measures the window and leaves the remedy to a later decision. It adds one unlabeled Prometheus counter over the window. It corrects a comment in internal/remote/fetch.go that called the failure benign. It records the gap in two files under docs/dev/.

What remains after this merge request

A retried remote cache fill still charges the namespace zero deduplicated bytes. A reconciliation pass is still what corrects deduplicated_size_bytes.

Two precisions keep that statement exact.

The retry does charge the namespace. deltas.newVersions reaches EmitNamespaceCounters as deltaComponents, so components_count moves. Only deduplicated_size_bytes is withheld.

repositories.size_bytes is unaffected. The repository charge keys on whether a row of that repository already referenced the digest. The failed attempt wrote no such row, so the retry charges the repository in full.

The counter

gitlab_artifact_registry_remote_cache_fill_post_blob_commit_failures_total is an unlabeled prometheus.Counter.

It counts entries into the window rather than bytes. A process death inside the window records nothing. The series is therefore a floor on how often a fill enters the window, rather than a measure of the bytes lost.

It carries no format label because no format is derivable at the site. cacheFillTee, Fetcher and FetchOptions carry no format field. A format dimension needs a format field on NewFetcher and on the four sites that construct a fetcher. internal/format/oci/remote_operations.go calls remote.NewFetcher directly. internal/format/maven/remote_operations.go, internal/format/maven/remote_artifact.go and internal/format/npm/npmremote/flight_registry.go reach it through remote.NewSingleFlight. The site is format-agnostic, so this one counter covers the npm, Maven and Container remote arms.

The shape follows a landed analogue, packumentRebuildOrphanedBlobs in internal/format/npm/metrics.go. That counter counts rebuilds that committed a blob and then failed to stamp its row. It is an unlabeled counter with a one-line doc comment and the operator detail in Help, and internal/format/npm/packument_cache.go increments it in the post-commit error arm.

What changed

File group	Change
internal/remote/metrics.go	Declares cacheFillPostBlobCommitFailures and registers it. RegisterMetrics now ranges over a two-element collector slice instead of making one reg.Register call
internal/remote/fetch.go	Increments the counter in the post-commit error arm of (*cacheFillTee).completeFill, and replaces the comment that called the window benign
internal/remote/export_test.go	Adds CacheFillPostBlobCommitFailuresCount, a test-only reader for the package-global counter
internal/remote/metrics_test.go, internal/remote/fetch_test.go	Add the scrape-path test and the delta assertion that the coverage table names. TestFetcher_Fetch_CommitError_SurfacesFromReadAndNeverCancels gets the matching negative assertion, and it drops t.Parallel() under a //nolint:paralleltest waiver, because a parallel sibling moves the same package-global counter
docs/dev/observability.md, docs/dev/storage-accounting.md	Add the catalog rows, the two remote-grid rows, and the ## Known gaps row

Acceptance coverage

Line citations are left out on purpose. The line numbers moved twice on this branch and move again at the rebase, so each row names the test instead.

#	Acceptance item (issue card 1087)	Coverage
A1	The fill commits its blob exactly once, never cancels the session, and calls UpsertCacheEntry exactly once. That sequence is what the counter counts	Already asserted and unchanged, in TestFetcher_Fetch_UpsertCacheEntryError_SurfacesFromReadPostEOF
A2	The failed attempt issues no storage-accounting delta, and increments the new Prometheus counter exactly once	Counter half: the require.InDelta assertion in TestFetcher_Fetch_UpsertCacheEntryError_SurfacesFromReadPostEOF. Emit half is structural rather than asserted here — the emit sits inside the per-format UpsertCacheEntry that returned the error, and no format package changes
A3	A retry of the same fill charges the repository in full and charges the namespace no deduplicated bytes	Not test-assertable in this merge request: no format code changes. It is a documentation-accuracy item, carried by the ## Known gaps caveat's wording
A4	A reconciliation pass after A3 raises deduplicated_size_bytes by the blob's byte length, so the fast path and NamespaceReconcileStore.RecomputeDeduplicatedSizeBytes disagree	Not test-assertable in this merge request, same reason. Documentation-accuracy item
A5	No file under internal/format/ changes, so the Maven and npm predicate tables and both TestEmitCommittedFillCounters_ScopeSplit tests are untouched and stay green	Diff-checkable and checked: git diff --name-only origin/main...HEAD lists no internal/format/ path
A6	The counter moves by exactly one over a fill whose Commit succeeded and whose UpsertCacheEntry failed; the fetch.go comment stops calling the window benign without naming the accounting half; ## Known gaps carries a row and both live remote grids point at it; docs/dev/observability.md carries the catalog row	Counter half test-assertable and covered by TestFetcher_Fetch_UpsertCacheEntryError_SurfacesFromReadPostEOF for the delta and TestRegisterMetrics_ExposesCounterOnScrapePath for the scrape path and the counter type. The three review-checkable halves belong to the implementation and documentation passes
A7	The merge request does not claim to fix the defect. The description says the counter measures the window and that deduplicated_size_bytes still reads low on a retried fill until reconciliation	Review-checkable, merge request description only. No test

Test-first authorship

The tests landed first, in commit dc36dd194. They failed on behavior rather than on a compile error. TestRegisterMetrics_ExposesCounterOnScrapePath failed with no metric family named ... was gathered. The delta assertion failed with Max difference between 1 and 0 allowed is 0, but difference was 1. The three pre-existing assertions in the same test kept passing. That is what shows the branch was reached and only the meter was missing.

The sha dc36dd194 is local-only, and no ref reaches it. A squash removed the commit before the first push, so it never left the authoring checkout and a git fetch cannot get it. This section stays because it is the only record of the one permitted --no-verify hook bypass on this branch.

Runtime evidence

The new counter was observed live at 0 on a fresh boot.

A plain scrape does not confirm the sibling gitlab_artifact_registry_remote_detached_cache_bumps_total. A CounterVec with no materialized children contributes no family at all to Gather. It was confirmed instead by driving a real npm remote cache hit. That is what proves the change from a single reg.Register call to a range over a two-element collector slice dropped nothing.

Notes for the reviewer

Why the feat commit carries comment-only edits to three test files

scripts/ci/check-comment-caps.sh is pinned to --base origin/main. It therefore grades the whole branch diff rather than the commit being made. The first hooked commit inherited seven over-cap comment blocks that the test-first commit's one authorized --no-verify had left ungraded. A separate compression commit was unreachable, because pre-commit stashes unstaged work and that commit would have met go-test with the implementation absent.

The commits this section names are the same local-only history that ## Test-first authorship records. The squash removed them from every ref before the first push, so a reviewer cannot fetch them.

The commit type

The commit type is feat while the issue carries type::bug. That divergence is deliberate. The issue's type describes the defect, and the commit type describes what this change does. This change adds a metric and changes no behavior. That is the shape of three landed feat commits which add metrics and nothing else: a5ef4d2c0, 3c174720a and e84b3297d.

Two catalog rows in docs/dev/observability.md

The first row is this merge request's counter. The second is a backfill for the already-registered sibling gitlab_artifact_registry_remote_detached_cache_bumps_total, which was absent from the catalog.

The two adjacent ## Known gaps rows are not in conflict

The new row says the counter measures the window on every remote arm. The #834 (closed) row directly below says the container remote fill emits nothing. Both are true, because they are about different things. The new row's claim is about the counter, whose site carries no format and which the container arm reaches through internal/format/oci/remote_operations.go into remote.NewFetcher. The #834 (closed) row is about a missing emit.

The new row now carries that distinction. While the container arm emits nothing, a container increment stands for a window with no marginal deduplicated-bytes loss behind it. Once that arm emits, every increment carries the same loss.

Why the Container remote grid carries no pointer

Work item #1120 reserves that block while merge request !2262 (merged) is open.

Merge order in ## Known gaps

!2210 (merged) has merged. Its merge commit is bd47ca0b1, which is the base this branch now sits on. It rewrote the #910 row directly above the row this branch adds, so the rebase onto it resolved a two-row conflict in that table: main's rewritten #910 row is kept and this branch's #1087 row sits under it.

Five open merge requests still change that section. This table gives the verdict git merge-tree returns for each one against this branch at 7575e913d, re-measured after the rebase.

Merge request	Verdict
!2262 (merged)	Conflicts
!2232 (merged)	Conflicts
!2270 (merged)	Clean
!2269 (merged)	Clean
!2039 (merged)	Clean

Both conflicts are in docs/dev/storage-accounting.md and are textual rather than semantic. Whichever merge request lands second resolves them, and that merge request owns the resolution.

!2232 was clean against the pre-rebase base and conflicts against this one, so a verdict in this table is a claim about the base it was measured at rather than a standing property of the merge request.

Merge order in docs/dev/observability.md

This merge request and !2283 (merged) both edit docs/dev/observability.md. Whichever of the two lands second resolves the conflict, and that merge request owns the resolution. No pipeline reports this, because a conflict between two open merge requests appears only when the second one rebases.

docs/testing/

No scenario is added, and no scenario is affected. This merge request adds one Prometheus counter and corrects documentation. It changes no request, no response and no stored row, so every scenario in docs/testing/ keeps its expected result.

The duplicate-registration test reaches the first collector only

RegisterMetrics returns on the first reg.Register error. detachedBumpsTotal is the first element of the slice, so TestRegisterMetrics_RejectsDuplicateRegistration trips on it and the new counter's error arm is never offered. This is left as it is, because it is the shape every registrar in the tree has. internal/authz, internal/storage, internal/format/oci, internal/format/npm and internal/accounting each loop over a collector slice, and each duplicate test reaches the first element only, over 2, 18, 18, 25 and 5 collectors. docs/specs/S03-b-metrics.md puts the obligation on the registration helper rather than on each collector. The new counter is covered positively, by TestRegisterMetrics_ExposesCounterOnScrapePath and by TestRegisterMetrics_PassesCardinalityAudit. A comment pinning the slice order, which is what internal/format/npm/metrics.go carries, does not fit here: scripts/ci/check-comment-caps.sh counts the exported doc comment and the body head as one block against a three-line cap, so two added lines would mean deleting a true claim from the doc comment.

The composition-root comment is left as it stands

cmd/artifact-registry/main.go is not in this diff. Its comment at the remote.RegisterMetrics call site names detachedBumpsTotal alone, and RegisterMetrics now registers two collectors. The comment stays true, so the rule that a comment must describe the code as the same change leaves it does not fire: this change deletes nothing it cites and contradicts nothing it claims. Naming one collector of several is what the other blocks in registerServiceMetrics already do, at wider ratios: the npm block names one of 25, the Maven block one of four. dc69b2649, the npm meter this counter is modelled on, added publishPostBlobCommitFailures to an already-wired registrar and left main.go untouched. That block is five lines and the cap for a function-body block is two, so an edit there compresses three true clauses into two lines to add one.

What issue #1087's body gets wrong once this merges

The issue is not edited here. Two of its sentences stop being true when this lands.

## Where it applies says "None of the three remote grids in docs/dev/storage-accounting.md records the window today". After this merge that holds for the Container remote grid alone. The npm and Maven remote grids each carry a row.

## What closes it option 3 reads "Accept it and document it. This is what the code does today, undocumented". This merge request does the documenting half and not the accepting half. The choice between options 1 and 2 is what the ## Known gaps row still tracks against #1087.

Older than this branch, and unrelated to it: the ## Where it applies bullet cites internal/format/oci/remote_fill_emit.go:45. That path is on neither main nor this branch. It arrives with !2262 (merged).

Diff size

The diff is 132 reviewable lines, which is less than the 500-line threshold that asks for a justification. The count is insertions plus deletions, from git diff --stat origin/main...HEAD. The split is:

• 47 lines of production Go, in internal/remote/metrics.go and internal/remote/fetch.go.
• 76 lines of test Go, in the three internal/remote/*_test.go files.
• 9 lines of documentation, in docs/dev/observability.md and docs/dev/storage-accounting.md.

Related to #1087

Related to rather than Closes, because the merge request meters the window and does not close the defect. A retried remote cache fill still charges the namespace zero deduplicated bytes, and reconciliation is still what corrects deduplicated_size_bytes.