Repository navigation
Conversation
This was referenced Sep 18, 2026
enocom
added this pull request to stack #577
September 19, 2026 04:15
enocom
force-pushed
the
telemetry-7-wire-connector
branch
from
September 21, 2026 19:44
dc31871 to
2603dd3
Compare
enocom
force-pushed
the
telemetry-7-wire-connector
branch
2 times, most recently
from
September 24, 2026 02:45
91754a9 to
7d91ea9
Compare
Hand the synchronous Connector a metric recorder per instance and report on every dial: a dial_count with a status, a dial_latency for successful dials, and the byte and connection counts collected by InstrumentedSocket. The refresh caches get the same recorder so their refresh_count lands under the same instance. enable_builtin_telemetry defaults to False for now. Everything here is inert until that default flips, which is a change of its own so that the moment telemetry starts running for everyone is one line to revert. Dial failures are tagged with the phase that raised them -------------------------------------------------------- Every dial records exactly one dial_count with a status: success, user_error, cache_error, tcp_error, tls_error, or mdx_error. The status is set on every exit path and the count recorded in a finally, rather than per except-branch, which would silently drop the count for any failure that does not match a branch. metadata_exchange names the phase that failed by tagging the exception on its way out, and connect_async reads the tag back. An exception carrying no tag is a problem with the cached connection info itself, e.g. create_ssl_context on a malformed certificate, and records as cache_error. Tagging keeps the classification out of the exceptions callers see: a refused connection is still a ConnectionRefusedError with its errno, traceback and stdlib subclass intact, so nothing here is a breaking change for callers. The Go connector likewise returns a plain error from its metadata exchange rather than a named type. The tag travels on the exception object itself, which survives the hop back from the executor thread that ran the dial. InstrumentedSocket is installed only when telemetry is enabled -------------------------------------------------------------- The wrapper stands between every driver and its socket for the life of the connection, reimplementing makefile() and close(). A caller who opted out should get the socket the driver would have had without this feature, so the wrap is behind the flag. The only thing lost is open_connections for a caller whose metrics are not being exported anyway. Shutdown -------- close() bounds only the telemetry flush, not the whole shutdown sequence. The provider gets TELEMETRY_SHUTDOWN_TIMEOUT_S, passed down to the write RPC as an explicit deadline so a stalled write fails fast; the previous 3s budget covered the whole sequence, which would let a slow cache close abandon the connector half-shut-down. Matches Dialer.Close in the Go connector, which bounds only the metric shutdown. The README's debug logging example is corrected while it is being referenced: google-api-core stops the google logger from propagating to the root logger as soon as any Google client is built, so logging.basicConfig() on its own never showed these records.
enocom
force-pushed
the
telemetry-7-wire-connector
branch
from
October 21, 2026 02:48
7d91ea9 to
64a522a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 7 of 9 in the built-in telemetry stack, splitting what was previously one ~3,200 line commit (#539) into reviewable pieces.
Based on #571 — review that one first; only the last commit here is new.
Unit tests,
ruff check,ruff format --checkandmypyare green at every commit in the stack, not just at the tip.Hand the synchronous Connector a metric recorder per instance and report
on every dial: a dial_count with a status, a dial_latency for successful
dials, and the byte and connection counts collected by
InstrumentedSocket. The refresh caches get the same recorder so their
refresh_count lands under the same instance.
enable_builtin_telemetry defaults to False for now. Everything here is
inert until that default flips, which is a change of its own so that the
moment telemetry starts running for everyone is one line to revert.
Dial failures are tagged with the phase that raised them
Every dial records exactly one dial_count with a status: success,
user_error, cache_error, tcp_error, tls_error, or mdx_error. The status
is set on every exit path and the count recorded in a finally, rather
than per except-branch, which would silently drop the count for any
failure that does not match a branch.
metadata_exchange names the phase that failed by tagging the exception on
its way out, and connect_async reads the tag back. An exception carrying
no tag is a problem with the cached connection info itself, e.g.
create_ssl_context on a malformed certificate, and records as
cache_error.
Tagging keeps the classification out of the exceptions callers see: a
refused connection is still a ConnectionRefusedError with its errno,
traceback and stdlib subclass intact, so nothing here is a breaking
change for callers. The Go connector likewise returns a plain error from
its metadata exchange rather than a named type. The tag travels on the
exception object itself, which survives the hop back from the executor
thread that ran the dial.
InstrumentedSocket is installed only when telemetry is enabled
The wrapper stands between every driver and its socket for the life of
the connection, reimplementing makefile() and close(). A caller who
opted out should get the socket the driver would have had without this
feature, so the wrap is behind the flag. The only thing lost is
open_connections for a caller whose metrics are not being exported
anyway.
Shutdown
close() bounds only the telemetry flush, not the whole shutdown sequence.
The provider gets TELEMETRY_SHUTDOWN_TIMEOUT_S, passed down to the write
RPC as an explicit deadline so a stalled write fails fast; the previous
3s budget covered the whole sequence, which would let a slow cache close
abandon the connector half-shut-down. Matches Dialer.Close in the Go
connector, which bounds only the metric shutdown.
The README's debug logging example is corrected while it is being
referenced: google-api-core stops the google logger from propagating to
the root logger as soon as any Google client is built, so
logging.basicConfig() on its own never showed these records.
The stack (merges bottom to top):
_metadata_exchangehelperInstrumentedSocketGroup 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_telemetrydefaults toFalsethroughout. #574 flips that default, so reverting it alone turns the whole feature off.