review: fix db_connections_open leak on the detach path; comment/doc polish

Bug (verified vs SQLAlchemy 2.0.49): on GC-cleanup of an abandoned async connection, _finalize_fairy routes through fairy.detach(), which nulls the record's dbapi_connection so NullPool's close is a no-op (the `close` event never fires) then emits `detach` with the record. Listening only to close/invalidate left the marker unpopped, so db_connections_open leaked upward and never reset until restart. Listen to `detach` too — it carries the ConnectionRecord and the marker dedupes, so exactly one decrement occurs.

Also: strip a bare PR-number provenance tag from the test docstring (plastic-labs comment-reconciliation rule); disambiguate db_connections_open from the existing db_pool_connections; note in initialize_bounded_metrics that DB-instrumentation metrics zero-init in their registrar.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/db.py

@@ -289,8 +289,12 @@ def register_db_connection_instrumentation(instance_type: str) -> None:
     _connection_tracker = DBConnectionTracker(open_child, established_child)
     sync_engine = engine.sync_engine
     event.listen(sync_engine, "connect", _connection_tracker.on_connect)
-    # close AND invalidate both tear a connection down; the marker dedupes them.
-    for teardown_event in ("close", "invalidate"):
+    # A connection is torn down by close (normal return / recycle discard),
+    # invalidate (broken connection), or detach — the last fires on GC-cleanup of an
+    # abandoned async connection, where NullPool's close is a no-op so `close` never
+    # fires. All three carry the ConnectionRecord; the marker dedupes if more than
+    # one fires for the same connection.
+    for teardown_event in ("close", "invalidate", "detach"):
         event.listen(sync_engine, teardown_event, _connection_tracker.on_close)
     _db_connection_instrumentation_registered = True
 

src/telemetry/prometheus/metrics.py

@@ -211,7 +211,9 @@ db_queries_in_flight_gauge = NamespacedGauge(
 # Physical DB connections, tracked via SQLAlchemy connection-lifecycle events
 # (see DBConnectionTracker in src/db.py) rather than the pool object, so they are
 # visible under EVERY pool class — including NullPool, whose pool holds no records
-# for the scrape-time db_pool_connections collector to read.
+# for the scrape-time db_pool_connections collector to read. Under QueuePool this
+# roughly equals db_pool_connections{checked_in} + {checked_out}; its unique value
+# is under NullPool, where that collector reads zero.
 db_connections_open_gauge = NamespacedGauge(
     "db_connections_open",
     "Physical DB connections currently open by this instance, across all pool "
@@ -428,6 +430,11 @@ class PrometheusMetrics:
         """Pre-create bounded-label counter children at 0 for this process, so an
         absent series means a broken scrape rather than "nothing happened".
 
+        Note: the DB-instrumentation metrics (db_queries_in_flight,
+        db_connections_open, db_connections_established) are NOT initialized here —
+        they zero-init via the pre-resolved labeled children in their register_db_*
+        functions in src/db.py, so an auditor should not read them as forgotten.
+
         Args:
             instance_type: "api" or "deriver" — selects the process-specific
                 counters. Event-type and buffer metrics are initialized in both.

tests/telemetry/test_db_connection_metrics.py

@@ -7,7 +7,7 @@ pool keeps no records for the scrape-time ``db_pool_connections`` collector to r
 
 Asserts two properties:
 - zero-init — resolving a labeled child materializes it at 0, so an absent series
-  means a broken scrape rather than "no connections" (the #927 convention);
+  means a broken scrape rather than "no connections";
 - ``DBConnectionTracker`` semantics — increment once per connect, decrement at most
   once per connection (marker-guarded, so it can't leak upward or go negative), and
   the establishment counter is monotonic (closes never decrement it).