Emit pool metrics and traces, and fix Count semantics in ChannelDbConnectionPool

[mdaigle]

Rebased onto main and now also contains the tracing parity work that was previously PR #4505.

ChannelDbConnectionPool did not emit any of the connection pool counters that WaitHandleDbConnectionPool emits, reported Count as the reservation count (which includes in-flight creations), and traced far less than the wait handle pool.

Bringing the two pools to counter parity surfaced several counter bugs that affect both pools. Those fixes are called out separately below.

Metrics

• Wire the pooled/free connection and soft/hard connect/disconnect counters through ChannelDbConnectionPool and IdleConnectionChannel, at the call sites matching WaitHandleDbConnectionPool.

Count semantics

• Add ConnectionPoolSlots.ConnectionCount and point Count at it, so Count reflects connections that actually belong to the pool rather than reservations held for connections still being opened. This matches WaitHandleDbConnectionPool and fixes the SQL Express user instance path, which branches on pool.Count <= 0.
• Gate warmup on ReservationCount instead of Count, matching the max-pool-size gate and the pruner. Count excludes in-flight creations, so warmup created duplicates for connections other threads were already opening.

Tracing parity

• Add pooler traces to match the wait handle pool, including a per-reason trace for connections rejected by the liveness gate and a trace for connections held by a transaction (which otherwise vanish from the trace stream after deactivation).
• Traces in TransactedConnectionPool keep the existing format, since it is shared with the wait handle pool and reformatting it would change v1's trace stream. PoolPruner is constructed only by ChannelDbConnectionPool, so it takes the new format. Converting the rest of the repo to a single trace format is left to its own PR.

Injectable metrics sink

• Add IDbConnectionPool.Metrics and let both pool implementations take a SqlClientMetrics in their constructor, defaulting to the global SqlClientDiagnostics.Metrics. SqlConnectionFactory holds its own sink and feeds it to each connection through DbConnectionInternal's constructor, so a test can observe connection-level counters too.
• Pools take their sink independently of the factory's so they can be constructed and asserted on in isolation. Both default to the same process-wide instance, so they agree in production.
• This gives the tests a per-test counter instance, so the metric tests can assert exact absolute counts rather than deltas against process-wide state. The metric tests are parameterized over both pool implementations, which is what establishes the parity.

Counter bugs fixed in both pools

Each of these drove a publicly documented gauge away from reality permanently, since a gauge never recovers from an unmatched increment or decrement.

Bug	Gauge affected	Symptom
ReplaceConnection counted the replacement checkout but nothing for the connection it displaced	active-soft-connects	Drifted up by one per replacement
ReplaceConnection disposed the displaced connection without recording its destruction	active-hard-connections	Drifted up by one per replacement (v1 only; v2 already handled this)
CreateObject swapped the replacement into the old connection's place but only counted the addition	number-of-pooled-connections	Drifted up by one per replacement (v1 only)
Both pools counted the checkout after activating, so a failed activation returned the connection and emitted an unpaired soft disconnect	active-soft-connects	Went negative
ActivateConnection counted after calling Activate while DeactivateConnection counts before calling Deactivate	number-of-active-connections	Went negative on failed activation
The wait handle pool counted a soft connect even when it vended no connection	active-soft-connects	Drifted up
ConnectionPoolSlots.Add disposed an opened connection on its failure path without recording the destruction	active-hard-connections	Drifted up (v2 only)

Counter semantics these were fixed against, for the record: a soft connect is the pool vending a connection, a soft disconnect is a connection returning to the pool, a hard connect is a new physical connection, and a hard disconnect is a connection being destroyed.

Known gap

number-of-reclaimed-connections reads flat zero under ChannelDbConnectionPool. ReclaimedConnectionRequest is only emitted by the wait handle pool, because the channel pool does not yet reclaim emancipated connections. That work is tracked in #4490 and the counter will be wired up there.

Suggested release note entry

Fixed several connection pool event counters that could drift permanently or report negative values. active-soft-connects, active-hard-connections, number-of-pooled-connections, and number-of-active-connections were miscounted when a connection was replaced or when a pooled connection failed to activate.

