Add pool tracing/metrics parity and surface pooled-open timeout cause - #4505
Draft
mdaigle wants to merge 1 commit into
Draft
Conversation
Instrument ChannelDbConnectionPool with TryPoolerTraceEvent calls across the connection lifecycle so it matches the categories traced by the WaitHandle pool: construction, get, create, return, remove/dispose, clear, startup, shutdown, prune, rate-limit throttle, error state, wait timeout, and the reason a connection was rejected as not live. Fill the two remaining metric gaps in ReplaceConnection, which disposed the old and failed-new connections without counting a hard disconnect. Also address GH#3545: record the last physical-connection-create exception on each pool and attach it as the inner exception of the pooled-open timeout, so callers see why the pool could not produce a connection. Fixes #3545 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Improves connection-pool diagnostics by bringing ChannelDbConnectionPool up to parity with the legacy WaitHandleDbConnectionPool for pooler tracing and metrics, and by surfacing the last physical connection creation failure as the inner exception on pooled-open timeouts (GH#3545).
Changes:
- Adds pool-lifecycle
TryPoolerTraceEventemissions throughoutChannelDbConnectionPoolto match legacy pool trace conventions. - Completes metric parity for the channel pool (notably around
ReplaceConnectionhard-disconnect accounting). - Introduces
IDbConnectionPool.LastConnectionCreateExceptionplusADP.PooledOpenTimeout(Exception inner)plumbing so pooled-open timeouts can carry the most recent physical open failure as an inner exception.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolBlockingPeriodTest.cs | Adds coverage that the legacy pool records and clears the last-create exception correctly. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs | Updates the test mock pool to implement the new LastConnectionCreateException interface member. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolInstrumentationTest.cs | New test suite validating channel-pool trace emission, metric deltas, and last-create-exception timeout plumbing. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Attaches the pool’s last-create exception to pooled-open timeouts thrown from the factory path. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | Tracks/clears the last physical create exception and uses it when pending opens time out. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs | Adds LastConnectionCreateException to standardize timeout-cause reporting across pool implementations. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | Adds trace parity, last-create-exception storage/clearing, and missing hard-disconnect accounting on replace paths. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs | Adds ADP.PooledOpenTimeout(Exception inner) overload to optionally carry an inner exception. |
Comment on lines
+582
to
+587
| private const string SqlClientEventSourceName = "Microsoft.Data.SqlClient.EventSource"; | ||
|
|
||
| // Mirrors SqlClientEventSource.Keywords.PoolerTrace. Duplicated as a literal because | ||
| // that type is not visible to this assembly. | ||
| private const EventKeywords PoolerTraceKeyword = (EventKeywords)32; | ||
|
|
mdaigle
marked this pull request as draft
August 5, 2026 16:48
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.
Stacked on #4504.
Summary
Brings
ChannelDbConnectionPoolto trace and metric parity withWaitHandleDbConnectionPool, and surfaces the cause of a pooled-open timeout.Trace parity
Adds
TryPoolerTraceEventcalls across the connection lifecycle, following the WaitHandle pool's message conventions:Constructed. MinPoolSize=..., MaxPoolSize=...GetInternalConnection:Getting connection.,Wait timed out.,Pool is shutting down; abandoning wait.GetIdleConnection:Popped from general pool.OpenNewInternalConnection:Errors are set.,Creating new connection.,Added to pool., rate-limiter saturation, pool-full, and create-threwPutConnectionInIdleChannel:Pushing to general pool.DeactivateAndRouteConnection: the stasis/transacted routing decisionRemoveConnection:Removing from pool.,Removed from pool.,Disposed.PruneConnections: prune start and resultIsLiveConnection: the reason a connection was rejected (idle timeout, dead, load balance timeout, stale generation)Metric parity
Most metric wiring landed in #4504 via
IdleConnectionChannel. This closes the two remaining gaps inReplaceConnection, which disposed the old connection and the failed new connection without counting aHardDisconnectRequest.Note: the WaitHandle pool leaks an
EnterPooledConnectionon replace. The channel pool swaps in place viaConnectionPoolSlots.TryReplace, so the pooled gauge is correctly left untouched. That behavior is intentionally not replicated.Pooled-open timeout cause (#3545)
Today a pooled-open timeout hides why the pool could not produce a connection. This adds
IDbConnectionPool.LastConnectionCreateException, recorded on create failure and cleared on success in both pools, and a newADP.PooledOpenTimeout(Exception inner)overload.SqlConnectionFactoryand the WaitHandle pool's pending-opens path now attach it as the inner exception.Fixes #3545
Out of scope
OpenTelemetry
db.client.connections.*semantic-convention metrics (Story 4 of the original request) are deferred to a follow-up.Suggested release note
UseConnectionPoolV2) now emits the same pooler trace events and connection metrics as the default pool.Testing
New
ChannelDbConnectionPoolInstrumentationTest(17 tests) covering trace emission for each operation category, metric deltas for soft connect/disconnect and hard disconnect, and the last-create-exception plumbing. Adds a matching WaitHandle-pool test. 343 connection-pool unit tests pass onnet9.0;net8.0builds clean.Checklist
IDbConnectionPoolis internal)