Skip to content

Commit 206cb09

Browse files
authored
fix(usage): use nearest-rank for latency percentiles (#1764)
## Bug `ToolUsage.Percentile` computes the target rank as `int64(float64(total) * p)`, which truncates. The nearest-rank method needs `ceil`. For small call counts the p95/p99 lands one rank too low, so the slowest call never shows up in the tail percentile. The Usage view (`GET /api/v1/activity/usage`, `httpapi/activity.go`) reports exactly these values as the tool's p95. ## Trigger One tool receives 9 calls at ~30 ms and 1 call at 3000 ms. - Expected p95: nearest rank `ceil(9.5) = 10`, which is the 3000 ms call, so the bucket bound is ≤ 5000 ms. - Actual: rank `int(9.5) = 9`, so p95 is ≤ 50 ms. The slow call is invisible. ## Fix Use `math.Ceil` for the target rank. The existing `target < 1` clamp is kept, and large totals are unaffected, because the existing 100-call tests still pass. ## Test `TestToolUsagePercentile_SmallTotalsUseNearestRank` covers the 9-fast + 1-slow case above. It asserts p95 = 5000 (the bucket that holds 3000 ms) and p50 = 50. ## Commands run - `CGO_ENABLED=0 go test -count=1 ./internal/runtime`: ok - `CGO_ENABLED=0 go test -count=1 -run 'Usage|Percentile' ./internal/httpapi`: ok - With the fix reverted (`git stash` of `usage_aggregate.go`), the new test fails with `p95 = 50 ms, want 5000`. - `gofmt -l` on the edited files: no output
1 parent 71ec709 commit 206cb09

2 files changed

Lines changed: 24 additions & 1 deletion

File tree

‎internal/runtime/usage_aggregate.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package runtime
22

33
import (
4+
"math"
45
"sort"
56
"strings"
67
"sync"
@@ -119,7 +120,9 @@ func (t *ToolUsage) Percentile(p float64) (ms int64, exceeds bool) {
119120
if total == 0 {
120121
return 0, false
121122
}
122-
target := int64(float64(total) * p)
123+
// Nearest-rank: truncating would land one rank low for small totals and
124+
// hide the slow tail (9 fast calls + 1 slow one would report p95 as fast).
125+
target := int64(math.Ceil(float64(total) * p))
123126
if target < 1 {
124127
target = 1
125128
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package runtime
2+
3+
import "testing"
4+
5+
// Nearest-rank p95 of 10 calls is the 10th fastest (ceil(9.5)); truncating to
6+
// rank 9 reported nine fast calls plus one 3 s call as a ~30 ms p95.
7+
func TestToolUsagePercentile_SmallTotalsUseNearestRank(t *testing.T) {
8+
tu := &ToolUsage{LatencyBuckets: make([]int64, len(latencyBucketBoundsMs)+1)}
9+
for i := 0; i < 9; i++ {
10+
tu.LatencyBuckets[latencyBucketIndex(30)]++
11+
}
12+
tu.LatencyBuckets[latencyBucketIndex(3000)]++
13+
14+
if got, _ := tu.Percentile(0.95); got != 5000 {
15+
t.Errorf("p95 = %d ms, want 5000 (bucket holding the 3000 ms call)", got)
16+
}
17+
if got, _ := tu.Percentile(0.50); got != 50 {
18+
t.Errorf("p50 = %d ms, want 50", got)
19+
}
20+
}

0 commit comments

Comments
 (0)