• Tests added or updated
• Public API changes documented (no public API changes)
• Ensure no breaking changes introduced

Tests: the pool instrumentation tests are consolidated in DbConnectionPoolInstrumentationTest and parameterized over both pools. They assert every counter's exact value, treating any counter the test does not name as zero, so an unexpected emission fails. New coverage in this PR: failed activation leaves all counters balanced, concurrent checkout/return settles every gauge to a consistent resting state, and idle pruning records the destruction it performs. The failed-activation test is what surfaced the two negative gauges. The stasis gauge also gained unit coverage, which it previously had none of: all three ways a pool puts a connection in stasis, and both ways it leaves.

Not verified: net462. This branch was developed on macOS, where that target cannot be built.

Deferred to separate PRs: emancipated-connection reclamation (#4490), surfacing the underlying cause of a pooled-open timeout as an inner exception, and the async idle-connection fast path.

[Copilot]

Pull request overview

This PR closes parity gaps between ChannelDbConnectionPool (V2) and WaitHandleDbConnectionPool (V1) discovered via differential testing, focusing on correct pooling semantics and consistent diagnostics/metrics behavior across implementations.

Changes:

• Emit pool metrics in the channel-based pool to match the wait-handle pool (pooled/free/active connection counters and connect/disconnect-related counters).
• Fix ChannelDbConnectionPool.Count semantics to report tracked connections (slot occupancy) rather than in-flight reservations.
• Add an async idle-connection fast path so OpenAsync can complete synchronously on a warm pool (excluding transactional requests), and update/parameterize tests accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs	Implements async idle fast path, fixes Count to use tracked connections, and wires metrics at key lifecycle points.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs	Adds ConnectionCount to distinguish tracked connections from reservations.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs	Emits free-connection metrics on idle enqueue/dequeue to centralize counter correctness.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs	Updates stress test to avoid hanging when async acquisition can complete synchronously.
src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/MetricsTest.cs	Parameterizes pooled connection metrics test across pool versions via ConnectionPoolVersionScope.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs	Parameterizes resiliency SPID test across pool versions via ConnectionPoolVersionScope.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs	Fixes Task.Factory.StartNew(async …) by unwrapping the nested task so failures/timeouts are observed correctly.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:963

• The new async idle fast-path can return a pooled connection without ever emitting the standard "Getting connection." pooler trace (it bypasses GetInternalConnection, where that trace is currently emitted). This makes trace captures inconsistent: a successful async open from the idle channel will only show "Popped from general pool" without the usual request-start marker, reducing diagnosability and trace parity with WaitHandleDbConnectionPool.

            Transaction? ambientTransaction = taskCompletionSource.Task.AsyncState as Transaction;

            // Try to satisfy the request synchronously from the idle channel before paying for a

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1869

• Pruning is documented as respecting the MinPoolSize floor, but the loop condition uses ReservationCount. Since ReservationCount includes in-flight opens, pruning can run even when the actual tracked-connection count (Count/ConnectionCount) is already at MinPoolSize, potentially dropping the pool below its minimum. WaitHandleDbConnectionPool’s cleanup loop uses Count > MinPoolSize for this floor check.

            while (count > 0
                && IsRunning
                && _connectionSlots.ReservationCount > MinPoolSize
                && _idleChannel.TryRead(out var connection))

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolInstrumentationTest.cs:33

• This test class mutates process-wide AppContext switches (e.g., IdleTimeoutEviction_EmitsReasonTrace sets UseLegacyIdleTimeoutBehavior). Without serializing via AppContextSwitchTestCollection, this can race with other tests and cause cross-test contamination/flakiness.

namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool
{
    /// <summary>
    /// Verifies the diagnostic instrumentation of <see cref="ChannelDbConnectionPool"/>: the pooler
    /// trace events emitted across the connection lifecycle.
    /// </summary>
    public class ChannelDbConnectionPoolInstrumentationTest
    {
        /// <summary>

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1845

• PruneConnections uses _connectionSlots.ReservationCount to enforce the MinPoolSize floor. ReservationCount includes in-flight connection creations, so pruning can drop the pool below MinPoolSize (in terms of actual tracked connections) while a create is pending; if that create later fails, the pool can end up permanently under the configured minimum. Since Count now reflects tracked connections (WaitHandle pool semantics), use Count/ConnectionCount here for parity and to keep the floor based on actual connections.

            while (count > 0
                && IsRunning
                && _connectionSlots.ReservationCount > MinPoolSize
                && _idleChannel.TryRead(out var connection))

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolInstrumentationTest.cs:33

• This test class mutates process-wide cached AppContext switch values (UseLegacyIdleTimeoutBehavior) and also asserts against process-wide metric gauges. To avoid cross-test interference/flakiness, it should be placed in the existing non-parallel AppContextSwitchTests collection (same pattern used by ChannelDbConnectionPoolTest and *IdleTimeoutTest).

    public class ChannelDbConnectionPoolInstrumentationTest
    {

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1855

• Pruning currently uses _connectionSlots.ReservationCount > MinPoolSize as its floor check. After this PR changes Count to reflect actual tracked connections (excluding in-flight creates), using ReservationCount here can cause the pruner to remove idle connections based on reservations held for connections still opening. If those in-flight opens fail/cancel, the pool can end up below MinPoolSize even though the loop condition was intended to enforce the floor. This should use Count (or _connectionSlots.ConnectionCount) to match the new semantics and the WaitHandle pool’s pruning logic.

            while (count > 0
                && IsRunning
                && _connectionSlots.ReservationCount > MinPoolSize
                && _idleChannel.TryRead(out var connection))

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:777

• The created pooled connection also receives the factory sink, so its active-connection counters do not honor IDbConnectionPool.Metrics when the injected sinks differ. Select pool.Metrics for pooled creation and retain the factory sink only when pool is null.

                metrics: Metrics);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:228

• Pooled hard-connects are still emitted through the factory's sink rather than the owning pool's Metrics. Because the pool and factory accept independent injected sinks, a pool-scoped sink can miss every hard connect (the tests currently hide this by passing the same fake to both). Route pooled connects through pool.Metrics; keep the factory sink for non-pooled connections.

This issue also appears on line 777 of the same file.

            Metrics.HardConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:210

• The tracing-parity behavior is untested: the new instrumentation suite asserts counters only, and the connection-pool tests contain no listener/assertions for TryPoolerTraceEvent. Add trace capture tests for lifecycle events and each liveness-rejection/transaction reason so missing or malformed events do not silently regress.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ChannelDbConnectionPool | INFO | {0}, Constructed. MinPoolSize={1}, MaxPoolSize={2}",
                Id,
                MinPoolSize,
                MaxPoolSize);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs:101

• The new in-flight reservation distinction is not covered by a regression test. Existing ConnectionPoolSlotsTest assertions only inspect ReservationCount after creation callbacks finish, so they cannot catch ConnectionCount accidentally tracking reservations again. Add a gated creation test that observes ReservationCount == 1 and ConnectionCount == 0 while creation is blocked, then verifies both are 1 after insertion (and that the channel pool's Count follows ConnectionCount).

        /// <summary>
        /// Gets the number of connections currently tracked by this collection. Unlike
        /// <see cref="ReservationCount"/>, this excludes reservations held for connections that are
        /// still being opened, so it reports connections that actually belong to the pool.
        /// </summary>
        internal int ConnectionCount => _connectionCount;

[cheenamalhotra]

Closing the metrics gap before v2 becomes the default is the right call, and parameterizing the tests over both pools is the correct way to lock parity in. Ran the new tests locally, 13/13 green on net9.0.

Most of my comments are about counters that don't balance, and about Count now meaning two different things inside the same class. Two more asks that don't belong on a line:

• Can we get the full v1 vs v2 counter delta written down in the description and a release note? ReclaimedConnectionRequest is never emitted by the channel pool (deferred to #4490), so that counter reads a flat zero the moment someone flips UseConnectionPoolV2. Anyone comparing dashboards across the switch will open a support case, and I'd rather we documented it than debugged it.
• The new per-connection traces add roughly four events per checkout on the hottest path we have. TryPoolerTraceEvent is IsEnabled-gated, but the Id / ObjectID arguments still box into the params object[] at every call site whether tracing is on or not. Can we confirm with a benchmark that tracing-off throughput is unchanged?

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (7)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:777

• A pooled internal connection must use its owning pool's sink, but this passes the factory sink. If the new pool-level injection is used without separately configuring the factory, activation/deactivation counters go to a different instance than the pool's free/pooled/disconnect counters, defeating exact pool-scoped assertions. Select pool.Metrics when a pool is present and retain the factory sink for non-pooled connections.

                metrics: Metrics);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1506

• PrepareConnection returns an activation failure through ReturnInternalConnection, which now emits SoftDisconnectRequest. Because the matching soft connect is recorded only after PrepareConnection succeeds, every failed enlistment/activation decrements the active-soft gauge even though no checkout was counted. Emit SoftConnectRequest before preparation so the return path balances it, matching the failure-safe accounting used by replacement.

            PrepareConnection(owningConnection, connection, transaction);
            Metrics.SoftConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:228

• This pooled-connect counter bypasses the pool-scoped sink and reports to the factory sink instead. A pool constructed with its own Metrics (the purpose of the new injection point) loses hard-connect events unless its caller also knows to inject the identical object into the factory. Route pooled events through pool.Metrics; the factory sink remains appropriate for non-pooled creation.

This issue also appears on line 777 of the same file.

            Metrics.HardConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:207

• The new and rewritten messages use a different payload grammar from the established pooler traces. Existing sites consistently emit the <prov.DbConnectionPool.Method|RES|...|CPOOL> markers (for example, WaitHandleDbConnectionPool.cs:1187 and DbConnectionInternal.cs:547), while this change uses Class.Method | INFO |. Besides preventing the stated tracing parity, rewriting existing payloads can break consumers that classify these string events by their markers. Keep the established pooler trace format across the new/updated messages.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ChannelDbConnectionPool | INFO | {0}, Constructed. MinPoolSize={1}, MaxPoolSize={2}",

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs:101

• The core Count fix has no regression test for the behavior that differs from ReservationCount: while an Add callback holds a reservation and has not yet returned a connection, ConnectionCount must remain zero, then become one only after insertion and return to zero on removal. Existing pool-count tests observe only completed opens, so they would also pass with the old reservation-based implementation. Add a deterministic blocked-create test (and replacement/removal assertions) for this new counter.

        internal int ConnectionCount => _connectionCount;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1248

• No automated test subscribes to the pooler EventSource or asserts any of the newly added trace payloads, including these four distinct liveness-rejection reasons. As a result, missing events and incompatible message formats can regress while all new instrumentation tests still pass because they assert counters only. Add trace-listener tests covering construction/acquisition, each rejection reason, transaction holding, removal, and pruning.

                SqlClientEventSource.Log.TryPoolerTraceEvent(
                    "ChannelDbConnectionPool.IsLiveConnection | INFO | {0}, Connection {1}, exceeded the connection idle timeout and removed.",
                    Id,
                    connection.ObjectID);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:404

• The idle-reuse branch leaves soft checkout metrics incorrect in both outcomes. On success, the final SoftConnectRequest records checkout of the idle replacement but retiring oldConnection never records a soft disconnect, so the active-soft gauge grows by one. On activation failure, PrepareConnection returns the candidate through ReturnInternalConnection, which records a soft disconnect before this method has recorded its soft connect, so the gauge drops while the old connection remains checked out. Record the candidate soft connect before PrepareConnection in the idle branch, balance the old checkout when it is retired, and keep the create-new branch's metrics conditional on successful activation.

This issue also appears on line 1505 of the same file.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ReplaceConnection | INFO | {0}, replacing connection.", Id);

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (6)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:530

• When replacement reuses an idle connection, the old checked-out connection is retired via RemoveConnection, but that branch never emits SoftDisconnectRequest. The unconditional soft connect here therefore leaves the active-soft gauge one too high after every idle-backed replacement. Balance the old checkout in both replacement branches.

            Metrics.SoftConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:541

• PrepareConnection calls ReturnInternalConnection when activation throws, before the caller reaches its matching SoftConnectRequest. This unconditional decrement therefore makes the active-soft counter negative on activation failures (including the existing idle-reuse activation-failure path). Ensure rollback either records the attempted soft connect first or returns without emitting a soft disconnect.

            Metrics.SoftDisconnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:228

• This is a pooled operation, but it records the hard connect on the factory's independently injected sink rather than the owning pool's sink. Constructing a pool with a test sink and a default factory then produces zero hard connects and one hard disconnect on that pool sink after disposal. Route this through pool.Metrics so the new pool-scoped injection is self-consistent.

            Metrics.HardConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs:101

• The key new semantic—excluding a reservation while its create callback is still in flight—is not covered by the existing slot or pool tests; they only inspect counts after creation completes. Add a blocked-create test that observes ReservationCount == 1 and ConnectionCount == 0, then verifies add/remove transitions, to protect the SQL Express user-instance fix.

        /// <summary>
        /// Gets the number of connections currently tracked by this collection. Unlike
        /// <see cref="ReservationCount"/>, this excludes reservations held for connections that are
        /// still being opened, so it reports connections that actually belong to the pool.
        /// </summary>
        internal int ConnectionCount => _connectionCount;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:210

• Tracing parity is a primary behavior of this change, but no unit test subscribes to PoolerTrace or asserts any of the newly added lifecycle and liveness-rejection messages. Add listener-based coverage for representative lifecycle events and each rejection reason so trace regressions are detected.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ChannelDbConnectionPool | INFO | {0}, Constructed. MinPoolSize={1}, MaxPoolSize={2}",
                Id,
                MinPoolSize,
                MaxPoolSize);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:195

• Non-pooled teardown still records HardDisconnectRequest on SqlClientDiagnostics.Metrics (DbConnectionInternal.cs:508), while this change records the matching connect on the factory's injected sink. A custom sink therefore permanently reports an extra active hard connection. Keep non-pooled connect/disconnect on the same sink (or leave both global); only pooled accounting can safely use the pool-scoped sink as currently structured.

                Metrics.HardConnectRequest();

[mdaigle]

Thanks for the thorough pass. Everything is addressed; a few notes on the two PR-level points.

Reclaimed connections. Correct, number-of-reclaimed-connections reads flat zero under the channel pool, since ReclaimedConnectionRequest is only emitted by the wait handle pool. The channel pool doesn't reclaim emancipated connections yet. That's #4490 and the counter gets wired up there. Called out as a known gap in the description.

Boxing in the trace calls. I don't think this one applies. TryPoolerTraceEvent has no params object[] overload; it's generic T0 through T3, so value-type arguments go through generic instantiation rather than boxing. ToString/string.Format only run inside the IsPoolerTraceEnabled() guard, so nothing is allocated when tracing is off. Happy to benchmark if you still want it, but there doesn't appear to be anything to measure on that path.

On the counter fixes. Pushing on the ReplaceConnection imbalance turned up more than the original comment: the same class of bug appears in five places across both pools, and two of them drive gauges negative rather than just upward. There's a table in the description. The one worth a second look is that active-soft-connects is somewhat redundant with number-of-active-connections, and it's .NET-only with no .NET Framework perf counter behind it. I've kept and fixed it rather than touching it, since it's public surface, but retiring it might be worth considering at a major version.

The counter semantics I fixed against: a soft connect is the pool vending a connection, a soft disconnect is a connection returning to the pool, a hard connect is a new physical connection, a hard disconnect is a connection being destroyed.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (6)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs:613

• The delegated-transaction cleanup path has the same sink mismatch: this exit uses the injected connection sink, while the asynchronous non-pooled completion path enters the process-wide sink at SqlConnectionFactory.cs:1106. Once the transaction ends, the two gauges become permanently unbalanced. Route the entry and exit through the same metrics instance.

                Metrics.ExitNonPooledConnection();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs:527

• This exit now uses the injected connection sink, but the matching synchronous non-pooled entry still calls SqlClientDiagnostics.Metrics.EnterNonPooledConnection() in SqlConnectionFactory.cs:487. With an injected factory sink, closing a non-pooled connection therefore decrements the test sink below zero and leaves the process-wide gauge incremented. Route both sides through the same sink.

This issue also appears on line 613 of the same file.

                                Metrics.ExitNonPooledConnection();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs:103

• Add a concurrency-focused test that holds ConnectionPoolSlots.Add inside its creation callback and verifies ReservationCount == 1 while ConnectionCount (and ChannelDbConnectionPool.Count) remains zero, then verifies add/remove transitions. Existing slot tests assert only ReservationCount, so they cannot detect this semantic regression or an unbalanced _connectionCount.

        internal int ConnectionCount => Volatile.Read(ref _connectionCount);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:210

• These new and rewritten channel-pool traces use a bespoke ChannelDbConnectionPool.Method | INFO | prefix instead of the established pooler format used throughout the legacy implementation, for example WaitHandleDbConnectionPool.cs:489 and :527 (<prov.DbConnectionPool.Method|RES|CPOOL>). That makes the two implementations non-parallel for consumers that group/filter messages by the existing method and category tags. Preserve the established prefix structure while adding the new events.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ChannelDbConnectionPool | INFO | {0}, Constructed. MinPoolSize={1}, MaxPoolSize={2}",
                Id,
                MinPoolSize,
                MaxPoolSize);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1050

• Add trace-listener tests for the newly introduced lifecycle events (creation, waits, liveness rejection reasons, transaction parking, removal, and pruning). No test under the connection-pool suites subscribes to pooler traces or asserts any of these messages, so the stated tracing parity—and even whether these calls are observable—can regress unnoticed.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.OpenNewInternalConnection | INFO | {0}, Creating new connection.", Id);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:534

• The soft-connect is recorded too late and the idle-reuse branch never records the replaced connection's soft disconnect. On successful idle reuse, the active-soft gauge increases from one checked-out connection to two; if activation of the idle connection fails, PrepareConnection returns it and emits a soft disconnect before this line is reached, driving the gauge negative. Record the replacement checkout before activation and balance the old checkout in both successful branches.

            Metrics.SoftConnectRequest();

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1695

• Add a regression test that holds a user-requested creation in flight, invokes warmup with MinPoolSize satisfied by that reservation, and verifies no duplicate creation is scheduled. The current warmup tests cover a user request arriving during warmup, but not this opposite ordering that specifically distinguishes ReservationCount from Count.

            if (_connectionSlots.ReservationCount >= MinPoolSize)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:534

• The soft counters are still unbalanced when replacement reuses an idle connection. This common SoftConnectRequest runs only after PrepareConnection: if activation fails, PrepareConnection returns the idle connection and emits a soft disconnect with no matching connect; if activation succeeds, RemoveConnection(oldConnection) does not emit the soft disconnect for the displaced checkout. Count the idle checkout before activation and balance the old checkout after successful replacement, as the wait-handle path does.

            Metrics.SoftConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:124

• The injected sink is not used consistently for non-pooled connections. Non-pooled opens still call the global sink at SqlConnectionFactory.cs:487 and :1106, while closing now calls the connection's injected sink at DbConnectionInternal.cs:527. With a custom factory sink, each lifecycle leaves the global gauge incremented and the injected gauge decremented. Route both EnterNonPooledConnection calls through this Metrics instance.

        protected ISqlClientMetrics Metrics { get; }

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:240

• Add a deterministic regression test that blocks creation after a slot is reserved and verifies Count remains zero until the connection is actually installed. Existing count assertions only run after creation completes, so they would not catch Count reverting to reservation semantics or the SQL Express first-connection failure this change targets.

This issue also appears on line 1695 of the same file.

        public int Count => _connectionSlots.ConnectionCount;

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:534

• The soft-connect is recorded only after both replacement branches complete. In the idle-reuse branch, PrepareConnection returns a failed activation through ReturnInternalConnection, emitting a soft disconnect before this line is reached; on success, RemoveConnection(oldConnection) never emits the displaced connection's soft disconnect. Thus failed idle activation lowers active-soft-connects, while successful idle replacement raises it permanently. Record the checkout before preparing an idle connection and balance the displaced checkout on successful idle replacement; add both cases to the metric test.

            Metrics.SoftConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:92

• The injected factory sink is not used by all factory-owned counters. In particular, both EnterNonPooledConnection call sites still target SqlClientDiagnostics.Metrics, while the created connection now targets this injected sink for ExitNonPooledConnection. A factory constructed with a test sink therefore leaves the global gauge incremented and drives the injected gauge to -1 on close. Route the remaining factory counter calls through Metrics as well.

            Metrics = metrics ?? SqlClientDiagnostics.Metrics;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:210

• The tracing parity work has no automated trace assertions: the unit-test tree contains no pooler-trace listener or checks for these newly added messages. Add focused coverage for lifecycle, timeout/shutdown, liveness-rejection, transaction-held, and pruning traces so message omissions and format regressions are detected.

This issue also appears on line 534 of the same file.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ChannelDbConnectionPool | INFO | {0}, Constructed. MinPoolSize={1}, MaxPoolSize={2}",
                Id,
                MinPoolSize,
                MaxPoolSize);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs:103

