Skip to content

chore: wire telemetry into AsyncConnector, off by default - #573

Draft
enocom wants to merge 1 commit into
telemetry-7-wire-connectorfrom
telemetry-8-wire-async-connector
Draft

enocom wants to merge 1 commit into
telemetry-7-wire-connectorfrom
telemetry-8-wire-async-connector

Conversation

@enocom

@enocom enocom commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Part 8 of 9 in the built-in telemetry stack, splitting what was previously one ~3,200 line commit (#539) into reviewable pieces.

Based on #572 — review that one first; only the last commit here is new.

Unit tests, ruff check, ruff format --check and mypy are green at every commit in the stack, not just at the tip.


Same wiring as the synchronous Connector: a recorder per instance, one
dial_count per dial with a status, dial_latency on success, and the
refresh caches reporting under the same instance.

enable_builtin_telemetry defaults to False here too. The default flips
for both connectors in a change of its own.

Two things differ from the synchronous path. asyncpg surfaces a single
error for the whole connect, so a PostgresError — the server answered and
rejected us — is classified as a user error and everything else as a TCP
error, rather than letting tcp_error become a catch-all that hides user
mistakes. And open_connections is decremented from an asyncpg termination
listener, which Connection._cleanup invokes at most once on both close()
and terminate().

Neither byte counts nor mdx_error are reported here: asyncpg exposes no
hook for observing bytes on the wire, and the async path performs no
metadata exchange.


The stack (merges bottom to top):

  1. refactor: extract _metadata_exchange helper #566 — refactor: extract _metadata_exchange helper
  2. fix: do not mask dial errors with KeyError #567 — fix: do not mask dial errors with KeyError
  3. fix: close the socket when the metadata exchange fails #568 — fix: close the socket when the metadata exchange fails
  4. chore: add telemetry provider, recorders, and exporter #569 — chore: add telemetry provider, recorders, and exporter
  5. chore: add InstrumentedSocket #570 — chore: add InstrumentedSocket
  6. chore: record refresh metrics #571 — chore: record refresh metrics
  7. chore: wire telemetry into Connector, off by default #572 — chore: wire telemetry into Connector, off by default
  8. chore: wire telemetry into AsyncConnector, off by default #573 — chore: wire telemetry into AsyncConnector, off by default ← you are here
  9. feat: enable built-in telemetry by default #574 — feat: enable built-in telemetry by default

Group A (1–3) is independent of telemetry and could ship on its own, and carries no breaking change: dial failures are classified for the metric by tagging the exception in #572, not by changing the type callers see. Group B (4–8) is inert because enable_builtin_telemetry defaults to False throughout. #574 flips that default, so reverting it alone turns the whole feature off.

Same wiring as the synchronous Connector: a recorder per instance, one
dial_count per dial with a status, dial_latency on success, and the
refresh caches reporting under the same instance.

enable_builtin_telemetry defaults to False here too. The default flips
for both connectors in a change of its own.

Two things differ from the synchronous path. asyncpg surfaces a single
error for the whole connect, so a PostgresError — the server answered and
rejected us — is classified as a user error and everything else as a TCP
error, rather than letting tcp_error become a catch-all that hides user
mistakes. And open_connections is decremented from an asyncpg termination
listener, which Connection._cleanup invokes at most once on both close()
and terminate().

Neither byte counts nor mdx_error are reported here: asyncpg exposes no
hook for observing bytes on the wire, and the async path performs no
metadata exchange.
@enocom
enocom force-pushed the telemetry-8-wire-async-connector branch from 1ad8c01 to a63fbf8 Compare September 24, 2026 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant