Skip to content

Commit f43c211

Browse files
committed
test(metrics): cover fairness_id bounding paths; document label cardinality
- Verify the request-metric family bounds fairness_id like the flow control family, and that RecordFlowControlRequestQueueDuration bounds its model labels. - Verify flow GC prunes the collected flow's metric series end to end. - Document the client-derived label caps and GC pruning in docs/metrics.md. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent c404985 commit f43c211

3 files changed

Lines changed: 107 additions & 0 deletions

File tree

docs/metrics.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ Names below omit the subsystem prefix. Unless a section states otherwise, the pr
6363
and the release stage is ALPHA. Request and latency metrics share the label set
6464
`{model_name, target_model_name, fairness_id, priority}`.
6565

66+
Client-derived label values are cardinality-bounded: `model_name` and `target_model_name` (from the
67+
request body) and `fairness_id` (from the `x-llm-d-inference-fairness-id` header) are each capped at
68+
1000 distinct values, after which new values are reported as `other`. Model names configured through
69+
InferenceModelRewrite rules never fold to `other`. Flow control series for a `fairness_id` are
70+
removed when its flow is garbage collected.
71+
6672
### Request and latency
6773

6874
| Name | Type | Notes |

pkg/epp/flowcontrol/registry/registry_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package registry
1919
import (
2020
"context"
2121
"fmt"
22+
"strconv"
2223
"sync"
2324
"sync/atomic"
2425
"testing"
@@ -28,10 +29,12 @@ import (
2829
"github.com/stretchr/testify/assert"
2930
"github.com/stretchr/testify/require"
3031
testclock "k8s.io/utils/clock/testing"
32+
crmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
3133

3234
"github.com/llm-d/llm-d-router/pkg/epp/flowcontrol/contracts"
3335
"github.com/llm-d/llm-d-router/pkg/epp/framework/interface/flowcontrol"
3436
"github.com/llm-d/llm-d-router/pkg/epp/framework/interface/flowcontrol/mocks"
37+
eppmetrics "github.com/llm-d/llm-d-router/pkg/epp/metrics"
3538
)
3639

3740
// --- Test Harness ---
@@ -1349,3 +1352,45 @@ func TestFlowRegistry_FlowErrorScoping(t *testing.T) {
13491352
assert.Equal(t, int32(concurrency), errorCount.Load(), "All requests should fail flow provisioning")
13501353
assert.Equal(t, int32(0), successCount.Load(), "No request should succeed if flow provisioning failed")
13511354
}
1355+
1356+
// countSeriesWithFairnessID gathers the global metrics registry and counts series carrying the
1357+
// given fairness_id label value, across all metric families.
1358+
func countSeriesWithFairnessID(t *testing.T, fairnessID string) int {
1359+
t.Helper()
1360+
families, err := crmetrics.Registry.Gather()
1361+
require.NoError(t, err, "gathering the metrics registry must succeed")
1362+
n := 0
1363+
for _, mf := range families {
1364+
for _, m := range mf.GetMetric() {
1365+
for _, lp := range m.GetLabel() {
1366+
if lp.GetName() == "fairness_id" && lp.GetValue() == fairnessID {
1367+
n++
1368+
}
1369+
}
1370+
}
1371+
}
1372+
return n
1373+
}
1374+
1375+
// Metric series are labeled by the flow's client-derived fairness ID, so they must not outlive the
1376+
// flow: gcFlows prunes them via metrics.DeleteFlowControlFlowSeries once the flow is collected.
1377+
func TestFlowRegistry_GarbageCollection_PrunesMetricSeries(t *testing.T) {
1378+
// Not parallel: reads the process-global metrics registry. The unique fairness ID keeps the
1379+
// assertions isolated from series recorded by other tests.
1380+
eppmetrics.Register()
1381+
h := newRegistryTestHarness(t, harnessOptions{manualGC: true})
1382+
const flowID = "gc-metric-prune-flow"
1383+
key := flowcontrol.FlowKey{ID: flowID, Priority: highPriority}
1384+
1385+
h.openConnectionOnFlow(key)
1386+
eppmetrics.RecordFlowControlRequestEnqueueDuration(
1387+
flowID, strconv.Itoa(highPriority), "Dispatched", time.Millisecond)
1388+
require.Positive(t, countSeriesWithFairnessID(t, flowID), "Setup: series must exist before GC")
1389+
1390+
h.fakeClock.Step(h.config.FlowGCTimeout + time.Second)
1391+
h.fr.ExecuteGCCycle()
1392+
1393+
h.assertFlowDoesNotExist(key, "Setup: idle flow must have been collected")
1394+
assert.Zero(t, countSeriesWithFairnessID(t, flowID),
1395+
"GC must prune every metric series labeled with the collected flow's fairness ID")
1396+
}

pkg/epp/metrics/cardinality_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,59 @@ func TestDeleteFlowControlFlowSeries(t *testing.T) {
152152
require.Equal(t, 1, promtestutil.CollectAndCount(llmdFlowControlRequestEnqueueDuration),
153153
"the llm_d_epp family must be pruned identically")
154154
}
155+
156+
// The bound is applied mechanically in every record function that takes a fairness ID; this guards
157+
// the request-metric family (distinct label ordering from the flow control family) against the
158+
// pattern regressing there.
159+
func TestFairnessLabelBoundOnRequestMetrics(t *testing.T) {
160+
const testCap = 3
161+
oldFairness := fairnessLabelLimiter
162+
fairnessLabelLimiter = newBoundedLabel(testCap)
163+
oldModels := modelLabelLimiter
164+
modelLabelLimiter = newBoundedLabel(10)
165+
requestCounter.Reset()
166+
llmdRequestCounter.Reset()
167+
t.Cleanup(func() {
168+
fairnessLabelLimiter = oldFairness
169+
modelLabelLimiter = oldModels
170+
requestCounter.Reset()
171+
llmdRequestCounter.Reset()
172+
})
173+
174+
for i := 0; i < 100; i++ {
175+
RecordRequestCounter("model-a", "model-a", fmt.Sprintf("tenant-%d", i), 0)
176+
}
177+
178+
require.Equal(t, testCap+1, promtestutil.CollectAndCount(llmdRequestCounter),
179+
"100 distinct fairness IDs must collapse to cap+overflow series on the request family")
180+
require.Equal(t, 1, promtestutil.CollectAndCount(requestCounter),
181+
"the deprecated family has no fairness_id label and must stay a single series")
182+
}
183+
184+
// RecordFlowControlRequestQueueDuration takes request-body model names; they must flow through the
185+
// model limiter like every sibling record function.
186+
func TestQueueDurationBoundsModelLabels(t *testing.T) {
187+
const testCap = 3
188+
oldModels := modelLabelLimiter
189+
modelLabelLimiter = newBoundedLabel(testCap)
190+
oldFairness := fairnessLabelLimiter
191+
fairnessLabelLimiter = newBoundedLabel(10)
192+
flowControlRequestQueueDuration.Reset()
193+
llmdFlowControlRequestQueueDuration.Reset()
194+
t.Cleanup(func() {
195+
modelLabelLimiter = oldModels
196+
fairnessLabelLimiter = oldFairness
197+
flowControlRequestQueueDuration.Reset()
198+
llmdFlowControlRequestQueueDuration.Reset()
199+
})
200+
201+
for i := 0; i < 100; i++ {
202+
m := fmt.Sprintf("model-%d", i)
203+
RecordFlowControlRequestQueueDuration("tenant", "0", "Dispatched", "pool", m, m, time.Millisecond)
204+
}
205+
206+
require.Equal(t, testCap+1, promtestutil.CollectAndCount(flowControlRequestQueueDuration),
207+
"100 distinct model names must collapse to cap+overflow series, not one series each")
208+
require.Equal(t, testCap+1, promtestutil.CollectAndCount(llmdFlowControlRequestQueueDuration),
209+
"the llm_d_epp family must be bounded identically")
210+
}

0 commit comments

Comments
 (0)