• Add unit coverage for the new ConnectionCount invariant. ConnectionPoolSlotsTest currently verifies only ReservationCount, so it would not catch missed increments/decrements or prove the key distinction that an in-flight reservation raises ReservationCount while ConnectionCount remains unchanged. Cover construction, add/remove/replace, failed creation, and a blocked create callback.

        /// <summary>
        /// Gets the number of connections currently tracked by this collection. Unlike
        /// <see cref="ReservationCount"/>, this excludes reservations held for connections that are
        /// still being opened, so it reports connections that actually belong to the pool.
        /// </summary>
        internal int ConnectionCount => Volatile.Read(ref _connectionCount);

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:534

• SoftConnectRequest is emitted only after the replacement has been prepared. When GetIdleConnection() supplies the replacement and activation throws, PrepareConnection returns that connection through ReturnInternalConnection, which emits SoftDisconnectRequest, but execution never reaches this line. That drives active-soft-connects negative—the same failure mode this PR fixes on normal checkout. Count the idle replacement before calling PrepareConnection (so its failure return balances it), while keeping the create-new branch balanced separately.

            Metrics.SoftConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:195

• The injected factory sink is not used consistently for non-pooled connections. This hard-connect event and the connection's later hard-disconnect/ExitNonPooledConnection now go to Metrics, but both non-pooled success paths in TryGetConnection still call SqlClientDiagnostics.Metrics.EnterNonPooledConnection() (lines 487 and 1106). With a test sink, closing a non-pooled connection therefore leaves its non-pooled gauge at -1 and the global gauge at +1. Route those enter events through the factory's Metrics as well.

                Metrics.HardConnectRequest();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs:66

