OTel-Native: Fix metric instrument types#3743
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
…etric-instrument-types
64f576c to
858116f
Compare
…etric-instrument-types
Fix two bugs in the UpDownCounter metric instrumentation: 1. Cache miss path double-counts idle connections (High Severity): newConn() records +1 idle for every new connection, but the cache miss path in getConn() only recorded +1 used without compensating -1 idle. This caused the idle counter to grow unboundedly. Fixed by adding -1 idle compensation in the cache miss path and in the hook failure path for new connections. 2. Hook-rejected idle connection causes incorrect state accounting (Medium Severity): When a hook rejects an idle connection without error, putConnWithoutTurn calls putConn which records -1 used, +1 idle. But the connection was never transitioned to used state, causing the used counter to go negative. Fixed by recording the idle->used transition before calling putConnWithoutTurn so putConn's used->idle accounting is balanced.
…ent idle counter drift
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
3 similar comments
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
|
@sera bypass |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
❌ Jit Scanner failed - Our team is investigatingJit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions. 💡 Need to bypass this check? Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e144ad7. Configure here.
This PR updates the redisotel-native observability instrumentation to use the correct OpenTelemetry instrument types for connection pool metrics, as specified by the OTel database semantic conventions v1.38.0.
Metric instrument type migration:
db.client.connection.count: Changed fromObservableGauge(async polling) →UpDownCounter(synchronous increment/decrement)db.client.connection.pending_requests: Changed fromObservableGauge→UpDownCounterfix #3730
Note
Medium Risk
Touches hot-path connection pool lifecycle and shutdown logic to emit metric deltas, so mistakes could cause incorrect counts or edge-case races under concurrency, though changes are scoped to instrumentation.
Overview
Updates
redisotel-nativeto match OTel DB semconv by switchingdb.client.connection.countanddb.client.connection.pending_requestsfrom asyncObservableGaugepolling to synchronousInt64UpDownCounterinstruments.Removes the pool registry +
RegisterCallbackgauge polling and instead emits +/- deltas frominternal/poolon connection create/idle↔used transitions, pending-request wait start/stop, PubSub track/untrack, and pool shutdown; also adds an optionalOTelConnectionCounterinterface (wired through theotelRecorderAdapter) so existing third-partyOTelRecorderimplementations aren’t broken by the new methods.Reviewed by Cursor Bugbot for commit e144ad7. Bugbot is set up for automated code reviews on this repo. Configure here.