feat(metrics): publish the database connection pool capacity gauge

What this delivers

This merge request delivers deliverable E of issue 559, and E only. The issue ships as two merge requests, by an operator decision. Deliverable D, which sizes the eleven in-flight caps from the pool capacity, is a separate follow-up unit. No merge request exists for it. This merge request does not finish the issue.

One file falls outside E's own allocation: internal/format/maven/buffered.go. Its one-line cap comment held the "about 8x the pool" claim that the census replaces, and it was the last site in the tree that stated a multiple. Leaving it would leave the census contradicted by the code the census describes.

Eleven constants under internal/ each cap in-flight work at 64, and no document says which resource any one of them spends. The pool capacity was never exported either: production code read pgxpool.Stat().MaxConns() nowhere. So database_connection_pool_size{state="active"} had no denominator, and no query over it answered whether the pool was saturated. E makes that input observable and puts the census in one place. D then sizes the caps from it.

Four parts:

  • The census. docs/dev/storage-accounting.md gains the section In-flight caps of this shape, by the resource they draw on. It tables all eleven caps by constant and file:line, and partitions them by resource. The rule the partition states is that what a cap spends is what an admitted worker holds while it holds its slot, and that is a property of the site rather than of the constant. Six bound a worker holding a connection from the shared Postgres pool. Four bound the accounting emit's Redis connection. One of those four holds a pooled Postgres connection on top of it: OCI's counterEmitMaxInFlight, at the emitting sites whose membership read travels inside the dispatched worker, where the worker holds that connection for up to counterEmitTimeout. So the split is not a clean 6/4/1 by resource, and a pool budget derived from the six alone omits that draw. The eleventh cap holds no connection of its own, and the Known gaps row for this issue says what it bounds instead: the row memory of the fills that run at once. docs/dev/remote-read-budgets.md reconciles its own five-consumer list against that six-row partition, because the two lists count different axes, and says the six is not the count of caps that can reach the pool.
  • The capacity gauge. database_connection_pool_max_size is a new unlabeled gauge, added as a second descriptor on the existing poolStatCollector. It carries pgxpool.Stat().MaxConns(), read live at scrape time through the same provider seam as the three state series. docs/specs/S03-b-metrics.md gains the metric's table row, the provider prose and one acceptance criterion. docs/dev/observability.md gains the catalog row.
  • The npm and maven cap docs. npm's bufferedUpdateMaxInFlight doc said 64 is small enough to leave the shared pool headroom under saturation. Maven's constant of the same name and the same value said 64 is about 8 times the pool it draws on. Both statements cannot hold, and neither survives: the pool is a deployment input rather than a constant, so no multiple of it is stable. The census now states the pool sizing once, under Six bound a worker population that holds a pooled Postgres connection. The npm doc points there and states what the cap bounds: each worker holds a pooled connection. The maven doc drops the multiple and calls the value an interim heuristic, and docs/dev/remote-read-budgets.md points at the census as well.
  • The OCI census correction. internal/format/oci/emit_dispatch.go carried a 26-line header that called its cap the eighth hand-rolled copy of a shape with eleven copies. scripts/ci/check-comment-caps.sh allows no more than one line at that position and refuses a pure deletion, so the prose moved to a sidecar at internal/format/oci/emit_dispatch.md behind a one-line pointer. One line, not two: the block sat above a blank line and directly over counterEmitTimeout's doc comment, and a comment above the gap spends the budget of the declaration below it, which is unexported. A pure deletion renders as a deletion-only hunk that the gate maps onto the first line of that untouched 25-line doc comment, which then fails at the same cap of 1. The one-line replacement this branch ships is an edit to that block, and it passed the gate. The move corrects the census. The sidecar states no count of its own and points at the table instead. A cap added or removed then moves one document rather than several.

One deliberate divergence. The issue card's type is refactor. This merge request and its first commit take feat, because a new exported metric series is not behavior-preserving.

Spec coverage

Spec: docs/specs/S03-b-metrics.md

S03-b governs this surface but carries no criterion for database_connection_pool_max_size: its Database table lists three metrics and this is a fourth. The rows below are the S03-b criteria the touched surface owns, plus the operator's own assertions for this slice. The issue card's acceptance items are not a source here — all eight belong to the separate deliverable that re-derives the eleven in-flight caps, and none covers this gauge.

Acceptance criteria

