Add connection-pool observability, release 0.6.0 - #28
Conversation
A request that dies in transit never reaches the server, so when a customer reports intermittent timeouts there is nothing on our side to look at. That was the position we were in with the Upstart investigation -- the diagnosis rested on one log correlation and a local reconstruction. Adds counters (client.connection_stats -> opened / reused / expired) and a debug log naming the host and exact idle duration each time a connection is discarded. Enough for support to ask "paste your connection_stats" and know immediately whether idle expiry is engaging. Counters are thread-safe and only present when expiry is enabled; with max_idle_seconds=None the pool is left entirely to urllib3.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThe Python SDK now records pooled connection outcomes as opened, reused, or expired when idle expiry is enabled. Statistics are thread-safe, exposed through the client, and accompanied by debug logging and documentation for diagnosing timeouts. Tests cover counter behavior, disabled expiry, and eviction logs. Package metadata, version constants, user-agent text, and debug output are updated to version 0.6.0. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python-sdk/src/coval_sdk/client.py`:
- Around line 104-121: Count the replacement connection created after an expired
pooled connection as opened: update the expiry branch in _ConnectionPool.get to
record both expired and opened while preserving the existing close behavior.
Update the expiry-related assertions in python-sdk/tests/test_client.py lines
223-241 and revise the connection-statistics wording in python-sdk/README.md
lines 77-78 to reflect that opened counts these replacement physical
connections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: df96fa39-2bd0-4dcc-9dbd-47f693e207a5
📒 Files selected for processing (7)
python-sdk/README.mdpython-sdk/pyproject.tomlpython-sdk/src/coval_sdk/__init__.pypython-sdk/src/coval_sdk/api_client.pypython-sdk/src/coval_sdk/client.pypython-sdk/src/coval_sdk/configuration.pypython-sdk/tests/test_client.py
CodeRabbit correctly spotted that `opened` under-reports physical connections: an expired connection is closed and then transparently reconnects, so it costs a handshake that no counter surfaced. Confirmed against a real server -- one expiry produced two accepted sockets while `opened` read 1. Kept the three counters as disjoint per-checkout outcomes rather than folding replacements into `opened`, so exactly one is incremented per pool checkout and they still total the number of checkouts. Added `connections_established` (opened + expired) for the physical count, and corrected the README, which described `opened` as "connections created". The new test asserts against sockets the server actually accepted, not just the counters agreeing with themselves.
1 participant
Makes the SDK's connection reuse visible, so the next report of intermittent timeouts can be answered with evidence instead of inference.
Why
A request that dies in transit never reaches API Gateway, so there is no server-side trace of it. During the Upstart investigation that left us diagnosing from a single log correlation plus a locally reconstructed repro. If they report another timeout on 0.5.0 today, we still cannot tell whether idle expiry engaged.
What
Counters —
client.connection_stats:opened= new connection created,reused= pooled connection still inside the idle bound,expired= discarded for being too old. Thread-safe, andrepr()is formatted for pasting into a ticket.Debug log on each expiry, naming the host and the exact idle duration:
Enabled with
logging.getLogger("coval_sdk.client").setLevel(logging.DEBUG).Counters are
Nonewhenmax_idle_seconds=None, since the pool is then left entirely to urllib3.Verified end to end
Against a local keep-alive server:
connection_stats{'opened': 1, 'reused': 2, 'expired': 0}{'opened': 1, 'reused': 2, 'expired': 1}The counters track reality in both directions — reuse when warm, expiry when stale.
Also
README gains a "Connection reuse" and a "Diagnosing timeouts" section, since none of this helps if customers don't know it exists.
Version bumped to 0.6.0 in this PR rather than a follow-up. #26 shipped without a bump and needed #27 purely to make it releasable.
31 tests passing. New coverage: counters start at zero, absent when expiry is disabled, count each outcome correctly, and the debug log fires with the right idle duration.
Note
No Linear issue — the Linear MCP isn't reachable from this session and there's no API key on disk, so I couldn't file one. Worth attaching a ticket before merge if you want it tracked.