diff --git a/cmd/spinloop/fleet_dashboard_test.go b/cmd/spinloop/fleet_dashboard_test.go index b248df05..6f698951 100644 --- a/cmd/spinloop/fleet_dashboard_test.go +++ b/cmd/spinloop/fleet_dashboard_test.go @@ -81,7 +81,7 @@ func (f *fakeDashNode) Metrics(ctx context.Context) (metrics.Stats, error) { s.UptimeSeconds = 60 s.CPU = &metrics.CpuStat{Utilization: 42} s.GPUs = []metrics.GpuStat{{Index: 0, Name: "H100", Utilization: 10, MemoryUsed: 1, MemoryTotal: 10}} - s.Tokens = &metrics.TokenStats{Running: 1, PromptTokens: 100, GenerationTokens: 50, Requests: 3} + s.Tokens = &metrics.TokenStats{Running: 1, PromptTokens: 100, GenerationTokens: 50, Requests: ptrInt(3)} } return s, nil } @@ -349,7 +349,7 @@ func TestDashTileRunningByteStable(t *testing.T) { CPU: &metrics.CpuStat{Utilization: 42}, Memory: &metrics.MemoryStat{Total: 1000, Used: 300}, GPUs: []metrics.GpuStat{{Index: 0, Name: "H100", Utilization: 61, MemoryUsed: 80, MemoryTotal: 160}}, - Tokens: &metrics.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024, Requests: 17}, + Tokens: &metrics.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024, Requests: ptrInt(17)}, }, } want := dashTileExpected([]string{ @@ -371,6 +371,41 @@ func TestDashTileRunningByteStable(t *testing.T) { } } +// A llamacpp node's statistics carry no request figure, and the tile draws no +// line for it — the block keeps its height, and the space the line would have +// taken stays blank. +func TestDashTileRunningNoRequestCount(t *testing.T) { + lipgloss.SetColorProfile(termenv.Ascii) + r := fleet.NodeResult{ + Name: "up", Outcome: fleet.OutcomeOK, + Metrics: metrics.Stats{ + State: "running", Runner: "llamacpp", ModelID: "org/qwen:q4", + UptimeSeconds: 7200, LastActiveAt: "2026-08-21T10:00:00Z", IdleSeconds: 12, + CPU: &metrics.CpuStat{Utilization: 42}, + Memory: &metrics.MemoryStat{Total: 1000, Used: 300}, + GPUs: []metrics.GpuStat{{Index: 0, Name: "H100", Utilization: 61, MemoryUsed: 80, MemoryTotal: 160}}, + Tokens: &metrics.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024}, + }, + } + want := dashTileExpected([]string{ + dashExpectedHeader("up running (up 2h 0m 0s)", dashHealthy), + "llamacpp org/qwen:q4", + " active 12s ago", + dashBar("CPU", 42), + dashBar("RAM", 30), + dashBar("GPU util", 61), + dashBar("GPU mem", 50), + "", + " running: 2", + " prompt tokens: 4096", + " generation tokens: 1024", + "", + }) + if got := dashTestTile("up", r, false, dashAction{}); got != want { + t.Errorf("tile mismatch:\ngot:\n%q\nwant:\n%q", got, want) + } +} + func TestDashTileOutcomeAndEmpty(t *testing.T) { lipgloss.SetColorProfile(termenv.Ascii) dead := fleet.NodeResult{ @@ -896,7 +931,7 @@ func TestDashTileTruncatesTallContent(t *testing.T) { CPU: &metrics.CpuStat{Utilization: 42}, Memory: &metrics.MemoryStat{Total: 1000, Used: 300}, GPUs: gpus, - Tokens: &metrics.TokenStats{Running: 1, PromptTokens: 100, GenerationTokens: 50, Requests: 3}, + Tokens: &metrics.TokenStats{Running: 1, PromptTokens: 100, GenerationTokens: 50, Requests: ptrInt(3)}, }, } lines := strings.Split(dashTestTile("many", r, false, dashAction{}), "\n") @@ -3370,7 +3405,7 @@ func dashHistoryNode() fleet.NodeResult { CPU: &metrics.CpuStat{Utilization: 42}, Memory: &metrics.MemoryStat{Total: 1000, Used: 300}, GPUs: []metrics.GpuStat{{Index: 0, Name: "H100", Utilization: 61, MemoryUsed: 80, MemoryTotal: 160}}, - Tokens: &metrics.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024, Requests: 17}, + Tokens: &metrics.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024, Requests: ptrInt(17)}, History: []metrics.HistorySample{ {Time: 1786276800, CPU: ptrPct(10), Mem: ptrPct(20), GPUs: []metrics.HistoryGPU{{Index: 0, Util: 50, Mem: ptrPct(40)}}}, {Time: 1786276815, CPU: ptrPct(20), Mem: ptrPct(30), GPUs: []metrics.HistoryGPU{{Index: 0, Util: 61, Mem: ptrPct(50)}}}, diff --git a/cmd/spinloop/metrics_render.go b/cmd/spinloop/metrics_render.go index 702a0cb8..02992dcb 100644 --- a/cmd/spinloop/metrics_render.go +++ b/cmd/spinloop/metrics_render.go @@ -428,7 +428,10 @@ func renderStatGauges(w io.Writer, cpu *metrics.CpuStat, mem *metrics.MemoryStat } // renderTokenLines draws the engine's token and request counters, the block -// both formats share. +// both formats share. Each line is drawn only for a figure the statistics +// carry: the request count is absent, not zero, for an engine family whose +// metrics expose no cumulative request counter, so the line is that +// figure's to omit, not the renderer's to keep drawing. func renderTokenLines(w io.Writer, tokens *metrics.TokenStats) { if tokens == nil { return @@ -437,7 +440,9 @@ func renderTokenLines(w io.Writer, tokens *metrics.TokenStats) { fmt.Fprintf(w, " running: %d\n", tokens.Running) fmt.Fprintf(w, " prompt tokens: %d\n", tokens.PromptTokens) fmt.Fprintf(w, " generation tokens: %d\n", tokens.GenerationTokens) - fmt.Fprintf(w, " requests: %d\n", tokens.Requests) + if tokens.Requests != nil { + fmt.Fprintf(w, " requests: %d\n", *tokens.Requests) + } } // renderGPUTable draws the per-GPU lines of the table format, plus the diff --git a/cmd/spinloop/metrics_render_test.go b/cmd/spinloop/metrics_render_test.go index 65711cf9..63fb5b48 100644 --- a/cmd/spinloop/metrics_render_test.go +++ b/cmd/spinloop/metrics_render_test.go @@ -13,6 +13,8 @@ import ( func ptrPct(v float64) *float64 { return &v } +func ptrInt(v int) *int { return &v } + func TestBarGlyph(t *testing.T) { cases := []struct { pct float64 @@ -485,7 +487,7 @@ func TestFormatMetricsBarRunning(t *testing.T) { CPU: &metrics.CpuStat{Utilization: 62}, Memory: &metrics.MemoryStat{Total: 1000, Used: 300}, GPUs: []metrics.GpuStat{{Index: 0, Name: "H100", Utilization: 61, MemoryUsed: 80, MemoryTotal: 160}}, - Tokens: &remote.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024, Requests: 17}, + Tokens: &remote.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024, Requests: ptrInt(17)}, History: []metrics.HistorySample{ {Time: 1, CPU: ptrPct(10), Mem: ptrPct(20), GPUs: []metrics.HistoryGPU{{Index: 0, Util: 50, Mem: ptrPct(50)}}}, {Time: 2, CPU: ptrPct(20), Mem: ptrPct(30), GPUs: []metrics.HistoryGPU{{Index: 0, Util: 61, Mem: ptrPct(50)}}}, @@ -516,6 +518,27 @@ func TestFormatMetricsBarRunning(t *testing.T) { } } +// An engine family whose metrics expose no cumulative request counter yields +// statistics without the figure, and the token block draws no line for it. +func TestFormatMetricsBarNoRequestCount(t *testing.T) { + resp := &remote.StatsResponse{ + Environment: "prod", State: "running", InstanceType: "g5.xlarge", + ModelID: "org/qwen:q4", Version: "0.4.3", + Tokens: &remote.TokenStats{Running: 2, PromptTokens: 4096, GenerationTokens: 1024}, + } + var b bytes.Buffer + if err := renderFleetMetrics(&b, nodeResultsFor(resp), "bar"); err != nil { + t.Fatal(err) + } + got := b.String() + if !strings.Contains(got, " running: 2\n") { + t.Errorf("running line missing: %q", got) + } + if strings.Contains(got, " requests:") { + t.Errorf("requests line drawn for an engine that exposes no request count: %q", got) + } +} + func TestFormatMetricsJSONCarriesHistory(t *testing.T) { resp := &remote.StatsResponse{ Environment: "prod", State: "running", diff --git a/cmd/spinloop/serve_view_test.go b/cmd/spinloop/serve_view_test.go index 38c33f86..512790d5 100644 --- a/cmd/spinloop/serve_view_test.go +++ b/cmd/spinloop/serve_view_test.go @@ -433,7 +433,7 @@ func TestServeViewFrame(t *testing.T) { CPU: &metrics.CpuStat{Utilization: 42}, Memory: &metrics.MemoryStat{Total: 1000, Used: 500}, History: []metrics.HistorySample{{Time: 1, CPU: f64ptr(10), Mem: f64ptr(40)}}, - Tokens: &metrics.TokenStats{PromptTokens: 10, GenerationTokens: 5, Requests: 2}, + Tokens: &metrics.TokenStats{PromptTokens: 10, GenerationTokens: 5, Requests: ptrInt(2)}, } m := newTestServeView( func() (metrics.Stats, error) { return stats, nil }, @@ -482,6 +482,36 @@ func TestServeViewFrame(t *testing.T) { } } +// Statistics without a request figure draw no requests line: the view shows +// the figures the engine exposes and nothing it does not. +func TestServeViewNoRequestCount(t *testing.T) { + fixDashNow(t, time.Date(2026, 9, 6, 12, 0, 0, 0, time.UTC)) + stats := metrics.Stats{ + State: "running", + UptimeSeconds: 90, + Runner: "llama.cpp", + ModelID: "org/model", + CPU: &metrics.CpuStat{Utilization: 42}, + Tokens: &metrics.TokenStats{PromptTokens: 10, GenerationTokens: 5}, + } + m := newTestServeView( + func() (metrics.Stats, error) { return stats, nil }, + func(offset int64, limit int) (daemon.LogsResponse, error) { + return daemon.LogsResponse{Content: "alpha\n", NextOffset: 6}, nil + }, + ) + m.Update(m.startMetricsRead()()) + m.Update(m.startLogPoll()()) + + v := m.View() + if strings.Contains(v, "requests:") { + t.Errorf("a requests line drawn for statistics without the figure:\n%s", v) + } + if !strings.Contains(v, "prompt tokens:") { + t.Errorf("the prompt tokens line is missing:\n%s", v) + } +} + // The same frame, the follow paused: the title bar says so. func TestServeViewFramePaused(t *testing.T) { fixDashNow(t, time.Date(2026, 9, 6, 12, 0, 0, 0, time.UTC)) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 1b2f02b0..e9b019d5 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -481,6 +481,12 @@ components: type: integer requests: type: integer + description: | + The engine's cumulative request counter, present only where the + engine exposes one — vLLM's request_success_total. Absent for an + engine family whose metrics carry no cumulative request count + (llama.cpp's, today): a missing figure is not a zero, it is a + counter the engine never produced. GpuStat: type: object diff --git a/examples/fleet-docker/engine/engine-config.yaml b/examples/fleet-docker/engine/engine-config.yaml index 1c687e92..feb13730 100644 --- a/examples/fleet-docker/engine/engine-config.yaml +++ b/examples/fleet-docker/engine/engine-config.yaml @@ -27,7 +27,9 @@ resources: llamacpp:n_decode_total 900 llamacpp:requests_processing 2 llamacpp:requests_deferred 1 - llamacpp:request_success_total 17 + # No cumulative request counter: a real llama.cpp server serves none, + # and spinloop reports no requests figure for an engine that exposes + # none. # An OpenAI-compatible route, so the node looks like a real endpoint if you # curl it while exploring. diff --git a/examples/gateway-docker/engine/engine-config.yaml b/examples/gateway-docker/engine/engine-config.yaml index b20a5be6..730ba87b 100644 --- a/examples/gateway-docker/engine/engine-config.yaml +++ b/examples/gateway-docker/engine/engine-config.yaml @@ -39,8 +39,9 @@ resources: llamacpp:requests_processing 2 # HELP llamacpp:requests_deferred Number of deferred requests. llamacpp:requests_deferred 1 - # HELP llamacpp:request_success_total Number of successful requests. - llamacpp:request_success_total 17 + # No cumulative request counter: a real llama.cpp server serves none, + # and spinloop reports no requests figure for an engine that exposes + # none. - path: /v1/models method: GET diff --git a/internal/daemon/activity_test.go b/internal/daemon/activity_test.go index 10b8ab90..0f31ff1b 100644 --- a/internal/daemon/activity_test.go +++ b/internal/daemon/activity_test.go @@ -446,6 +446,48 @@ while true; do sleep 0.05; done`) } } +// TestMetricsOmitsRequestCountWhereUnexposed covers the request figure's +// absence through the daemon's own path: the sampler scrapes an engine whose +// metrics expose no cumulative request counter, and the metrics reply carries +// the token stats without the requests field — a missing figure, not a zero +// the engine never produced. +func TestMetricsOmitsRequestCountWhereUnexposed(t *testing.T) { + engineMetrics := &fakeEngine{counter: 100} + engine := httptest.NewServer(engineMetrics) + defer engine.Close() + + d := testDaemon(t, `trap 'exit 0' TERM +while true; do sleep 0.05; done`) + d.SetScrape(metrics.ScrapeTarget{BaseURL: engine.URL, Engine: "llamacpp"}) + if err := d.Push(inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}); err != nil { + t.Fatal(err) + } + if err := d.StartEngine(); err != nil { + t.Fatal(err) + } + waitForState(t, d.Sup, StateRunning) + defer d.Sup.Stop() + d.sampleOnce(context.Background()) + + stats := d.Metrics(context.Background()) + if stats.Tokens == nil { + t.Fatal("metrics carried no token stats") + } + if stats.Tokens.Requests != nil { + t.Errorf("requests = %d, want absent: the engine exposes no cumulative request counter", *stats.Tokens.Requests) + } + body, err := json.Marshal(stats) + if err != nil { + t.Fatal(err) + } + if bytes.Contains(body, []byte(`"requests"`)) { + t.Errorf("an absent request count still serialised: %s", body) + } + if !bytes.Contains(body, []byte(`"promptTokens"`)) { + t.Errorf("the token stats are missing from the reply: %s", body) + } +} + // TestMetricsReportsActivity covers what /v1/metrics now says about activity: // the same answer /v1/status gives, from the same record, including after the // engine has stopped and when there is nothing to report at all. diff --git a/internal/fleet/remote_node_test.go b/internal/fleet/remote_node_test.go index 5ac62dc3..04cb2b41 100644 --- a/internal/fleet/remote_node_test.go +++ b/internal/fleet/remote_node_test.go @@ -127,7 +127,8 @@ func TestStatusFromRemote(t *testing.T) { } func TestStatsFromRemote(t *testing.T) { - tokens := &metrics.TokenStats{Running: 2, PromptTokens: 5, GenerationTokens: 7, Requests: 3} + requests := 3 + tokens := &metrics.TokenStats{Running: 2, PromptTokens: 5, GenerationTokens: 7, Requests: &requests} cpuPct := 30.0 history := []metrics.HistorySample{{Time: 1, CPU: &cpuPct, GPUs: []metrics.HistoryGPU{{Index: 0, Util: 61, Mem: &cpuPct}}}} got := statsFromRemote(remote.StatsResponse{ @@ -139,7 +140,7 @@ func TestStatsFromRemote(t *testing.T) { if got.State != "running" || got.Runner != "llamacpp" || got.ModelID != "org/m" || got.UptimeSeconds != 10 { t.Errorf("statsFromRemote = %+v", got) } - if got.Tokens == nil || got.Tokens.Running != 2 || got.Tokens.Requests != 3 { + if got.Tokens == nil || got.Tokens.Running != 2 || got.Tokens.Requests == nil || *got.Tokens.Requests != 3 { t.Errorf("token stats not carried over: %+v", got.Tokens) } if got.IdleSeconds != 5 || got.LastActiveAt == "" { @@ -521,7 +522,7 @@ func TestRemoteNodeStartStopMetricsOverTheControlPlane(t *testing.T) { if err != nil || stats.State != "running" || stats.ModelID != "org/m" { t.Errorf("Metrics = %+v, %v", stats, err) } - if stats.Tokens == nil || stats.Tokens.Running != 1 || stats.Tokens.Requests != 2 { + if stats.Tokens == nil || stats.Tokens.Running != 1 || stats.Tokens.Requests == nil || *stats.Tokens.Requests != 2 { t.Errorf("metrics tokens not mapped: %+v", stats.Tokens) } stopped, err := node.Stop(ctx) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 99af154f..7499cfe9 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -72,7 +72,14 @@ type TokenStats struct { Counter int `json:"counter"` PromptTokens int `json:"promptTokens"` GenerationTokens int `json:"generationTokens"` - Requests int `json:"requests"` + // Requests is the engine's cumulative request counter, present only + // where the engine family's metrics expose one — vLLM's + // request_success_total, today. A family whose metrics carry no + // cumulative request count (llama.cpp's, today) leaves it nil, so a + // missing figure and a genuine zero stay distinguishable: a zero is a + // counter the engine served, a missing one is a figure no engine + // produced. + Requests *int `json:"requests,omitempty"` } // GpuStat holds per-GPU metrics from nvidia-smi. diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 172ca2b3..34184252 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -1,7 +1,9 @@ package metrics import ( + "bytes" "context" + "encoding/json" "errors" "fmt" "net/http" @@ -122,13 +124,28 @@ func TestParseVMStatMemory(t *testing.T) { } } -const llamacppMetricsFixture = `# HELP llamacpp:prompt_tokens_total Number of prompt tokens processed. +// llamacppMetricsFixture carries the lines a real llama.cpp server run with +// --metrics serves: the token, decode and speculative-decode counters and the +// in-flight request gauges. It deliberately names no cumulative request +// counter, because the engine exposes none. +const llamacppMetricsFixture = `# HELP llamacpp:prompt_tokens_total Number of prompt tokens processed, excluding cached tokens +# TYPE llamacpp:prompt_tokens_total counter llamacpp:prompt_tokens_total 4096 +# HELP llamacpp:tokens_predicted_total Number of generation tokens processed +# TYPE llamacpp:tokens_predicted_total counter llamacpp:tokens_predicted_total 1024 +# HELP llamacpp:n_decode_total Total number of llama_decode() calls, excluding speculative decoding and multimodal decoding +# TYPE llamacpp:n_decode_total counter llamacpp:n_decode_total 900 +# HELP llamacpp:requests_processing Number of requests processing +# TYPE llamacpp:requests_processing gauge llamacpp:requests_processing 2 +# HELP llamacpp:requests_deferred Number of requests deferred +# TYPE llamacpp:requests_deferred gauge llamacpp:requests_deferred 1 -llamacpp:request_success_total 17 +# HELP llamacpp:predicted_tokens_seconds Average generation throughput in tokens/s +# TYPE llamacpp:predicted_tokens_seconds gauge +llamacpp:predicted_tokens_seconds 12.5 other:noise 5 ` @@ -137,10 +154,12 @@ func TestParseTokenStatsLlamacpp(t *testing.T) { if tokens == nil { t.Fatal("got nil token stats") } - want := TokenStats{Running: 3, Counter: 4096 + 1024 + 900, - PromptTokens: 4096, GenerationTokens: 1024, Requests: 17} - if *tokens != want { - t.Errorf("tokens = %+v, want %+v", *tokens, want) + if tokens.Running != 3 || tokens.Counter != 4096+1024+900 || + tokens.PromptTokens != 4096 || tokens.GenerationTokens != 1024 { + t.Errorf("tokens = %+v", *tokens) + } + if tokens.Requests != nil { + t.Errorf("requests = %d, want absent: llama.cpp's metrics expose no cumulative request counter", *tokens.Requests) } } @@ -156,10 +175,62 @@ func TestParseTokenStatsVllm(t *testing.T) { if tokens == nil { t.Fatal("got nil token stats") } - want := TokenStats{Running: 5, Counter: 159, PromptTokens: 100, - GenerationTokens: 50, Requests: 9} - if *tokens != want { - t.Errorf("tokens = %+v, want %+v", *tokens, want) + if tokens.Running != 5 || tokens.Counter != 159 || + tokens.PromptTokens != 100 || tokens.GenerationTokens != 50 { + t.Errorf("tokens = %+v", *tokens) + } + if tokens.Requests == nil || *tokens.Requests != 9 { + t.Errorf("requests = %v, want 9", tokens.Requests) + } +} + +// An engine that serves its request counter before it has served anything +// reports a genuine zero: the figure is present, not absent. +func TestParseTokenStatsVllmZeroRequests(t *testing.T) { + out := strings.Replace(vllmMetricsFixture, + `vllm:request_success_total{finished_reason="stop",model_name="m"} 9`, + `vllm:request_success_total{finished_reason="stop",model_name="m"} 0`, 1) + tokens := ParseTokenStats(out, "vllm") + if tokens == nil { + t.Fatal("got nil token stats") + } + if tokens.Requests == nil || *tokens.Requests != 0 { + t.Errorf("requests = %v, want a present 0", tokens.Requests) + } +} + +// The request figure's absence is a fact in the wire shape: a family without +// a request counter serialises no requests field, and a genuine zero from a +// family that has one still does. +func TestTokenStatsSerialisesRequestCountOptionally(t *testing.T) { + absent, err := json.Marshal(TokenStats{Running: 1, PromptTokens: 10, GenerationTokens: 5}) + if err != nil { + t.Fatal(err) + } + if bytes.Contains(absent, []byte(`"requests"`)) { + t.Errorf("an absent request count still serialised: %s", absent) + } + zero := 0 + present, err := json.Marshal(TokenStats{Running: 1, PromptTokens: 10, GenerationTokens: 5, Requests: &zero}) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(present, []byte(`"requests":0`)) { + t.Errorf("a genuine zero did not serialise: %s", present) + } +} + +// A line whose value matches a metric's shape but does not parse is skipped, +// and the rest of the scrape still parses. +func TestParseTokenStatsSkipsUnparsableValue(t *testing.T) { + out := llamacppMetricsFixture + "llamacpp:tokens_predicted_total +\n" + tokens := ParseTokenStats(out, "llamacpp") + if tokens == nil { + t.Fatal("got nil token stats") + } + if tokens.PromptTokens != 4096 || tokens.GenerationTokens != 1024 || + tokens.Counter != 4096+1024+900 { + t.Errorf("tokens = %+v", *tokens) } } diff --git a/internal/metrics/parse.go b/internal/metrics/parse.go index c87c974f..a40115e5 100644 --- a/internal/metrics/parse.go +++ b/internal/metrics/parse.go @@ -155,14 +155,20 @@ func ParseVMStatMemory(memsize, vmStat string) *MemoryStat { } // engineSpec names the Prometheus metrics one engine family exposes: the -// prefix on every metric, the gauges that count in-flight work, and the -// cumulative counters. The names mirror the stats Lambda's runner-aware -// scrape (remote/lambda/shared/idle.ts) exactly. +// prefix on every metric, the gauges that count in-flight work, the +// cumulative counters, and — where the family exposes one — the cumulative +// counter that carries its request count. The names mirror the stats +// Lambda's runner-aware scrape (remote/lambda/shared/idle.ts) exactly. type engineSpec struct { prefix string running map[string]bool counters map[string]bool generation string + // requests is the family's cumulative request counter, or "" where the + // family's metrics carry none: llama.cpp's serve no request counter at + // all, so a family that named a shared one would read a metric no + // engine serves. + requests string } var engineSpecs = map[string]engineSpec{ @@ -171,6 +177,7 @@ var engineSpecs = map[string]engineSpec{ running: map[string]bool{"num_requests_running": true, "num_requests_waiting": true}, counters: map[string]bool{"prompt_tokens_total": true, "generation_tokens_total": true, "request_success_total": true}, generation: "generation_tokens_total", + requests: "request_success_total", }, "llamacpp": { prefix: "llamacpp", @@ -219,13 +226,20 @@ func ParseTokenStats(out, engine string) *TokenStats { if !matched { return nil } - return &TokenStats{ + tokens := &TokenStats{ Running: int(running), Counter: int(counter), PromptTokens: extractCounter(out, spec, "prompt_tokens_total"), GenerationTokens: extractCounter(out, spec, spec.generation), - Requests: extractCounter(out, spec, "request_success_total"), } + // Set only where the family names a request counter: a missing figure + // is "the engine exposes none", distinct from the zero an engine that + // does expose one reports before it has served anything. + if spec.requests != "" { + requests := extractCounter(out, spec, spec.requests) + tokens.Requests = &requests + } + return tokens } // extractCounter pulls a single named counter out of the raw scrape, matching diff --git a/internal/remote/remote_test.go b/internal/remote/remote_test.go index 58989c1f..5f7ee050 100644 --- a/internal/remote/remote_test.go +++ b/internal/remote/remote_test.go @@ -1276,7 +1276,7 @@ func TestStats_Success(t *testing.T) { if resp.RetainUntil != "2026-01-02T04:00:00Z" { t.Errorf("retainUntil not decoded: %q", resp.RetainUntil) } - if resp.Tokens == nil || resp.Tokens.Requests != 342 { + if resp.Tokens == nil || resp.Tokens.Requests == nil || *resp.Tokens.Requests != 342 { t.Errorf("unexpected tokens: %+v", resp.Tokens) } if len(resp.GPUs) != 1 || resp.GPUs[0].Utilization != 85 { diff --git a/openspec/changes/fix-llamacpp-requests-always-zero/.openspec.yaml b/openspec/changes/fix-llamacpp-requests-always-zero/.openspec.yaml new file mode 100644 index 00000000..abd7c5ae --- /dev/null +++ b/openspec/changes/fix-llamacpp-requests-always-zero/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-25 diff --git a/openspec/changes/fix-llamacpp-requests-always-zero/design.md b/openspec/changes/fix-llamacpp-requests-always-zero/design.md new file mode 100644 index 00000000..ae8cfb71 --- /dev/null +++ b/openspec/changes/fix-llamacpp-requests-always-zero/design.md @@ -0,0 +1,117 @@ +## Context + +See proposal.md — Why. The implementation-relevant current state: + +- `ParseTokenStats` (`internal/metrics/parse.go`) parses an engine's + Prometheus text against a per-family spec: the metric prefix, the gauges + that count in-flight work, and the cumulative counters. It sets + `Requests` from a metric named `request_success_total` for **every** + family — a name only vLLM's endpoint serves. +- A live llama.cpp server run with `--metrics` serves no request counter at + all. Its request-bearing lines are the in-flight gauges + `requests_processing` and `requests_deferred` (already read into + `Running`); its counters are token, time, decode and speculative-decode + figures. Nothing counts completed requests. +- `TokenStats.Requests` is a plain `int` with no `omitempty`, so the field + serialises whatever it holds — including a zero no engine produced — and + the token block draws it unconditionally on every surface that renders + the statistics. +- The engine-metrics spec says the collected statistics include the request + count *as exposed by the engine*, and the package charter says every stat + is optional: a host without a source for one omits it rather than + erroring, and an absence is distinguishable from a zero. +- The field crosses the daemon API (`docs/openapi.yaml`), the remote relay + (`remote/lambda/shared/stats.ts`) and the `metrics` JSON output, so its + optionality is a contract change as well as a parsing change. + +## Goals / Non-Goals + +**Goals:** + +- A `requests` figure a user reads is one the engine's own metrics + produced. +- An engine family whose metrics carry no cumulative request counter yields + statistics without the figure, the way an engine with no metrics endpoint + yields statistics without any. +- The fix is verifiable against what engines actually serve, and the + fixtures say what engines say. + +**Non-Goals:** + +- No estimation. No counter of request completions kept across scrapes by + the daemon from the in-flight gauges: with parallel slots, gauge + transitions between scrapes are lossy, and a figure the engine never said + is a second fabrication where the first was. +- No change to what either engine is asked to serve, and no change to + `Running`, `Counter`, the token counters, or any system stat. +- No new output format, and no change to the formats' structure beyond a + line that can now be absent. + +## Decisions + +**D1: The request counter is per engine family, and may be none.** + +`engineSpec` names, beside its gauges and counters, the cumulative counter +that carries the family's request count: `request_success_total` for vLLM, +none for llama.cpp. `ParseTokenStats` sets the figure from that counter +where the family names one and leaves it unset otherwise. + +The alternative — reading one shared name for every family — is the bug. +Estimating the figure from gauge transitions was rejected in the +Non-Goals. + +**D2: Absence is a pointer, not a zero.** + +`TokenStats.Requests` becomes `*int` with `json:"requests,omitempty"`. +*Set* means the family names a request counter: the value is that +counter's, including a genuine zero from an engine that has started but +served nothing. *Unset* means the family names none, and the field does +not serialise at all. Telling "no figure" from "a zero figure" is the same +distinction the spec already requires of the last-active pair and of every +system stat. + +The alternative — leaving the field a plain `int` and having the renderers +hide a zero — was rejected: it keeps the field in every JSON reply and +forces every consumer to know that a zero from a llama.cpp node means +"not exposed" while a zero from a vLLM node means "served nothing". The +pointer makes the distinction in the data, where a contract change belongs. + +**D3: The token block draws a line only for a figure the statistics carry.** + +The shared renderer omits the `requests:` line where the figure is unset. +Which lines appear is the statistics' affair, not the renderer's: one code +path draws whatever every surface's statistics carry, and no surface +special-cases a runner. + +**D4: The fixtures serve what engines serve.** + +The unit-test fixture and the two example stacks' engine configs drop the +fabricated `llamacpp:request_success_total` line and carry the lines a real +engine of that family serves. The example engines remain "real" in the +sense the fleet-docker-example spec asks — a process serving the dialect +spinloop parses — and now in the stronger sense too: the lines it serves +are lines an engine of that family actually serves. + +## Risks / Trade-offs + +- [A consumer that reads `tokens.requests` unconditionally sees the field + vanish on llama.cpp nodes] → the OpenAPI description and the TypeScript + mirror both mark it optional in the same change, and a consumer asking + "how many requests did this engine serve" already met an answer that + depends on the engine family: the field is documented as the family's own + counter, present where the family has one. +- [The tile loses a line on llama.cpp] → the line today is a zero the + engine never said; removing a fabrication is the point. The `running` + figure beside it already carries what the gauges say, and the token + counters carry the lifetime totals. +- [Old daemons always sent the field, so the relay's TypeScript mirror is + briefly ahead of the daemons it relays] → an optional field accepts both + an absent and a present value, so a mirror updated before, with, or after + the daemon relays no field is correct throughout. + +## Migration Plan + +None: nothing is persisted, and the change is a revert. The relay's mirror +gains the optionality in the same change as the daemon that first omits the +field; because an optional field accepts both shapes, no ordering +constraint exists between them. diff --git a/openspec/changes/fix-llamacpp-requests-always-zero/proposal.md b/openspec/changes/fix-llamacpp-requests-always-zero/proposal.md new file mode 100644 index 00000000..43a7f6c3 --- /dev/null +++ b/openspec/changes/fix-llamacpp-requests-always-zero/proposal.md @@ -0,0 +1,69 @@ +## Why + +The metrics tile's `requests` figure is always 0 for llama.cpp, while the +other engine-sourced counters work. The collector reads the figure from +`request_success_total` for every engine family — a name only vLLM's metrics +endpoint serves. llama.cpp's server exposes no cumulative request counter at +all: its `/metrics` carries the token, decode and speculative-decode counters +and the in-flight gauges (`requests_processing`, `requests_deferred`, already +read as the `running` figure) and nothing more. So for llama.cpp the tile +draws a zero no engine ever produced. + +The test suite never caught it: the unit-test fixture and the two example +stacks' engine configs fabricate a `llamacpp:request_success_total` line a +real engine would never serve, so the parser is only ever exercised against +output no engine produces. + +The engine-metrics spec already requires the collected statistics to include +the request count *as exposed by the engine*, and the collector's own charter +says every stat is optional — a host without a source for one omits it. This +change enforces that: where the engine exposes no cumulative request counter, +the figure is absent from the collected statistics rather than reported as +zero, and the renderers draw the line only for a figure the statistics +carry. + +## What Changes + +- The collected statistics carry the request count only where the engine's + metrics expose a cumulative request counter: vLLM's + `request_success_total`, as today; none for llama.cpp. The statistics' + `requests` field becomes optional — present with the counter's value, + including a genuine zero, absent where the engine exposes none — in the + daemon's reply, the remote relay's reply, and the `metrics` JSON output. +- The shared token block — the dashboard tile, the `metrics` bar and table + formats, the serve view — draws the `requests:` line only where the + figure is present. +- The unit-test fixture and the two example stacks' engine configs stop + fabricating `llamacpp:request_success_total` and carry the lines a real + engine of that family serves. +- The OpenAPI description and the control plane's TypeScript mirror mark + the field as optional. + +## Capabilities + +### New Capabilities + +(None.) + +### Modified Capabilities + +- `engine-metrics`: the request count is read from the engine's cumulative + request counter where the engine exposes one, and is absent from the + collected statistics where it exposes none — never a zero the engine never + produced. + +## Impact + +- `internal/metrics`: `TokenStats.Requests` becomes a pointer with + `omitempty`; the parser sets it from the engine family's request counter + where the family names one. +- `cmd/spinloop`: the token-block renderer omits the line for an absent + figure. +- `docs/openapi.yaml`, `remote/lambda/shared/stats.ts`: the field optional. +- `internal/metrics/metrics_test.go`, + `examples/fleet-docker/engine/engine-config.yaml`, + `examples/gateway-docker/engine/engine-config.yaml`: fixtures carry what + real engines serve. +- The tests that construct `TokenStats` with the plain `Requests` field, + across `cmd/spinloop`, `internal/remote` and `internal/fleet`, take the + pointer form. diff --git a/openspec/changes/fix-llamacpp-requests-always-zero/specs/engine-metrics/spec.md b/openspec/changes/fix-llamacpp-requests-always-zero/specs/engine-metrics/spec.md new file mode 100644 index 00000000..0c2124d2 --- /dev/null +++ b/openspec/changes/fix-llamacpp-requests-always-zero/specs/engine-metrics/spec.md @@ -0,0 +1,61 @@ +## MODIFIED Requirements + +### Requirement: Engine stats collection + +The system SHALL collect token and request statistics from a running engine by +querying the engine's own metrics endpoint over HTTP on the engine's serving +address. The collected statistics SHALL include the prompt and generated token +counts as the engine exposes them, and the engine's cumulative request count +where the engine exposes one. A request count the engine's metrics do not +expose SHALL be absent from the collected statistics rather than reported as +zero: every stat is optional by design, and a figure no source produced is not +a zero. When the engine requires API-key authentication for its metrics +endpoint, the collector SHALL authenticate with the key the engine was +started with. + +The serving address SHALL be the one the engine was actually told to bind: +when the engine's command states a host or port, those SHALL determine where +the collector looks, in preference to any address configured elsewhere or +compiled in as that engine's default. An engine started on a non-default port +is the ordinary case, not an exception — a deployment that describes what to +serve without stating a base URL still binds wherever its arguments say. + +A bind naming every interface SHALL be read as loopback for collection +purposes. Collection is always to an engine on the same host, and a wildcard +is not an address to dial. + +Where the engine's command states no address at all, the collector SHALL fall +back to a configured base URL, and failing that to the engine's default. + +#### Scenario: Running engine yields token stats + +- **WHEN** metrics are collected while a supervised engine is running and + serving requests +- **THEN** the result includes the engine's prompt token and generated token + counts, and its request count where the engine's metrics expose one + +#### Scenario: An engine without a request counter omits the figure + +- **WHEN** metrics are collected from an engine whose metrics expose no + cumulative request count +- **THEN** the result omits the request count rather than reporting zero, and + the figures the engine does expose are present + +#### Scenario: The engine's own arguments locate it + +- **WHEN** an engine is started on a port other than its default, and no base + URL is configured +- **THEN** the collector queries the port the engine was given, not the + engine's default + +#### Scenario: A wildcard bind is collected over loopback + +- **WHEN** an engine is started bound to every interface +- **THEN** the collector queries it on loopback + +#### Scenario: Unreachable engine does not fail collection + +- **WHEN** metrics are collected and the engine's metrics endpoint cannot be + reached +- **THEN** the result omits engine stats, reports the rest, and the collection + as a whole does not error diff --git a/openspec/changes/fix-llamacpp-requests-always-zero/tasks.md b/openspec/changes/fix-llamacpp-requests-always-zero/tasks.md new file mode 100644 index 00000000..2e8fd6bd --- /dev/null +++ b/openspec/changes/fix-llamacpp-requests-always-zero/tasks.md @@ -0,0 +1,48 @@ +## 1. The collector + +- [x] 1.1 Give `engineSpec` a per-family request-counter name: + `request_success_total` for vllm, none for llamacpp (design D1). + Verify the existing vllm parse test still reads the figure, and a + llamacpp parse of a real engine's lines yields none. +- [x] 1.2 Make `TokenStats.Requests` a `*int` with + `json:"requests,omitempty"`; set it from the family's counter where the + family names one, leave it unset otherwise (design D2). Verify a + genuine zero from an engine that serves the counter still serialises as + `"requests": 0`, and the field is absent from the serialised statistics + where the family names none. +- [x] 1.3 Replace the fabricated `llamacpp:request_success_total` line in the + unit-test fixture with the lines a real llama.cpp engine serves + (design D4), and assert the parse carries no request figure. + +## 2. The renderers and the contract + +- [x] 2.1 Draw the `requests:` line in the shared token block only where the + figure is present (design D3). Verify every surface that draws the + block — the dashboard tile, the `metrics` bar and table formats, the + serve view — keeps the line for statistics that carry the figure and + drops it for statistics that do not. +- [x] 2.2 Mark the field optional in `docs/openapi.yaml`, with a description + saying which engines set it, and in the control plane's TypeScript + mirror (`remote/lambda/shared/stats.ts`). Verify the OpenAPI suite still + passes — it compares field names, so the field stays described; its + optionality is the description's. + +## 3. Fixtures and tests + +- [x] 3.1 Drop the fabricated line from + `examples/fleet-docker/engine/engine-config.yaml` and + `examples/gateway-docker/engine/engine-config.yaml`. Verify neither + example engine serves a line a real engine of that family would not + serve, and the fleet-docker-example spec's scenario — the reported + counters are those the engine's metrics endpoint served — still holds. +- [x] 3.2 Update every test that constructs `TokenStats` with the plain + `Requests:` field, across `cmd/spinloop`, `internal/remote` and + `internal/fleet`. Verify `go test ./...` is green. + +## 4. Verification + +- [x] 4.1 Run `gofmt -l .` (expect no output), `go vet ./...` and + `go test ./... -cover`, confirming total coverage is still >= 80%. +- [x] 4.2 Against a real llama.cpp engine run with `--metrics`, verify the + daemon's statistics reply carries no `requests` field, and the tile and + the `metrics` formats draw no `requests:` line for it. diff --git a/remote/lambda/shared/stats.ts b/remote/lambda/shared/stats.ts index a8be9c19..699b6906 100644 --- a/remote/lambda/shared/stats.ts +++ b/remote/lambda/shared/stats.ts @@ -35,8 +35,12 @@ export interface TokenStats { promptTokens: number; /** Total generation/predicted tokens. */ generationTokens: number; - /** Total successful requests. */ - requests: number; + /** + * The engine's cumulative request counter, present only where the engine + * exposes one — vLLM's request_success_total. Absent for an engine family + * whose metrics carry no cumulative request count (llama.cpp's, today). + */ + requests?: number; } /**