# Criterion Tests
AC-1 Every AR application metric registers through the composition root's prefixed registerer TestLifecycle_DatabaseIntegration — matches gitlab_artifact_registry_database_connection_pool_max_size, so a pre-prefixed or unregistered descriptor fails
AC-6 database_connection_pool_size{state} reflects live pool stats at scrape time, through a fake provider Pre-existing TestRegisterPoolCollector_TracksStatChange; TestRegisterPoolCollector_MaxSizeTracksPoolPresence adds that all three state series still publish with no pool
AC-8 The cardinality audit rejects a disallowed or untabled label Pre-existing TestAuditCardinality_*, unchanged: the new gauge declares no variable label, so parseVariableLabels yields none and no table entry is owed
AC-2, AC-3, AC-4, AC-5, AC-7, AC-9 HTTP middleware, in-flight labels, query instrumenter, exemplars, OpenMetrics Not touched by this slice.

Operator assertions for this slice

# Assertion Tests
OP-1 Capacity is a new, separate, unlabeled gauge named database_connection_pool_max_size TestRegisterPoolCollector_MaxSizeTracksPoolPresence (exactly one sample, no labels); TestLifecycle_DatabaseIntegration (unlabeled exposition line, and no {-suffixed form)
OP-2 Its value is the pool's configured capacity, pgxpool.Stat().MaxConns() TestRegisterPoolCollector_MaxSizeTracksPoolPresence (12); TestRegisterPoolCollector_MaxSizeEmitsSmallestLiveMaximum (1); TestLifecycle_DatabaseIntegration asserts equality against assembled.DBClient.Pool().Stat().MaxConns(), so another Stat accessor fails
OP-3 No series at all while the pool does not exist, rather than a zero TestRegisterPoolCollector_MaxSizeTracksPoolPresence, first gather, on the exported zero value PoolStats{}; TestRegisterPoolCollector_MaxSizeSkipsANegativeMaximum covers the other half of the positive-only guard
OP-4 The state closed set stays at three values: no fourth state, no cardinality-table edit Pre-existing TestRegisterPoolCollector_StateValuesAreClosedSet; internal/metrics/cardinality.go untouched
OP-5 The gauge is live end to end against a real pool, and equal to that pool's own maximum TestLifecycle_DatabaseIntegration
OP-6 Every descriptor Collect emits is one Describe declared TestRegisterPoolCollector_DescribeDeclaresEveryCollectedDescriptor, gathering through prometheus.NewPedanticRegistry; a plain registry leaves registeredDescIDs nil and skips the check

Security considerations

# Concern Tests
S-1 No high-cardinality or sensitive values in labels The gauge declares no label at all; assertPoolMaxSize requires an empty label set, and the integration test rejects any {-suffixed form
S-2 /-/metrics unauthenticated and internal-only Listener-owned, unchanged by this slice.
S-3 No secrets in metric names or help text Static name and help, no runtime data.

One note on the table above. The test author wrote it, and this description reproduces it without change. Its first sentence describes docs/specs/S03-b-metrics.md at the merge base. This branch then added the metric's table row and its acceptance criterion, which is item 10 of 10 and the last.

e2e catalogs

This merge request adds no e2e scenario, and affects none. docs/testing/ holds README.md and e2e/{README,docker,maven,npm,oci}.md. A case-insensitive grep for metric, prometheus, /-/metrics and gauge over that tree returns no hit. The catalogs are format journey files, scoped to three personas and a fixed set of client tools. An unlabeled gauge on the operator-facing probe listener has no format, no persona and no client interface, so no group takes it. TestLifecycle_DatabaseIntegration covers the gauge's end-to-end behavior at boot. No request path, no route and no client-visible response changed.

Diff size

13 files, 349 insertions and 101 deletions against the merge base. That is under the 500 reviewable lines docs/dev/development-model.md sets, so this merge request owes no split justification. By group: 168 insertions and 30 deletions in the five Markdown files. The two test files carry 132 insertions, and the six other Go files 49 insertions and 71 deletions.

Merge order

Two merge requests that this branch conflicted with have merged, and the rebase in 0c701005 absorbed both. !2039 removed the #919 row from the Known gaps table in docs/dev/storage-accounting.md, and edited the #438 row. It merged at 10:45Z. !2287 added the VerdictResolver field to assembledApp in cmd/artifact-registry/main.go, and rewrote the one-line return into a multi-line struct literal. It merged at 12:09Z. Both resolutions keep every change from both sides. No line of either side is dropped.

Two open merge requests still conflict with this branch. Every measurement below used git merge-tree --write-tree --name-only against the peer's head at 12:45Z, and was re-run against main to attribute the conflict to this branch.

  • !2262 "feat(oci): container remote fill emits its storage-accounting deltas" conflicts on internal/format/oci/emit_dispatch.go. Both sides delete the same 30-line header block, and each replaces it with a different pointer. This branch points at the new sibling file internal/format/oci/emit_dispatch.md. !2262 points at docs/dev/storage-accounting.md. The two replacements are not interchangeable, so whichever merges second must re-point that header rather than take one side.
  • !2270 "feat(managementapi): emit counter deltas on the six remote evict routes" conflicts on docs/dev/storage-accounting.md. It deletes the Known gaps row for the six single-delete remote eviction routes, one line above two rows this branch rewrites. The two sides touch no common line, so whichever merges second keeps both edits.

Six more open merge requests share a file with this branch and conflict with main on their own. In each case the conflicted path set against this branch is byte-identical to the set against main, so none of them is evidence about this branch. !2263 is one of the six. It shares cmd/artifact-registry/main.go, which merges cleanly against this branch, and its docs/dev/observability.md conflict comes from a base 24 commits behind main.

Why the capacity is its own gauge, not a fourth state value

A capacity is not a connection state. So the state label keeps its closed set of three values, and sum by (state) stays correct for the saturation SLI that reads it first. internal/metrics/cardinality.go needs no entry, because the new gauge declares no label at all.

An operator reads saturation as:

gitlab_artifact_registry_database_connection_pool_size{state="active"}
  / ignoring(state) gitlab_artifact_registry_database_connection_pool_max_size

The matching modifier is not decoration. PromQL matches one-to-one on the full label set once __name__ is dropped, the left operand carries state and the right carries none, so an unqualified division matches nothing and returns empty at every evaluation. An alert on an empty vector never fires and reads exactly like a healthy one. ignoring(state) rather than on(): a bare on() matches everything to everything and errors on duplicate series once more than one pod is scraped, while working in a single-pod dev test. docs/dev/alerting.md writes the identical failure up for the lifecycle purger's and on (env).

Why the series is absent rather than zero

The pool does not exist until postgres.Client.Start runs, which is after the collector registers. All three state series still publish in that window, because zero connections in a state is a true count. Zero is not a capacity. A published 0 describes a pool that admits no connection at all, and it turns the saturation query into a division by zero. The collector publishes the capacity only above zero. A live pool cannot report a maximum below 1, because puddle rejects one and pgxpool hands MaxConns straight to it. A negative maximum is not published either: PoolStats is exported with no constructor, so any caller can declare one, and only a positive value describes a capacity.

Runtime evidence

A local boot of this branch answered HTTP 200 on /-/metrics at the probe listener, with one unlabeled sample:

gitlab_artifact_registry_database_connection_pool_max_size 22

22 is the live value rather than a literal the code carries, and it is a development boot rather than a production default. nproc on that host is 22, and no /etc/labkit/infrastructure.yaml was active there, so pgxpool's own default of max(4, numCPU) applied. A deployment sets the maximum through that file instead: LabKit's postgres.Client reads postgresql.max_conns from it and applies any positive value, and this repository carries no configuration key of its own for it. A deployed maximum of 1, 2 or 3 is therefore legal, and no assertion in this merge request assumes a floor of 4. Under a burst of 40 concurrent management reads, the three {state} series climbed toward the capacity and summed to exactly 22 after the drain. That is the saturation denominator observed rather than argued.

ADR check

The check ran and found no conflict. scripts/adr-freshness.sh exits 0 and reports the mirror current, with docs/adr/ identical between this branch and main. ADR-023's bundling clause governs the package tree, and its reverse-dependency ban fences the shared-infrastructure packages against an import of internal/format/*. This change adds no package and does not touch .golangci.yaml. It adds one import line, gitlab.com/gitlab-org/labkit/v2/postgres in cmd/artifact-registry/main.go, so the composition root can hand the pool to the boot test. That is the composition root reaching a shared infrastructure package, which is the direction the ban permits, and no shared-infrastructure package gains an import of internal/format/*. ADR-023 also admits a Markdown sidecar inside a format package. Its file-layout guidance is a starting point, and its sub-package prohibition is about Go packages. The mirrored ADRs say nothing about metric naming, label shape or connection pooling. ADR-012 is the one that names connection-pool statistics, and it excludes them from its decision as operational metrics. One ADR is internal and never mirrors, so this check covers the mirrored set.

Related to #559

What remains is deliverable D: the eleven caps still hold the literal 64, and no capacity input reaches any of them. All eight acceptance items belong to D: each one is an invariant that D's re-derivation must preserve. Merged state already establishes five of them, and this merge request changes none of that state. The other three have no evidence anywhere in the tree, because D's derivation does not exist yet.

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports

Loading
Loading