redisotel: unsigned used-connection subtraction can wrap to billions on inconsistent pool stats #4030

Expected Behavior

db.client.connections.usage{state="used"} should report a non-negative count of connections in use. Reading pool statistics while the pool changes should not produce a count near the maximum uint32 value.

Current Behavior

In v9.22.0, redisotel computes the used count as:

int64(stats.TotalConns - stats.IdleConns)

Both operands are uint32, so the subtraction wraps before the conversion to int64 if IdleConns > TotalConns. A difference of minus one becomes 4,294,967,295, and minus two becomes 4,294,967,294.

This can corrupt both a total reconstructed by adding the idle and used series and a utilization percentage calculated from those series. A single invalid used sample can make the reconstructed total enormous and utilization approach 100%, even when the idle series is normal.

The same expression is still present on master at commit 7f3b3dffde59329db9fa7a71d650ef373affb599.

Possible Solution

Consider both:

• Reading total and idle as one coherent pool snapshot, rather than acquiring and releasing the pool lock separately for each field.
• Preventing unsigned wrap in the telemetry calculation and defining how an inconsistent snapshot should be handled. Converting each operand before subtracting prevents wrap, but by itself can still publish a negative connection count.

The desired outcome is correct pool telemetry, rather than smoothing or clamping the resulting dashboard percentage.

Steps to Reproduce

This is a minimal reproduction of the arithmetic failure, not an end-to-end reproduction of a concurrent pool transition.

1. Save the following as main.go:

package main

import "fmt"

func main() {
    var total, idle uint32 = 0, 1
    fmt.Println("current calculation:", int64(total-idle))
    fmt.Println("signed difference:", int64(total)-int64(idle))
}

2. Run go run main.go.
3. The output is:

current calculation: 4294967295
signed difference: -1

4. Inspect ConnPool.Stats(): it calls p.Len() and p.IdleLen() separately. Each method independently locks and unlocks connsMu. A possible interleaving is that the total is read, an idle connection is added, and then idle is read. The pair can therefore describe different moments even if each individual read is synchronized.

A deterministic concurrency regression test for that interleaving is still needed. This report does not claim to have reproduced the precise runtime transition or to have established it as the only possible trigger.

Context (Environment)

• Source examined: github.com/redis/go-redis/v9 v9.22.0 and github.com/redis/go-redis/extra/redisotel/v9 v9.22.0.
• Also checked current master at 7f3b3dffde59329db9fa7a71d650ef373affb599.
• The arithmetic reproduction requires no Redis server, credentials provider, or telemetry backend.

Detailed Description

Relevant public source:

• Used-connection observation in redisotel v9.22.0.
• TotalConns and IdleConns are uint32.
• Separate locks in Len() / IdleLen() and separate reads in Stats().
• The subtraction on the checked master commit.
• Pool snapshot on the checked master commit.

Related prior report and fix

#3542 reported the same maximum-uint32 used-connection symptom with stable idle counts. The maintainer identified the subtraction, and #3546 fixed a specific pool-bookkeeping problem: a connection removed during streaming-credential authentication failure could remain in the idle list. That report was closed as resolved in v9.14.1.

This report is about the remaining unsigned arithmetic and non-coherent snapshot path. The historical fix changed removal bookkeeping, not the subtraction or the separate snapshot reads. It should not be interpreted as evidence that the old credential-specific bug itself has regressed.

Possible Implementation

A focused follow-up PR could add a regression test for consistent total/idle observations under pool changes, along with telemetry coverage demonstrating that an inconsistent pair cannot turn into a multi-billion used count. The exact handling of inconsistent observations should be agreed with maintainers; no public metric names or instrument types need to change.