• The connection is published to the channel before the free gauge is incremented. A concurrent reader can consume it and call ExitFreeConnection during this window, making number-of-free-connections transiently negative and observable by an EventCounter poll. Increment the count/gauge before publishing, and roll both back if TryWrite fails.

                    Interlocked.Increment(ref _count);
                    _metrics.EnterFreeConnection();

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:210

• The new tracing behavior is not exercised by any test: there is no pool test that subscribes to the EventSource or asserts any of the added lifecycle/rejection messages. Because trace parity is a primary behavior of this PR and these exact strings are the operator-facing contract, add listener-based coverage for the representative success, timeout/shutdown, transaction-held, and liveness-rejection paths.

            SqlClientEventSource.Log.TryPoolerTraceEvent(
                "ChannelDbConnectionPool.ChannelDbConnectionPool | INFO | {0}, Constructed. MinPoolSize={1}, MaxPoolSize={2}",
                Id,
                MinPoolSize,
                MaxPoolSize);

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs:103

• ConnectionCount has no direct regression coverage. Existing ConnectionPoolSlotsTest assertions only inspect ReservationCount, so they would not detect a missing increment/decrement or prove the defining behavior that an in-flight Add reservation is excluded. Add a gated create-callback test that observes both counts while creation is blocked, then verifies add, remove, and replace semantics.

        /// Gets the number of connections currently tracked by this collection. Unlike
        /// <see cref="ReservationCount"/>, this excludes reservations held for connections that are
        /// still being opened, so it reports connections that actually belong to the pool.
        /// </summary>
        internal int ConnectionCount => Volatile.Read(ref _connectionCount);